Fix #30, #152, #209: Implement NumberWithUnits UI and classification rules #6153
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| package org.oppia.android.app.customview.interaction | ||
|
|
||
| import android.content.Context | ||
| import android.graphics.Typeface | ||
| import android.util.AttributeSet | ||
| import android.view.KeyEvent | ||
| import android.view.View | ||
| import android.view.inputmethod.EditorInfo | ||
| import androidx.appcompat.widget.AppCompatEditText | ||
| import org.oppia.android.app.player.state.listener.StateKeyboardButtonListener | ||
| import org.oppia.android.app.utility.KeyboardHelper.Companion.hideSoftKeyboard | ||
| import org.oppia.android.app.utility.KeyboardHelper.Companion.showSoftKeyboard | ||
|
|
||
| // TODO(#249): These are the attributes which should be defined in XML, that are required for this interaction view to work correctly | ||
| // hint="Write here." | ||
| // inputType="text" | ||
| // background="@drawable/edit_text_background" | ||
| // maxLength="200". | ||
|
|
||
| /** The custom AppCompatEditText class for number with units input interaction view. */ | ||
| class NumberWithUnitsInputInteractionView @JvmOverloads constructor( | ||
| context: Context, | ||
| attrs: AttributeSet? = null, | ||
| defStyle: Int = android.R.attr.editTextStyle | ||
| ) : AppCompatEditText(context, attrs, defStyle), View.OnFocusChangeListener { | ||
| private var hintText: CharSequence = "" | ||
| private val stateKeyboardButtonListener: StateKeyboardButtonListener | ||
|
|
||
| init { | ||
| onFocusChangeListener = this | ||
| // Assume multi-line for the purpose of properly showing long hints. | ||
| isSingleLine = hint != null | ||
| stateKeyboardButtonListener = context as StateKeyboardButtonListener | ||
| } | ||
|
|
||
| // TODO(#4574): Add tests to verify that the placeholder correctly shows/doesn’t show when expected | ||
| override fun onFocusChange(v: View, hasFocus: Boolean) = if (hasFocus) { | ||
| hintText = hint | ||
| hideHint() | ||
| showSoftKeyboard(v, context) | ||
| } else { | ||
| restoreHint() | ||
| hideSoftKeyboard(v, context) | ||
| } | ||
|
|
||
| override fun onKeyPreIme(keyCode: Int, event: KeyEvent): Boolean { | ||
| if (event.keyCode == KeyEvent.KEYCODE_BACK && event.action == KeyEvent.ACTION_UP) { | ||
| clearFocus() | ||
| restoreHint() | ||
| } | ||
| return super.onKeyPreIme(keyCode, event) | ||
| } | ||
|
|
||
| override fun onEditorAction(actionCode: Int) { | ||
| if (actionCode == EditorInfo.IME_ACTION_DONE) { | ||
| stateKeyboardButtonListener.onEditorAction(EditorInfo.IME_ACTION_DONE) | ||
| } | ||
| super.onEditorAction(actionCode) | ||
| } | ||
|
|
||
| private fun hideHint() { | ||
| hint = "" | ||
| typeface = Typeface.DEFAULT | ||
| isSingleLine = true | ||
| } | ||
|
|
||
| private fun restoreHint() { | ||
| hint = hintText | ||
| if (text?.isEmpty() == true) setTypeface(typeface, Typeface.ITALIC) | ||
| isSingleLine = false | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,183 @@ | ||
| package org.oppia.android.app.player.state.itemviewmodel | ||
|
|
||
| import android.text.Editable | ||
| import android.text.TextWatcher | ||
| import androidx.annotation.StringRes | ||
| import androidx.databinding.Observable | ||
| import androidx.databinding.ObservableField | ||
| import org.oppia.android.app.model.AnswerErrorCategory | ||
| import org.oppia.android.app.model.Interaction | ||
| import org.oppia.android.app.model.InteractionObject | ||
| import org.oppia.android.app.model.UserAnswer | ||
| import org.oppia.android.app.model.UserAnswerState | ||
| import org.oppia.android.app.model.WrittenTranslationContext | ||
| import org.oppia.android.app.player.state.answerhandling.InteractionAnswerErrorOrAvailabilityCheckReceiver | ||
| import org.oppia.android.app.player.state.answerhandling.InteractionAnswerHandler | ||
| import org.oppia.android.app.player.state.answerhandling.InteractionAnswerReceiver | ||
| import org.oppia.android.app.translation.AppLanguageResourceHandler | ||
| import org.oppia.android.app.view.models.R | ||
| import org.oppia.android.domain.translation.TranslationController | ||
| import javax.inject.Inject | ||
|
|
||
| /** [StateItemViewModel] for the number with units input interaction. */ | ||
| class NumberWithUnitsInputViewModel private constructor( | ||
| interaction: Interaction, | ||
| val hasConversationView: Boolean, | ||
| private val interactionAnswerErrorOrAvailabilityCheckReceiver: InteractionAnswerErrorOrAvailabilityCheckReceiver, // ktlint-disable max-line-length | ||
| val isSplitView: Boolean, | ||
| private val writtenTranslationContext: WrittenTranslationContext, | ||
| private val resourceHandler: AppLanguageResourceHandler, | ||
| private val translationController: TranslationController, | ||
| userAnswerState: UserAnswerState | ||
| ) : StateItemViewModel(ViewType.NUMBER_WITH_UNITS_INPUT_INTERACTION), InteractionAnswerHandler { | ||
| var answerText: CharSequence = userAnswerState.textInputAnswer | ||
| private var answerErrorCategory: AnswerErrorCategory = AnswerErrorCategory.NO_ERROR | ||
| val hintText: CharSequence = deriveHintText(interaction) | ||
| private var pendingAnswerError: String? = null | ||
|
|
||
| var isAnswerAvailable = ObservableField<Boolean>(false) | ||
|
There was a problem hiding this comment. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🤖 get_repo_knowledge executed:
Length of output: 26120 🏁 Script executed: #!/bin/bash
set -e
file="app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/NumberWithUnitsInputViewModel.kt"
printf '%s\n' '--- target file ---'
cat -n "$file" | sed -n '1,210p'
printf '%s\n' '--- TextParsingUiError declarations and uses ---'
rg -n -C 5 "TextParsingUiError|isAnswerAvailable" app/src/main/java app/src/test 2>/dev/null | head -240Repository: oppia/oppia-android Length of output: 40375 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- NumberWithUnitsInputViewModel references ---'
rg -n -C 2 "NumberWithUnitsInputViewModel|isAnswerAvailable|TextParsingUiError" \
app/src/main app/src/test 2>/dev/null | rg -C 2 \
"NumberWithUnitsInputViewModel|isAnswerAvailable|TextParsingUiError"
printf '%s\n' '--- assignments to the target properties ---'
rg -n "\b(isAnswerAvailable|error)\s*=" \
app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/NumberWithUnitsInputViewModel.ktRepository: oppia/oppia-android Length of output: 42424 Declare the property references as
🤖 Prompt for AI AgentsSource: Path instructions |
||
| val errorMessage = ObservableField<String>("") | ||
|
|
||
| init { | ||
| val callback: Observable.OnPropertyChangedCallback = | ||
| object : Observable.OnPropertyChangedCallback() { | ||
| override fun onPropertyChanged(sender: Observable, propertyId: Int) { | ||
| interactionAnswerErrorOrAvailabilityCheckReceiver.onPendingAnswerErrorOrAvailabilityCheck( | ||
| pendingAnswerError = pendingAnswerError, | ||
| inputAnswerAvailable = true // Allow submit on empty answer. | ||
| ) | ||
| } | ||
| } | ||
| isAnswerAvailable.addOnPropertyChangedCallback(callback) | ||
| errorMessage.addOnPropertyChangedCallback(callback) | ||
|
|
||
| // Initializing with default values so that submit button is enabled by default. | ||
| interactionAnswerErrorOrAvailabilityCheckReceiver.onPendingAnswerErrorOrAvailabilityCheck( | ||
| pendingAnswerError = null, | ||
| inputAnswerAvailable = true | ||
| ) | ||
| checkPendingAnswerError(userAnswerState.answerErrorCategory) | ||
| } | ||
|
|
||
| override fun checkPendingAnswerError(category: AnswerErrorCategory): String? { | ||
| answerErrorCategory = category | ||
| return when (category) { | ||
| AnswerErrorCategory.REAL_TIME -> null | ||
| AnswerErrorCategory.SUBMIT_TIME -> { | ||
| TextParsingUiError.createForText( | ||
| answerText.toString() | ||
| ).createForText(resourceHandler) | ||
| } | ||
| else -> null | ||
| }.also { | ||
| pendingAnswerError = it | ||
| errorMessage.set(it) | ||
| } | ||
| } | ||
|
|
||
| fun getAnswerTextWatcher(): TextWatcher { | ||
|
There was a problem hiding this comment. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Add KDoc for 🤖 Prompt for AI Agents |
||
| return object : TextWatcher { | ||
| override fun beforeTextChanged(s: CharSequence, start: Int, count: Int, after: Int) { | ||
| } | ||
|
|
||
| override fun onTextChanged(answer: CharSequence, start: Int, before: Int, count: Int) { | ||
| answerText = answer.toString().trim() | ||
| val isAnswerTextAvailable = answerText.isNotEmpty() | ||
| if (isAnswerTextAvailable != isAnswerAvailable.get()) { | ||
| isAnswerAvailable.set(isAnswerTextAvailable) | ||
| } | ||
| checkPendingAnswerError(AnswerErrorCategory.REAL_TIME) | ||
| } | ||
|
|
||
| override fun afterTextChanged(s: Editable) { | ||
| } | ||
| } | ||
| } | ||
|
|
||
| override fun getPendingAnswer(): UserAnswer = UserAnswer.newBuilder().apply { | ||
| if (answerText.isNotEmpty()) { | ||
| val answerTextString = answerText.toString() | ||
| answer = InteractionObject.newBuilder().apply { | ||
| normalizedString = answerTextString | ||
| }.build() | ||
|
Comment on lines
+101
to
+103
There was a problem hiding this comment. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 5 --type kotlin \
'NumberWithUnits|numberWithUnits|normalizedString|getPendingAnswer|onAnswerReadyForSubmission' \
app/src domain/srcRepository: oppia/oppia-android Length of output: 50376 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- target view model ---'
sed -n '1,180p' app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/NumberWithUnitsInputViewModel.kt
printf '%s\n' '--- relevant definitions and callers ---'
rg -n -C 8 --glob '*.kt' \
'class StateRetriever|object StateRetriever|fun .*getPendingAnswer|getPendingAnswer\(|onAnswerReadyForSubmission\(|normalizedString|numberWithUnits' \
app/src/main/java domain/src/main/java \
| rg 'StateRetriever|NumberWithUnits|numberWithUnits|normalizedString|getPendingAnswer|onAnswerReadyForSubmission' \
| head -n 500Repository: oppia/oppia-android Length of output: 43544 🤖 get_repo_knowledge executed:
Length of output: 6045 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- answer submission path ---'
sed -n '90,190p' app/src/main/java/org/oppia/android/app/player/state/StateViewModel.kt
sed -n '100,145p' app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt
sed -n '1,90p' app/src/main/java/org/oppia/android/app/player/state/answerhandling/InteractionAnswerHandler.kt
printf '%s\n' '--- classification entry points ---'
rg -n -C 6 --glob '*.kt' \
'classify.*answer|answer.*classif|classifyAnswer|UserAnswer|interactionId|NUMBER_WITH_UNITS|NumberWithUnitsRuleClassifier' \
domain/src/main/java app/src/main/java \
| rg 'classif|Classif|UserAnswer|NUMBER_WITH_UNITS|NumberWithUnits' \
| head -n 600Repository: oppia/oppia-android Length of output: 50375 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- number-with-units classifier files ---'
fd -i 'numberwithunits' domain/src/main/java app/src/main/java
rg -n -C 8 --glob '*.kt' \
'class .*NumberWithUnits|NumberWithUnits.*Classifier|answer\.numberWithUnits|numberWithUnits.*answer|ObjectTypeCase' \
domain/src/main/java/org/oppia/android/domain/classify \
domain/src/main/java/org/oppia/android/domain \
| head -n 800
printf '%s\n' '--- model answer declarations ---'
rg -n -C 5 --glob '*.proto' \
'message UserAnswer|message InteractionObject|number_with_units|normalized_string' \
model/srcRepository: oppia/oppia-android Length of output: 50376 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- number-with-units rule providers ---'
for f in domain/src/main/java/org/oppia/android/domain/classify/rules/numberwithunits/*.kt; do
echo "### $f"
sed -n '1,180p' "$f"
done
printf '%s\n' '--- generic classifier contract ---'
sed -n '1,280p' domain/src/main/java/org/oppia/android/domain/classify/rules/GenericRuleClassifier.kt
sed -n '1,180p' domain/src/main/java/org/oppia/android/domain/classify/AnswerClassificationController.kt
printf '%s\n' '--- classification call ---'
sed -n '480,530p' domain/src/main/java/org/oppia/android/domain/question/QuestionAssessmentProgressController.ktRepository: oppia/oppia-android Length of output: 22890 Serialize the answer as
🤖 Prompt for AI Agents |
||
| plainAnswer = answerTextString | ||
| writtenTranslationContext = this@NumberWithUnitsInputViewModel.writtenTranslationContext | ||
| } | ||
| }.build() | ||
|
|
||
| override fun getUserAnswerState(): UserAnswerState { | ||
| return UserAnswerState.newBuilder().apply { | ||
| this.textInputAnswer = answerText.toString() | ||
| this.answerErrorCategory = answerErrorCategory | ||
| }.build() | ||
| } | ||
|
|
||
| private fun deriveHintText(interaction: Interaction): CharSequence { | ||
| // The subtitled unicode can apparently exist in the structure in two different formats. | ||
| val placeholderUnicodeOption1 = | ||
| interaction.customizationArgsMap["placeholder"]?.subtitledUnicode | ||
| val placeholderUnicodeOption2 = | ||
| interaction.customizationArgsMap["placeholder"]?.customSchemaValue?.subtitledUnicode | ||
| val placeholder1 = | ||
| placeholderUnicodeOption1?.let { unicode -> | ||
| translationController.extractString(unicode, writtenTranslationContext) | ||
| } ?: "" | ||
| val placeholder2 = | ||
| placeholderUnicodeOption2?.let { unicode -> | ||
| translationController.extractString(unicode, writtenTranslationContext) | ||
| } ?: "" // The default placeholder for text input is empty. | ||
| return when { | ||
| placeholder1.isNotEmpty() -> placeholder1 | ||
| placeholder2.isNotEmpty() -> placeholder2 | ||
| else -> resourceHandler.getStringInLocale(R.string.number_with_units_input_hint_text) | ||
| } | ||
| } | ||
|
|
||
| /** Implementation of [StateItemViewModel.InteractionItemFactory] for this view model. */ | ||
| class FactoryImpl @Inject constructor( | ||
| private val resourceHandler: AppLanguageResourceHandler, | ||
| private val translationController: TranslationController | ||
| ) : InteractionItemFactory { | ||
| override fun create( | ||
| entityId: String, | ||
| hasConversationView: Boolean, | ||
| interaction: Interaction, | ||
| interactionAnswerReceiver: InteractionAnswerReceiver, | ||
| answerErrorReceiver: InteractionAnswerErrorOrAvailabilityCheckReceiver, | ||
| hasPreviousButton: Boolean, | ||
| isSplitView: Boolean, | ||
| writtenTranslationContext: WrittenTranslationContext, | ||
| timeToStartNoticeAnimationMs: Long?, | ||
| userAnswerState: UserAnswerState | ||
| ): StateItemViewModel { | ||
| return NumberWithUnitsInputViewModel( | ||
| interaction, | ||
| hasConversationView, | ||
| answerErrorReceiver, | ||
| isSplitView, | ||
| writtenTranslationContext, | ||
| resourceHandler, | ||
| translationController, | ||
| userAnswerState | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| private enum class TextParsingUiError(@StringRes private var error: Int?) { | ||
| /** Corresponds to non empty input. */ | ||
| VALID(error = null), | ||
|
|
||
| /** Corresponds to empty input. */ | ||
| EMPTY_INPUT(error = R.string.text_error_empty_input); | ||
|
|
||
| /** Returns the string corresponding to this error's string resources, or null if there is none. */ | ||
| fun createForText(resourceHandler: AppLanguageResourceHandler): String? = | ||
| error?.let(resourceHandler::getStringInLocale) | ||
|
|
||
| companion object { | ||
| /** Returns the [TextParsingUiError] corresponding to the input. */ | ||
| fun createForText(text: String): TextParsingUiError = | ||
| if (text.isEmpty()) EMPTY_INPUT else VALID | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the commented attribute block.
Configure required attributes in XML, or replace this block with a requirement-only TODO. Do not keep commented-out code.
As per path instructions, follow the linked Oppia coding guide rule that prohibits commented-out code. (github.com)
🤖 Prompt for AI Agents
Source: Path instructions