Skip to content

Commit

Permalink
Fix part of #5070: Display empty answer message in ratio input intera…
Browse files Browse the repository at this point in the history
…ction (#5263)

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

Fixes part of #5070

Enables the `submit_answer_button` when the pending answer is empty.
Instead of disabling the button, an error message, stating "_**Enter a
ratio to continue.**_", is now displayed when the user attempts to
submit a blank answer.

`RatioInputInteractionViewTestActivity` is added to accessibility-label
exemption as this is a test activity.


https://github.com/oppia/oppia-android/assets/84731134/33c797c5-af44-4eaa-980e-fafaccf72d26

## 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

---------

Co-authored-by: Adhiambo Peres <[email protected]>
  • Loading branch information
theMr17 and adhiamboperes authored Jan 11, 2024
1 parent 9ad24ef commit bb69b22
Show file tree
Hide file tree
Showing 12 changed files with 702 additions and 325 deletions.
1 change: 1 addition & 0 deletions app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@
<activity android:name=".app.testing.HtmlParserTestActivity" />
<activity android:name=".app.testing.HomeTestActivity" />
<activity android:name=".app.testing.InputInteractionViewTestActivity" />
<activity android:name=".app.testing.RatioInputInteractionViewTestActivity" />
<activity android:name=".app.testing.ImageRegionSelectionTestActivity" />
<activity android:name=".app.testing.ImageViewBindingAdaptersTestActivity" />
<activity android:name=".app.testing.ListItemLeadingMarginSpanTestActivity" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ import org.oppia.android.app.testing.NavigationDrawerTestActivity
import org.oppia.android.app.testing.PoliciesFragmentTestActivity
import org.oppia.android.app.testing.ProfileChooserFragmentTestActivity
import org.oppia.android.app.testing.ProfileEditFragmentTestActivity
import org.oppia.android.app.testing.RatioInputInteractionViewTestActivity
import org.oppia.android.app.testing.SplashTestActivity
import org.oppia.android.app.testing.SpotlightFragmentTestActivity
import org.oppia.android.app.testing.StateAssemblerMarginBindingAdaptersTestActivity
Expand Down Expand Up @@ -149,6 +150,7 @@ interface ActivityComponentImpl :
fun inject(imageRegionSelectionTestActivity: ImageRegionSelectionTestActivity)
fun inject(imageViewBindingAdaptersTestActivity: ImageViewBindingAdaptersTestActivity)
fun inject(inputInteractionViewTestActivity: InputInteractionViewTestActivity)
fun inject(ratioInputInteractionViewTestActivity: RatioInputInteractionViewTestActivity)
fun inject(licenseListActivity: LicenseListActivity)
fun inject(licenseTextViewerActivity: LicenseTextViewerActivity)
fun inject(listItemLeadingMarginSpanTestActivity: ListItemLeadingMarginSpanTestActivity)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class StringToRatioParser {
val normalized = text.normalizeWhitespace()
val ratio = parseRatioOrNull(normalized)
return when {
normalized.isBlank() -> RatioParsingError.EMPTY_INPUT
!normalized.matches(invalidRatioRegex) || ratio == null -> RatioParsingError.INVALID_FORMAT
numberOfTerms != 0 && ratio.ratioComponentCount != numberOfTerms -> {
RatioParsingError.INVALID_SIZE
Expand Down Expand Up @@ -77,7 +78,8 @@ class StringToRatioParser {
INVALID_FORMAT(error = R.string.ratio_error_invalid_format),
INVALID_COLONS(error = R.string.ratio_error_invalid_colons),
INVALID_SIZE(error = R.string.ratio_error_invalid_size),
INCLUDES_ZERO(error = R.string.ratio_error_includes_zero);
INCLUDES_ZERO(error = R.string.ratio_error_includes_zero),
EMPTY_INPUT(error = R.string.ratio_error_empty_input);

/**
* Returns the string corresponding to this error's string resources, or null if there is none.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,18 @@ class RatioExpressionInputInteractionViewModel 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 All @@ -67,23 +73,24 @@ class RatioExpressionInputInteractionViewModel private constructor(
}
}.build()

/** It checks the pending error for the current ratio input, and correspondingly updates the error string based on the specified error category. */
/**
* It checks the pending error for the current ratio 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 ->
pendingAnswerError =
stringToRatioParser.getRealTimeAnswerError(answerText.toString())
.getErrorMessageFromStringRes(resourceHandler)
AnswerErrorCategory.SUBMIT_TIME ->
pendingAnswerError =
stringToRatioParser.getSubmitTimeError(
answerText.toString(),
numberOfTerms = numberOfTerms
).getErrorMessageFromStringRes(resourceHandler)
}
errorMessage.set(pendingAnswerError)
pendingAnswerError = when (category) {
AnswerErrorCategory.REAL_TIME ->
if (answerText.isNotEmpty())
stringToRatioParser.getRealTimeAnswerError(answerText.toString())
.getErrorMessageFromStringRes(resourceHandler)
else null
AnswerErrorCategory.SUBMIT_TIME ->
stringToRatioParser.getSubmitTimeError(
answerText.toString(),
numberOfTerms = numberOfTerms
).getErrorMessageFromStringRes(resourceHandler)
}
errorMessage.set(pendingAnswerError)
return pendingAnswerError
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,13 @@ import org.oppia.android.app.model.InputInteractionViewTestActivityParams.MathIn
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.SchemaObject
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.RatioExpressionInputInteractionViewModel
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.itemviewmodel.TextInputViewModel
Expand Down Expand Up @@ -53,25 +51,13 @@ class InputInteractionViewTestActivity :
@Inject
lateinit var textInputViewModelFactory: TextInputViewModel.FactoryImpl

@Inject
lateinit var ratioViewModelFactory: RatioExpressionInputInteractionViewModel.FactoryImpl

@Inject
lateinit var mathExpViewModelFactoryFactory: MathExpViewModelFactoryFactoryImpl

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

val textInputViewModel by lazy { textInputViewModelFactory.create<TextInputViewModel>() }

val ratioExpressionInputInteractionViewModel by lazy {
ratioViewModelFactory.create<RatioExpressionInputInteractionViewModel>(
interaction = Interaction.newBuilder().putCustomizationArgs(
"numberOfTerms",
SchemaObject.newBuilder().setSignedInt(3).build()
).build()
)
}

lateinit var mathExpressionViewModel: MathExpressionInteractionsViewModel
lateinit var writtenTranslationContext: WrittenTranslationContext

Expand Down Expand Up @@ -118,14 +104,11 @@ class InputInteractionViewTestActivity :

binding.numericInputViewModel = numericInputViewModel
binding.textInputViewModel = textInputViewModel
binding.ratioInteractionInputViewModel = ratioExpressionInputInteractionViewModel
binding.mathExpressionInteractionsViewModel = mathExpressionViewModel
}

fun getPendingAnswerErrorOnSubmitClick(v: View) {
numericInputViewModel.checkPendingAnswerError(AnswerErrorCategory.SUBMIT_TIME)
ratioExpressionInputInteractionViewModel
.checkPendingAnswerError(AnswerErrorCategory.SUBMIT_TIME)
}

override fun onPendingAnswerErrorOrAvailabilityCheck(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
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.RatioInputInteractionView
import org.oppia.android.app.model.InputInteractionViewTestActivityParams
import org.oppia.android.app.model.Interaction
import org.oppia.android.app.model.SchemaObject
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.RatioExpressionInputInteractionViewModel
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.ActivityRatioInputInteractionViewTestBinding
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 [RatioInputInteractionView].
*/
class RatioInputInteractionViewTestActivity :
InjectableAutoLocalizedAppCompatActivity(),
StateKeyboardButtonListener,
InteractionAnswerErrorOrAvailabilityCheckReceiver,
InteractionAnswerReceiver {
private lateinit var binding: ActivityRatioInputInteractionViewTestBinding

@Inject
lateinit var ratioViewModelFactory: RatioExpressionInputInteractionViewModel.FactoryImpl

/**
* Gives access to the [RatioExpressionInputInteractionViewModel].
*/
val ratioExpressionInputInteractionViewModel by lazy {
ratioViewModelFactory.create<RatioExpressionInputInteractionViewModel>(
interaction = Interaction.newBuilder().putCustomizationArgs(
"numberOfTerms",
SchemaObject.newBuilder().setSignedInt(3).build()
).build()
)
}

/**
* 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<ActivityRatioInputInteractionViewTestBinding>(
this, R.layout.activity_ratio_input_interaction_view_test
)

val params =
intent.getProtoExtra(
TEST_ACTIVITY_PARAMS_ARGUMENT_KEY,
InputInteractionViewTestActivityParams.getDefaultInstance()
)
writtenTranslationContext = params.writtenTranslationContext

binding.ratioInteractionInputViewModel = ratioExpressionInputInteractionViewModel
}

/**
* Checks for submit time errors.
*/
fun getPendingAnswerErrorOnSubmitClick(v: View) {
ratioExpressionInputInteractionViewModel
.checkPendingAnswerError(AnswerErrorCategory.SUBMIT_TIME)
}

override fun onAnswerReadyForSubmission(answer: UserAnswer) { }

override fun onEditorAction(actionCode: Int) { }

private inline fun <reified T : StateItemViewModel> InteractionItemFactory.create(
interaction: Interaction = Interaction.getDefaultInstance()
): T {
return create(
entityId = "fake_entity_id",
hasConversationView = false,
interaction = interaction,
interactionAnswerReceiver = this@RatioInputInteractionViewTestActivity,
answerErrorReceiver = this@RatioInputInteractionViewTestActivity,
hasPreviousButton = false,
isSplitView = false,
writtenTranslationContext,
timeToStartNoticeAnimationMs = null
) as T
}

companion object {
private const val TEST_ACTIVITY_PARAMS_ARGUMENT_KEY =
"RatioInputInteractionViewTestActivity.params"

/**
* Creates an intent to open [RatioInputInteractionViewTestActivity].
*/
fun createIntent(
context: Context,
extras: InputInteractionViewTestActivityParams
): Intent {
return Intent(context, RatioInputInteractionViewTestActivity::class.java).also {
it.putProtoExtra(TEST_ACTIVITY_PARAMS_ARGUMENT_KEY, extras)
}
}
}
}
41 changes: 0 additions & 41 deletions app/src/main/res/layout/activity_input_interaction_view_test.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@
name="textInputViewModel"
type="org.oppia.android.app.player.state.itemviewmodel.TextInputViewModel" />

<variable
name="ratioInteractionInputViewModel"
type="org.oppia.android.app.player.state.itemviewmodel.RatioExpressionInputInteractionViewModel" />

<variable
name="mathExpressionInteractionsViewModel"
type="org.oppia.android.app.player.state.itemviewmodel.MathExpressionInteractionsViewModel" />
Expand All @@ -36,43 +32,6 @@
android:orientation="vertical"
tools:context=".testing.InputInteractionViewTestActivity">

<org.oppia.android.app.customview.interaction.RatioInputInteractionView
android:id="@+id/test_ratio_input_interaction_view"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_margin="8dp"
android:background="@drawable/edit_text_background"
android:fontFamily="sans-serif"
android:hint="@{ratioInteractionInputViewModel.hintText}"
android:imeOptions="actionDone"
android:longClickable="false"
android:maxLength="200"
android:minHeight="48dp"
android:paddingStart="16dp"
android:paddingEnd="16dp"
android:paddingBottom="8dp"
android:singleLine="true"
android:text="@={ratioInteractionInputViewModel.answerText}"
android:textColor="@color/component_color_shared_primary_text_color"
android:textColorHint="@color/component_color_shared_edit_text_hint_color"
android:textSize="14sp"
android:textStyle="italic"
app:textChangedListener="@{ratioInteractionInputViewModel.answerTextWatcher}" />

<TextView
android:id="@+id/ratio_input_error"
style="@style/TextViewStart"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginStart="16dp"
android:layout_marginEnd="8dp"
android:fontFamily="sans-serif"
android:minHeight="32dp"
android:text="@{ratioInteractionInputViewModel.errorMessage}"
android:textColor="@color/component_color_shared_input_error_color"
android:textSize="12sp"
android:visibility="@{ratioInteractionInputViewModel.errorMessage.length() > 0 ? View.VISIBLE : View.INVISIBLE}" />

<org.oppia.android.app.customview.interaction.NumericInputInteractionView
android:id="@+id/test_number_input_interaction_view"
android:layout_width="match_parent"
Expand Down
Loading

0 comments on commit bb69b22

Please sign in to comment.