Skip to content

Commit

Permalink
Fix #4135, Fix part of #5070: In FractionInteraction UI, leave submit…
Browse files Browse the repository at this point in the history
… button enabled when answer is empty. (#5224)

Fix part of #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
oppia/oppia#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)](oppia/design-team#71 (comment))

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
<!-- 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
masclot and adhiamboperes authored Jan 9, 2024
1 parent 082c1b6 commit 6723268
Show file tree
Hide file tree
Showing 17 changed files with 847 additions and 465 deletions.
1 change: 1 addition & 0 deletions app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@
<activity android:name=".app.testing.DrawableBindingAdaptersTestActivity" />
<activity android:name=".app.testing.ExplorationInjectionActivity" />
<activity android:name=".app.testing.ExplorationTestActivity" />
<activity android:name=".app.testing.FractionInputInteractionViewTestActivity" />
<activity
android:name=".app.testing.TestFontScaleConfigurationUtilActivity"
android:theme="@style/OppiaThemeWithoutActionBar" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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<FractionInteractionViewModel>()
}

/** 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<ActivityFractionInputInteractionViewTestBinding>(
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 <reified T : StateItemViewModel> 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)
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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(),
Expand All @@ -55,9 +53,6 @@ class InputInteractionViewTestActivity :
@Inject
lateinit var textInputViewModelFactory: TextInputViewModel.FactoryImpl

@Inject
lateinit var fractionInteractionViewModelFactory: FractionInteractionViewModel.FactoryImpl

@Inject
lateinit var ratioViewModelFactory: RatioExpressionInputInteractionViewModel.FactoryImpl

Expand All @@ -68,10 +63,6 @@ class InputInteractionViewTestActivity :

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

val fractionInteractionViewModel by lazy {
fractionInteractionViewModelFactory.create<FractionInteractionViewModel>()
}

val ratioExpressionInputInteractionViewModel by lazy {
ratioViewModelFactory.create<RatioExpressionInputInteractionViewModel>(
interaction = Interaction.newBuilder().putCustomizationArgs(
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?xml version="1.0" encoding="utf-8"?>
<layout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools">

<data>

<import type="android.view.View" />

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

<ScrollView
android:layout_width="match_parent"
android:layout_height="match_parent">
<LinearLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:descendantFocusability="beforeDescendants"
android:focusableInTouchMode="true"
android:gravity="center"
android:orientation="vertical"
tools:context=".testing.FractionInputInteractionViewTestActivity">

<org.oppia.android.app.customview.interaction.FractionInputInteractionView
android:id="@+id/test_fraction_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="@{fractionInteractionViewModel.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="@={fractionInteractionViewModel.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="@{fractionInteractionViewModel.answerTextWatcher}" />

<TextView
android:id="@+id/fraction_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="@{fractionInteractionViewModel.errorMessage}"
android:textColor="@color/component_color_shared_input_error_color"
android:textSize="12sp"
android:visibility="@{fractionInteractionViewModel.errorMessage.length() > 0 ? View.VISIBLE : View.INVISIBLE}" />

<Button
android:id="@+id/submit_button"
style="@style/StateButtonActive"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_margin="8dp"
android:onClick="getPendingAnswerErrorOnSubmitClick"
android:text="@string/submit_button_text"
android:textColor="@color/component_color_shared_secondary_4_text_color" />
</LinearLayout>
</ScrollView>
</layout>
Loading

0 comments on commit 6723268

Please sign in to comment.