Skip to content

Commit

Permalink
Fix part of #5070: Display empty answer message in Math expressions i…
Browse files Browse the repository at this point in the history
…nput interaction. (#5317)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## 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)



<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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
<!-- Delete these section if this PR does not include UI-related
changes. -->
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
  • Loading branch information
Vishwajith-Shettigar committed Mar 12, 2024
1 parent 59a41ab commit b763b6a
Show file tree
Hide file tree
Showing 13 changed files with 389 additions and 98 deletions.
1 change: 1 addition & 0 deletions app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@
<activity android:name=".app.testing.InputInteractionViewTestActivity" />
<activity android:name=".app.testing.RatioInputInteractionViewTestActivity" />
<activity android:name=".app.testing.ImageRegionSelectionTestActivity" />
<activity android:name=".app.testing.MathExpressionInteractionsViewTestActivity" />
<activity android:name=".app.testing.ImageViewBindingAdaptersTestActivity" />
<activity android:name=".app.testing.ListItemLeadingMarginSpanTestActivity" />
<activity android:name=".app.testing.MarginBindingAdaptersTestActivity" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -420,6 +427,25 @@ class MathExpressionInteractionsViewModel private constructor(
allowedVariables: List<String>,
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 -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -46,12 +39,8 @@ class InputInteractionViewTestActivity :
@Inject
lateinit var numericInputViewModelFactory: NumericInputViewModel.FactoryImpl

@Inject
lateinit var mathExpViewModelFactoryFactory: MathExpViewModelFactoryFactoryImpl

val numericInputViewModel by lazy { numericInputViewModelFactory.create<NumericInputViewModel>() }

lateinit var mathExpressionViewModel: MathExpressionInteractionsViewModel
lateinit var writtenTranslationContext: WrittenTranslationContext

override fun onCreate(savedInstanceState: Bundle?) {
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<ActivityMathExpressionInteractionViewTestBinding>(
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 <reified T : StateItemViewModel> 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)
}
}
}
}
15 changes: 0 additions & 15 deletions app/src/main/res/layout/activity_input_interaction_view_test.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@
name="numericInputViewModel"
type="org.oppia.android.app.player.state.itemviewmodel.NumericInputViewModel" />

<variable
name="mathExpressionInteractionsViewModel"
type="org.oppia.android.app.player.state.itemviewmodel.MathExpressionInteractionsViewModel" />
</data>

<ScrollView
Expand Down Expand Up @@ -61,18 +58,6 @@
android:textSize="12sp"
android:visibility="@{numericInputViewModel.errorMessage.length() > 0 ? View.VISIBLE : View.INVISIBLE}" />

<org.oppia.android.app.customview.interaction.MathExpressionInteractionsView
android:id="@+id/test_math_expression_input_interaction_view"
style="@style/InputInteractionEditText"
android:inputType="text"
android:minHeight="48dp"
android:text="@={mathExpressionInteractionsViewModel.answerText}"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"
app:placeholder="@{mathExpressionInteractionsViewModel.hintText}"
app:textChangedListener="@{mathExpressionInteractionsViewModel.answerTextWatcher}" />

<Button
android:id="@+id/submit_button"
style="@style/StateButtonActive"
Expand Down

0 comments on commit b763b6a

Please sign in to comment.