From b763b6a31b262dcb11adef5ef35535da148309d5 Mon Sep 17 00:00:00 2001 From: Vishwajith Shettigar <76042077+Vishwajith-Shettigar@users.noreply.github.com> Date: Wed, 13 Mar 2024 05:28:22 +0530 Subject: [PATCH] Fix part of #5070: Display empty answer message in Math expressions input interaction. (#5317) ## Explanation Fix part of #5070, In Math expressions Interaction UI, leave submit button enabled when answer is empty. Show an error on submitting an empty answer. MathExpressionInteractionsViewTestActivity added to accessibility_label_exemptions and test_file_exemptions files. ### Numeric expression [numerix_expression.webm](https://github.com/oppia/oppia-android/assets/76042077/ce7e3e2a-e209-4197-ac3b-4a4ab281ac53) ### Algebraic expression [algebraic_expression.webm](https://github.com/oppia/oppia-android/assets/76042077/62222fe4-9cef-49cf-a86d-d74a0908d0a0) ### Math equation [math_equation.webm](https://github.com/oppia/oppia-android/assets/76042077/fb097e88-c8b2-4159-bdcf-2b4e64386d78) ## 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 --- app/src/main/AndroidManifest.xml | 1 + .../app/activity/ActivityComponentImpl.kt | 2 + .../MathExpressionInteractionsViewModel.kt | 50 ++++-- .../InputInteractionViewTestActivity.kt | 39 ----- ...hExpressionInteractionsViewTestActivity.kt | 147 ++++++++++++++++++ .../activity_input_interaction_view_test.xml | 15 -- ..._math_expression_interaction_view_test.xml | 64 ++++++++ app/src/main/res/values/strings.xml | 3 + .../MathExpressionInteractionsViewTest.kt | 106 +++++++++++-- .../app/player/state/StateFragmentTest.kt | 46 +++--- model/src/main/proto/arguments.proto | 12 ++ .../accessibility_label_exemptions.textproto | 1 + scripts/assets/test_file_exemptions.textproto | 1 + 13 files changed, 389 insertions(+), 98 deletions(-) create mode 100644 app/src/main/java/org/oppia/android/app/testing/MathExpressionInteractionsViewTestActivity.kt create mode 100644 app/src/main/res/layout/activity_math_expression_interaction_view_test.xml diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index dd85589b1b9..41d1ce55918 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -224,6 +224,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 a4f6d5a6d5d..12c9b38749e 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 @@ -73,6 +73,7 @@ import org.oppia.android.app.testing.ImageViewBindingAdaptersTestActivity import org.oppia.android.app.testing.InputInteractionViewTestActivity import org.oppia.android.app.testing.ListItemLeadingMarginSpanTestActivity import org.oppia.android.app.testing.MarginBindingAdaptersTestActivity +import org.oppia.android.app.testing.MathExpressionInteractionsViewTestActivity import org.oppia.android.app.testing.NavigationDrawerTestActivity import org.oppia.android.app.testing.PoliciesFragmentTestActivity import org.oppia.android.app.testing.ProfileChooserFragmentTestActivity @@ -152,6 +153,7 @@ interface ActivityComponentImpl : fun inject(imageViewBindingAdaptersTestActivity: ImageViewBindingAdaptersTestActivity) fun inject(inputInteractionViewTestActivity: InputInteractionViewTestActivity) fun inject(textInputInteractionViewTestActivity: TextInputInteractionViewTestActivity) + fun inject(mathExpressionInteractionsViewTestActivity: MathExpressionInteractionsViewTestActivity) fun inject(ratioInputInteractionViewTestActivity: RatioInputInteractionViewTestActivity) fun inject(licenseListActivity: LicenseListActivity) fun inject(licenseTextViewerActivity: LicenseTextViewerActivity) 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 132c988774b..06b6bb3a47c 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 @@ -105,12 +105,18 @@ class MathExpressionInteractionsViewModel private constructor( override fun onPropertyChanged(sender: Observable, propertyId: Int) { errorOrAvailabilityCheckReceiver.onPendingAnswerErrorOrAvailabilityCheck( pendingAnswerError, - answerText.isNotEmpty() + inputAnswerAvailable = true // Allow blank answer submission. ) } } errorMessage.addOnPropertyChangedCallback(callback) isAnswerAvailable.addOnPropertyChangedCallback(callback) + + // Initializing with default values so that submit button is enabled by default. + errorOrAvailabilityCheckReceiver.onPendingAnswerErrorOrAvailabilityCheck( + pendingAnswerError = null, + inputAnswerAvailable = true + ) } override fun getPendingAnswer(): UserAnswer = UserAnswer.newBuilder().apply { @@ -147,18 +153,16 @@ class MathExpressionInteractionsViewModel private constructor( }.build() override fun checkPendingAnswerError(category: AnswerErrorCategory): String? { - if (answerText.isNotEmpty()) { - pendingAnswerError = when (category) { - // There's no support for real-time errors. - AnswerErrorCategory.REAL_TIME -> null - AnswerErrorCategory.SUBMIT_TIME -> { - interactionType.computeSubmitTimeError( - answerText.toString(), allowedVariables, resourceHandler - ) - } + pendingAnswerError = when (category) { + // There's no support for real-time errors. + AnswerErrorCategory.REAL_TIME -> null + AnswerErrorCategory.SUBMIT_TIME -> { + interactionType.computeSubmitTimeError( + answerText.toString(), allowedVariables, resourceHandler + ) } - errorMessage.set(pendingAnswerError) } + errorMessage.set(pendingAnswerError) return pendingAnswerError } @@ -290,7 +294,10 @@ class MathExpressionInteractionsViewModel private constructor( } private companion object { - private enum class InteractionType( + /** + * Enum class representing different types of interactions in a mathematical expression input field. + */ + enum class InteractionType( val viewType: ViewType, @StringRes val defaultHintTextStringId: Int, val hasPlaceholder: Boolean, @@ -420,6 +427,25 @@ class MathExpressionInteractionsViewModel private constructor( allowedVariables: List, appLanguageResourceHandler: AppLanguageResourceHandler ): String? { + if (answerText.isBlank()) { + return when (this) { + NUMERIC_EXPRESSION -> { + appLanguageResourceHandler.getStringInLocale( + R.string.numeric_expression_error_empty_input + ) + } + ALGEBRAIC_EXPRESSION -> { + appLanguageResourceHandler.getStringInLocale( + R.string.algebraic_expression_error_empty_input + ) + } + MATH_EQUATION -> { + appLanguageResourceHandler.getStringInLocale( + R.string.math_equation_error_empty_input + ) + } + } + } return when (val parseResult = parseAnswer(answerText, allowedVariables)) { is MathParsingResult.Failure -> when (val error = parseResult.error) { is DisabledVariablesInUseError -> { 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 86e41201dad..20706aa12a3 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 @@ -10,18 +10,12 @@ import org.oppia.android.app.activity.ActivityComponentImpl import org.oppia.android.app.activity.InjectableAutoLocalizedAppCompatActivity import org.oppia.android.app.customview.interaction.NumericInputInteractionView import org.oppia.android.app.model.InputInteractionViewTestActivityParams -import org.oppia.android.app.model.InputInteractionViewTestActivityParams.MathInteractionType.ALGEBRAIC_EXPRESSION -import org.oppia.android.app.model.InputInteractionViewTestActivityParams.MathInteractionType.MATH_EQUATION -import org.oppia.android.app.model.InputInteractionViewTestActivityParams.MathInteractionType.MATH_INTERACTION_TYPE_UNSPECIFIED -import org.oppia.android.app.model.InputInteractionViewTestActivityParams.MathInteractionType.NUMERIC_EXPRESSION -import org.oppia.android.app.model.InputInteractionViewTestActivityParams.MathInteractionType.UNRECOGNIZED 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.MathExpressionInteractionsViewModel import org.oppia.android.app.player.state.itemviewmodel.NumericInputViewModel import org.oppia.android.app.player.state.itemviewmodel.StateItemViewModel import org.oppia.android.app.player.state.itemviewmodel.StateItemViewModel.InteractionItemFactory @@ -30,7 +24,6 @@ import org.oppia.android.databinding.ActivityInputInteractionViewTestBinding import org.oppia.android.util.extensions.getProtoExtra import org.oppia.android.util.extensions.putProtoExtra import javax.inject.Inject -import org.oppia.android.app.player.state.itemviewmodel.MathExpressionInteractionsViewModel.FactoryImpl.FactoryFactoryImpl as MathExpViewModelFactoryFactoryImpl /** * This is a dummy activity to test input interaction views. @@ -46,12 +39,8 @@ class InputInteractionViewTestActivity : @Inject lateinit var numericInputViewModelFactory: NumericInputViewModel.FactoryImpl - @Inject - lateinit var mathExpViewModelFactoryFactory: MathExpViewModelFactoryFactoryImpl - val numericInputViewModel by lazy { numericInputViewModelFactory.create() } - lateinit var mathExpressionViewModel: MathExpressionInteractionsViewModel lateinit var writtenTranslationContext: WrittenTranslationContext override fun onCreate(savedInstanceState: Bundle?) { @@ -67,36 +56,8 @@ class InputInteractionViewTestActivity : InputInteractionViewTestActivityParams.getDefaultInstance() ) writtenTranslationContext = params.writtenTranslationContext - when (params.mathInteractionType) { - NUMERIC_EXPRESSION -> { - mathExpressionViewModel = - mathExpViewModelFactoryFactory - .createFactoryForNumericExpression() - .create(interaction = params.interaction) - } - ALGEBRAIC_EXPRESSION -> { - mathExpressionViewModel = - mathExpViewModelFactoryFactory - .createFactoryForAlgebraicExpression() - .create(interaction = params.interaction) - } - MATH_EQUATION -> { - mathExpressionViewModel = - mathExpViewModelFactoryFactory - .createFactoryForMathEquation() - .create(interaction = params.interaction) - } - MATH_INTERACTION_TYPE_UNSPECIFIED, UNRECOGNIZED, null -> { - // Default to numeric expression arbitrarily (since something needs to be defined). - mathExpressionViewModel = - mathExpViewModelFactoryFactory - .createFactoryForNumericExpression() - .create(interaction = params.interaction) - } - } binding.numericInputViewModel = numericInputViewModel - binding.mathExpressionInteractionsViewModel = mathExpressionViewModel } fun getPendingAnswerErrorOnSubmitClick(v: View) { diff --git a/app/src/main/java/org/oppia/android/app/testing/MathExpressionInteractionsViewTestActivity.kt b/app/src/main/java/org/oppia/android/app/testing/MathExpressionInteractionsViewTestActivity.kt new file mode 100644 index 00000000000..55219840ea9 --- /dev/null +++ b/app/src/main/java/org/oppia/android/app/testing/MathExpressionInteractionsViewTestActivity.kt @@ -0,0 +1,147 @@ +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.model.Interaction +import org.oppia.android.app.model.MathExpressionInteractionsViewTestActivityParams +import org.oppia.android.app.model.MathExpressionInteractionsViewTestActivityParams.MathInteractionType +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.MathExpressionInteractionsViewModel +import org.oppia.android.app.player.state.itemviewmodel.StateItemViewModel +import org.oppia.android.app.player.state.listener.StateKeyboardButtonListener +import org.oppia.android.databinding.ActivityMathExpressionInteractionViewTestBinding +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 input interaction views. + * It contains [MathExpressionInteractionsView]. + */ +class MathExpressionInteractionsViewTestActivity : + InjectableAutoLocalizedAppCompatActivity(), + StateKeyboardButtonListener, + InteractionAnswerErrorOrAvailabilityCheckReceiver, + InteractionAnswerReceiver { + + private lateinit var binding: + ActivityMathExpressionInteractionViewTestBinding + + /** + * Injects the [MathExpressionInteractionsViewModel.FactoryImpl] for creating + * [MathExpressionInteractionsViewModel] instances. + */ + @Inject + lateinit var mathExpViewModelFactoryFactory: + MathExpressionInteractionsViewModel.FactoryImpl.FactoryFactoryImpl + + /** The [MathExpressionInteractionsViewModel] instance. */ + lateinit var mathExpressionViewModel: MathExpressionInteractionsViewModel + + /** 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_math_expression_interaction_view_test + ) + val params = + intent.getProtoExtra( + TEST_ACTIVITY_PARAMS_ARGUMENT_KEY, + MathExpressionInteractionsViewTestActivityParams.getDefaultInstance() + ) + writtenTranslationContext = params.writtenTranslationContext + when (params.mathInteractionType) { + MathInteractionType.NUMERIC_EXPRESSION -> { + mathExpressionViewModel = + mathExpViewModelFactoryFactory + .createFactoryForNumericExpression() + .create(interaction = params.interaction) + } + MathInteractionType.ALGEBRAIC_EXPRESSION -> { + mathExpressionViewModel = + mathExpViewModelFactoryFactory + .createFactoryForAlgebraicExpression() + .create(interaction = params.interaction) + } + MathInteractionType.MATH_EQUATION -> { + mathExpressionViewModel = + mathExpViewModelFactoryFactory + .createFactoryForMathEquation() + .create(interaction = params.interaction) + } + MathInteractionType.MATH_INTERACTION_TYPE_UNSPECIFIED, + MathInteractionType.UNRECOGNIZED, null -> { + // Default to numeric expression arbitrarily (since something needs to be defined). + mathExpressionViewModel = + mathExpViewModelFactoryFactory + .createFactoryForNumericExpression() + .create(interaction = params.interaction) + } + } + + binding.mathExpressionInteractionsViewModel = mathExpressionViewModel + } + + /** Checks submit-time errors. */ + fun getPendingAnswerErrorOnSubmitClick(v: View) { + mathExpressionViewModel.checkPendingAnswerError(AnswerErrorCategory.SUBMIT_TIME) + } + + override fun onPendingAnswerErrorOrAvailabilityCheck( + pendingAnswerError: String?, + inputAnswerAvailable: Boolean + ) { + binding.submitButton.isEnabled = pendingAnswerError == null + } + + override fun onAnswerReadyForSubmission(answer: UserAnswer) { + } + + override fun onEditorAction(actionCode: Int) { + } + + private inline fun StateItemViewModel + .InteractionItemFactory.create( + interaction: Interaction = Interaction.getDefaultInstance() + ): T { + return create( + entityId = "fake_entity_id", + hasConversationView = false, + interaction = interaction, + interactionAnswerReceiver = this@MathExpressionInteractionsViewTestActivity, + answerErrorReceiver = this@MathExpressionInteractionsViewTestActivity, + hasPreviousButton = false, + isSplitView = false, + writtenTranslationContext, + timeToStartNoticeAnimationMs = null + ) as T + } + + companion object { + private const val TEST_ACTIVITY_PARAMS_ARGUMENT_KEY = + "MathExpressionInteractionsViewTestActivity.params" + + /** Function to create intent for MathExpressionInteractionsViewTestActivity. */ + fun createIntent( + context: Context, + extras: MathExpressionInteractionsViewTestActivityParams + ): Intent { + return Intent(context, MathExpressionInteractionsViewTestActivity::class.java).also { + it.putProtoExtra(TEST_ACTIVITY_PARAMS_ARGUMENT_KEY, extras) + } + } + } +} diff --git a/app/src/main/res/layout/activity_input_interaction_view_test.xml b/app/src/main/res/layout/activity_input_interaction_view_test.xml index 9a7a8b277f3..c60eb2fec5e 100644 --- a/app/src/main/res/layout/activity_input_interaction_view_test.xml +++ b/app/src/main/res/layout/activity_input_interaction_view_test.xml @@ -11,9 +11,6 @@ name="numericInputViewModel" type="org.oppia.android.app.player.state.itemviewmodel.NumericInputViewModel" /> - - -