From 6723268c87de074b264022eac0e2dbad5db55d58 Mon Sep 17 00:00:00 2001 From: masclot <103062089+masclot@users.noreply.github.com> Date: Tue, 9 Jan 2024 18:29:19 +0100 Subject: [PATCH 1/3] Fix #4135, Fix part of #5070: In FractionInteraction UI, leave submit button enabled when answer is empty. (#5224) Fix part of https://github.com/oppia/oppia-android/issues/5070: In FractionInteraction UI, leave submit button enabled when answer is empty. Show an error on submitting an empty answer. The error message already exists and is the same as in https://github.com/oppia/oppia/pull/18379. Demo video: [leave_submit_button_enabled_on_empty_answer_v3.webm](https://github.com/oppia/oppia-android/assets/103062089/d072ae88-c462-455c-a324-57680d4a82c5) The new error messages for empty inputs on submit are listed here: [oppia/design-team#71(comment)](https://github.com/oppia/design-team/issues/71#issuecomment-1581232000) I added an accessibility-label exemption for FractionInputInteractionViewTestActivity as this activity is only used in tests. Fix #4135: incidentaly, this change also fixes #4135, since I had to split the tests for FractionInputInteraction ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) - Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide)) - For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing --------- Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com> --- app/src/main/AndroidManifest.xml | 1 + .../app/activity/ActivityComponentImpl.kt | 2 + .../app/parser/FractionParsingUiError.kt | 6 +- .../FractionInteractionViewModel.kt | 29 +- .../MathExpressionInteractionsViewModel.kt | 8 +- ...ractionInputInteractionViewTestActivity.kt | 112 ++++ .../InputInteractionViewTestActivity.kt | 13 +- ...y_fraction_input_interaction_view_test.xml | 75 +++ .../activity_input_interaction_view_test.xml | 41 -- app/src/main/res/values/strings.xml | 1 + .../app/player/state/StateFragmentTest.kt | 7 +- ...ionInputInteractionViewTestActivityTest.kt | 608 ++++++++++++++++++ .../InputInteractionViewTestActivityTest.kt | 394 ------------ .../app/parser/FractionParsingUiErrorTest.kt | 2 +- .../accessibility_label_exemptions.textproto | 1 + .../oppia/android/util/math/FractionParser.kt | 8 +- .../android/util/math/FractionParserTest.kt | 4 +- 17 files changed, 847 insertions(+), 465 deletions(-) create mode 100644 app/src/main/java/org/oppia/android/app/testing/FractionInputInteractionViewTestActivity.kt create mode 100644 app/src/main/res/layout/activity_fraction_input_interaction_view_test.xml create mode 100644 app/src/sharedTest/java/org/oppia/android/app/testing/FractionInputInteractionViewTestActivityTest.kt diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index c324a1a7119..76102786288 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -213,6 +213,7 @@ + diff --git a/app/src/main/java/org/oppia/android/app/activity/ActivityComponentImpl.kt b/app/src/main/java/org/oppia/android/app/activity/ActivityComponentImpl.kt index 6641b39e76c..14645028aa5 100644 --- a/app/src/main/java/org/oppia/android/app/activity/ActivityComponentImpl.kt +++ b/app/src/main/java/org/oppia/android/app/activity/ActivityComponentImpl.kt @@ -64,6 +64,7 @@ import org.oppia.android.app.testing.DragDropTestActivity import org.oppia.android.app.testing.DrawableBindingAdaptersTestActivity import org.oppia.android.app.testing.ExplorationInjectionActivity import org.oppia.android.app.testing.ExplorationTestActivity +import org.oppia.android.app.testing.FractionInputInteractionViewTestActivity import org.oppia.android.app.testing.HomeFragmentTestActivity import org.oppia.android.app.testing.HomeTestActivity import org.oppia.android.app.testing.HtmlParserTestActivity @@ -139,6 +140,7 @@ interface ActivityComponentImpl : fun inject(faqSingleActivity: FAQSingleActivity) fun inject(forceNetworkTypeActivity: ForceNetworkTypeActivity) fun inject(forceNetworkTypeTestActivity: ForceNetworkTypeTestActivity) + fun inject(fractionInputInteractionViewTestActivity: FractionInputInteractionViewTestActivity) fun inject(helpActivity: HelpActivity) fun inject(homeActivity: HomeActivity) fun inject(homeFragmentTestActivity: HomeFragmentTestActivity) diff --git a/app/src/main/java/org/oppia/android/app/parser/FractionParsingUiError.kt b/app/src/main/java/org/oppia/android/app/parser/FractionParsingUiError.kt index 731d26e0590..81cd9e14035 100644 --- a/app/src/main/java/org/oppia/android/app/parser/FractionParsingUiError.kt +++ b/app/src/main/java/org/oppia/android/app/parser/FractionParsingUiError.kt @@ -20,7 +20,10 @@ enum class FractionParsingUiError(@StringRes private var error: Int?) { DIVISION_BY_ZERO(error = R.string.fraction_error_divide_by_zero), /** Corresponds to [FractionParsingError.NUMBER_TOO_LONG]. */ - NUMBER_TOO_LONG(error = R.string.fraction_error_larger_than_seven_digits); + NUMBER_TOO_LONG(error = R.string.fraction_error_larger_than_seven_digits), + + /** Corresponds to [FractionParsingError.EMPTY_INPUT]. */ + EMPTY_INPUT(error = R.string.fraction_error_empty_input); /** * Returns the string corresponding to this error's string resources, or null if there is none. @@ -39,6 +42,7 @@ enum class FractionParsingUiError(@StringRes private var error: Int?) { FractionParsingError.INVALID_FORMAT -> INVALID_FORMAT FractionParsingError.DIVISION_BY_ZERO -> DIVISION_BY_ZERO FractionParsingError.NUMBER_TOO_LONG -> NUMBER_TOO_LONG + FractionParsingError.EMPTY_INPUT -> EMPTY_INPUT } } } diff --git a/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/FractionInteractionViewModel.kt b/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/FractionInteractionViewModel.kt index 22d42a74744..193248effe7 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/FractionInteractionViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/FractionInteractionViewModel.kt @@ -43,12 +43,17 @@ class FractionInteractionViewModel private constructor( override fun onPropertyChanged(sender: Observable, propertyId: Int) { errorOrAvailabilityCheckReceiver.onPendingAnswerErrorOrAvailabilityCheck( pendingAnswerError, - answerText.isNotEmpty() + true // Allow submit on empty answer. ) } } errorMessage.addOnPropertyChangedCallback(callback) isAnswerAvailable.addOnPropertyChangedCallback(callback) + // Force-update the UI to reflect the state of the errorMessage and isAnswerAvailable property: + errorOrAvailabilityCheckReceiver.onPendingAnswerErrorOrAvailabilityCheck( + /* pendingAnswerError= */null, + /* inputAnswerAvailable= */true + ) } override fun getPendingAnswer(): UserAnswer = UserAnswer.newBuilder().apply { @@ -64,23 +69,25 @@ class FractionInteractionViewModel private constructor( /** It checks the pending error for the current fraction input, and correspondingly updates the error string based on the specified error category. */ override fun checkPendingAnswerError(category: AnswerErrorCategory): String? { - if (answerText.isNotEmpty()) { - when (category) { - AnswerErrorCategory.REAL_TIME -> { + when (category) { + AnswerErrorCategory.REAL_TIME -> { + if (answerText.isNotEmpty()) { pendingAnswerError = FractionParsingUiError.createFromParsingError( fractionParser.getRealTimeAnswerError(answerText.toString()) ).getErrorMessageFromStringRes(resourceHandler) + } else { + pendingAnswerError = null } - AnswerErrorCategory.SUBMIT_TIME -> { - pendingAnswerError = - FractionParsingUiError.createFromParsingError( - fractionParser.getSubmitTimeError(answerText.toString()) - ).getErrorMessageFromStringRes(resourceHandler) - } } - errorMessage.set(pendingAnswerError) + AnswerErrorCategory.SUBMIT_TIME -> { + pendingAnswerError = + FractionParsingUiError.createFromParsingError( + fractionParser.getSubmitTimeError(answerText.toString()) + ).getErrorMessageFromStringRes(resourceHandler) + } } + errorMessage.set(pendingAnswerError) return pendingAnswerError } diff --git a/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/MathExpressionInteractionsViewModel.kt b/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/MathExpressionInteractionsViewModel.kt index 5d0822fae6c..132c988774b 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/MathExpressionInteractionsViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/MathExpressionInteractionsViewModel.kt @@ -73,6 +73,12 @@ class MathExpressionInteractionsViewModel private constructor( * bound to the corresponding edit text. */ var answerText: CharSequence = "" + // The value of ths field is set from the Binding and from the TextWatcher. Any + // programmatic modification needs to be done here, so that the Binding and the TextWatcher + // do not step on each other. + set(value) { + field = value.toString().trim() + } /** * Defines whether an answer is currently available to parse. This is expected to be directly @@ -166,7 +172,7 @@ class MathExpressionInteractionsViewModel private constructor( } override fun onTextChanged(answer: CharSequence, start: Int, before: Int, count: Int) { - answerText = answer.toString().trim() + answerText = answer val isAnswerTextAvailable = answerText.isNotEmpty() if (isAnswerTextAvailable != isAnswerAvailable.get()) { isAnswerAvailable.set(isAnswerTextAvailable) diff --git a/app/src/main/java/org/oppia/android/app/testing/FractionInputInteractionViewTestActivity.kt b/app/src/main/java/org/oppia/android/app/testing/FractionInputInteractionViewTestActivity.kt new file mode 100644 index 00000000000..bb812a550a9 --- /dev/null +++ b/app/src/main/java/org/oppia/android/app/testing/FractionInputInteractionViewTestActivity.kt @@ -0,0 +1,112 @@ +package org.oppia.android.app.testing + +import android.content.Context +import android.content.Intent +import android.os.Bundle +import android.view.View +import androidx.databinding.DataBindingUtil +import org.oppia.android.R +import org.oppia.android.app.activity.ActivityComponentImpl +import org.oppia.android.app.activity.InjectableAutoLocalizedAppCompatActivity +import org.oppia.android.app.customview.interaction.FractionInputInteractionView +import org.oppia.android.app.model.InputInteractionViewTestActivityParams +import org.oppia.android.app.model.Interaction +import org.oppia.android.app.model.UserAnswer +import org.oppia.android.app.model.WrittenTranslationContext +import org.oppia.android.app.player.state.answerhandling.AnswerErrorCategory +import org.oppia.android.app.player.state.answerhandling.InteractionAnswerErrorOrAvailabilityCheckReceiver +import org.oppia.android.app.player.state.answerhandling.InteractionAnswerReceiver +import org.oppia.android.app.player.state.itemviewmodel.FractionInteractionViewModel +import org.oppia.android.app.player.state.itemviewmodel.StateItemViewModel +import org.oppia.android.app.player.state.itemviewmodel.StateItemViewModel.InteractionItemFactory +import org.oppia.android.app.player.state.listener.StateKeyboardButtonListener +import org.oppia.android.databinding.ActivityFractionInputInteractionViewTestBinding +import org.oppia.android.util.extensions.getProtoExtra +import org.oppia.android.util.extensions.putProtoExtra +import javax.inject.Inject + +/** + * This is a dummy activity to test [FractionInputInteractionView]. + */ +class FractionInputInteractionViewTestActivity : + InjectableAutoLocalizedAppCompatActivity(), + StateKeyboardButtonListener, + InteractionAnswerErrorOrAvailabilityCheckReceiver, + InteractionAnswerReceiver { + private lateinit var binding: ActivityFractionInputInteractionViewTestBinding + + @Inject + lateinit var fractionInteractionViewModelFactory: FractionInteractionViewModel.FactoryImpl + + /** Gives access to the [FractionInteractionViewModel]. */ + val fractionInteractionViewModel by lazy { + fractionInteractionViewModelFactory.create() + } + + /** Gives access to the translation context. */ + lateinit var writtenTranslationContext: WrittenTranslationContext + + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + (activityComponent as ActivityComponentImpl).inject(this) + binding = DataBindingUtil.setContentView( + this, R.layout.activity_fraction_input_interaction_view_test + ) + + val params = + intent.getProtoExtra( + TEST_ACTIVITY_PARAMS_ARGUMENT_KEY, + InputInteractionViewTestActivityParams.getDefaultInstance() + ) + writtenTranslationContext = params.writtenTranslationContext + binding.fractionInteractionViewModel = fractionInteractionViewModel + } + + /** Checks submit-time errors. */ + fun getPendingAnswerErrorOnSubmitClick(v: View) { + fractionInteractionViewModel.checkPendingAnswerError(AnswerErrorCategory.SUBMIT_TIME) + } + + override fun onPendingAnswerErrorOrAvailabilityCheck( + pendingAnswerError: String?, + inputAnswerAvailable: Boolean + ) { + } + + override fun onAnswerReadyForSubmission(answer: UserAnswer) { + } + + override fun onEditorAction(actionCode: Int) { + } + + private inline fun InteractionItemFactory.create( + interaction: Interaction = Interaction.getDefaultInstance() + ): T { + return create( + entityId = "fake_entity_id", + hasConversationView = false, + interaction = interaction, + interactionAnswerReceiver = this@FractionInputInteractionViewTestActivity, + answerErrorReceiver = this@FractionInputInteractionViewTestActivity, + hasPreviousButton = false, + isSplitView = false, + writtenTranslationContext, + timeToStartNoticeAnimationMs = null + ) as T + } + + companion object { + private const val TEST_ACTIVITY_PARAMS_ARGUMENT_KEY = + "FractionInputInteractionViewTestActivity.params" + + /** Creates an intent to open this activity. */ + fun createIntent( + context: Context, + extras: InputInteractionViewTestActivityParams + ): Intent { + return Intent(context, FractionInputInteractionViewTestActivity::class.java).also { + it.putProtoExtra(TEST_ACTIVITY_PARAMS_ARGUMENT_KEY, extras) + } + } + } +} diff --git a/app/src/main/java/org/oppia/android/app/testing/InputInteractionViewTestActivity.kt b/app/src/main/java/org/oppia/android/app/testing/InputInteractionViewTestActivity.kt index 7190382efa0..ac786bb146a 100644 --- a/app/src/main/java/org/oppia/android/app/testing/InputInteractionViewTestActivity.kt +++ b/app/src/main/java/org/oppia/android/app/testing/InputInteractionViewTestActivity.kt @@ -8,7 +8,6 @@ import androidx.databinding.DataBindingUtil import org.oppia.android.R import org.oppia.android.app.activity.ActivityComponentImpl import org.oppia.android.app.activity.InjectableAutoLocalizedAppCompatActivity -import org.oppia.android.app.customview.interaction.FractionInputInteractionView import org.oppia.android.app.customview.interaction.NumericInputInteractionView import org.oppia.android.app.customview.interaction.TextInputInteractionView import org.oppia.android.app.model.InputInteractionViewTestActivityParams @@ -24,7 +23,6 @@ import org.oppia.android.app.model.WrittenTranslationContext import org.oppia.android.app.player.state.answerhandling.AnswerErrorCategory import org.oppia.android.app.player.state.answerhandling.InteractionAnswerErrorOrAvailabilityCheckReceiver import org.oppia.android.app.player.state.answerhandling.InteractionAnswerReceiver -import org.oppia.android.app.player.state.itemviewmodel.FractionInteractionViewModel import org.oppia.android.app.player.state.itemviewmodel.MathExpressionInteractionsViewModel import org.oppia.android.app.player.state.itemviewmodel.NumericInputViewModel import org.oppia.android.app.player.state.itemviewmodel.RatioExpressionInputInteractionViewModel @@ -40,7 +38,7 @@ import org.oppia.android.app.player.state.itemviewmodel.MathExpressionInteractio /** * This is a dummy activity to test input interaction views. - * It contains [FractionInputInteractionView], [NumericInputInteractionView],and [TextInputInteractionView]. + * It contains [NumericInputInteractionView],and [TextInputInteractionView]. */ class InputInteractionViewTestActivity : InjectableAutoLocalizedAppCompatActivity(), @@ -55,9 +53,6 @@ class InputInteractionViewTestActivity : @Inject lateinit var textInputViewModelFactory: TextInputViewModel.FactoryImpl - @Inject - lateinit var fractionInteractionViewModelFactory: FractionInteractionViewModel.FactoryImpl - @Inject lateinit var ratioViewModelFactory: RatioExpressionInputInteractionViewModel.FactoryImpl @@ -68,10 +63,6 @@ class InputInteractionViewTestActivity : val textInputViewModel by lazy { textInputViewModelFactory.create() } - val fractionInteractionViewModel by lazy { - fractionInteractionViewModelFactory.create() - } - val ratioExpressionInputInteractionViewModel by lazy { ratioViewModelFactory.create( interaction = Interaction.newBuilder().putCustomizationArgs( @@ -127,13 +118,11 @@ class InputInteractionViewTestActivity : binding.numericInputViewModel = numericInputViewModel binding.textInputViewModel = textInputViewModel - binding.fractionInteractionViewModel = fractionInteractionViewModel binding.ratioInteractionInputViewModel = ratioExpressionInputInteractionViewModel binding.mathExpressionInteractionsViewModel = mathExpressionViewModel } fun getPendingAnswerErrorOnSubmitClick(v: View) { - fractionInteractionViewModel.checkPendingAnswerError(AnswerErrorCategory.SUBMIT_TIME) numericInputViewModel.checkPendingAnswerError(AnswerErrorCategory.SUBMIT_TIME) ratioExpressionInputInteractionViewModel .checkPendingAnswerError(AnswerErrorCategory.SUBMIT_TIME) diff --git a/app/src/main/res/layout/activity_fraction_input_interaction_view_test.xml b/app/src/main/res/layout/activity_fraction_input_interaction_view_test.xml new file mode 100644 index 00000000000..ddd18d752d9 --- /dev/null +++ b/app/src/main/res/layout/activity_fraction_input_interaction_view_test.xml @@ -0,0 +1,75 @@ + + + + + + + + + + + + + + + + + +