Fix part #376: Fraction input interaction view validation (#419)

* nit * UI hi-fi for text,number,and fraction input views * UI hi-fi for text,number,and fraction input views * UI hi-fi for text,number,and fraction input views nit * nit * nit * test cases update * accent color * input type in fraction input type * input type in fraction input type * Merge branches 'develop' and 'hi-fi-input-interaction-views' of https://github.com/oppia/oppia-android into hi-fi-input-interaction-views # Conflicts: # app/src/main/res/layout/text_input_interaction_item.xml * text color in input type views * changed inputtype in edit text * margin updated in input views * nit * keyboardhelper to handle softinoutkeyboard * Edit text focus removed. On click of input type interaction item, it requires two clicks to display keyboard, which should actually be just a single click. thus preventing scroll due to edit text focus * as per review suggestion added binding.stateRecyclerView.smoothScrollToPosition(0) in processEphemeralStateResult * nit * Fix-406 * nit changes and keybord helper class renamed. * nit * kdoc for keyboardhelper.nit changes * kdoc for keyboardhelper * nit * nit * nit * nit * nit * validation in fraction input * nit * nit * nit * errorcode enum * errorcode enum * nit * nit * nit * nit * error text on Fraction input * error text on Fraction input * nit * nit * updated FractionParsingErrors Enum with string resources, added getPendingAnswerError in InteractionAnswerHandler,in error text of fraction set minimum height 32dp and text size 12sp ,color code updated,and othere nit changes * nit * nit * nit * Merge conflict issue fix Merge branches 'develop' and 'hi-fi-input-interaction-views-validation' of https://github.com/oppia/oppia-android into hi-fi-input-interaction-views-validation # Conflicts: # app/src/main/java/org/oppia/app/parser/StringToFractionParser.kt # app/src/main/java/org/oppia/app/player/state/itemviewmodel/ContinueInteractionViewModel.kt # app/src/main/java/org/oppia/app/player/state/itemviewmodel/FractionInteractionViewModel.kt # app/src/main/java/org/oppia/app/player/state/itemviewmodel/InteractionViewModelModule.kt # app/src/main/java/org/oppia/app/player/state/itemviewmodel/NumericInputViewModel.kt # app/src/main/java/org/oppia/app/player/state/itemviewmodel/TextInputViewModel.kt # app/src/main/res/values/strings.xml * not showing error on fiest - symbol in fraction input, new method for submit button click * not showing error on fiest - symbol in fraction input, new method for submit button click * nit * partial answer removed error message display by regex for partial values * nit import optimisation changes reverted * nit import optimisation changes reverted * nit import optimisation changes reverted * carsh fix in EditTextBindingAdapters * on submit button displays error for partial input and divided by 0 * instance check * moved FractionParsingError to StringToFractionParser, In InteractionAnswerHandler added isExplicitErrorCheckRequired, and updated existing with getPendingAnswerErrorOnSubmit and other changes required for the above. * color names updated casing * Introduced disparity between the patterns used to validate vs the ones we use to parse. Suggested by ben. * test cases for error messages. * nit * partial mixed fraction issue fix * nit * nit * nit * added few testcases * nit changes. * parseFunction introduced and the helper is used for both for parsing and validation * getter and setter for PendingAnswerError in FractionInteractionViewModel, removed valid string resource and returns string "valid" inside enum, setPendingAnswerError,hasPendingAnswerError flags in InteractionAnswerHandler, and other nit * removed digit filter in test activity nit * removed digit filter in test activity nit * added new method onAnswerRealTimeError in InteractionAnswerHandler * added new method onAnswerRealTimeError in InteractionAnswerHandler * state fragment is added as parameter to FractionInteractionViewModel * submit button active/inactive on realtime error implementation on StateFragmentPresenter * nit changes * nit changes, InputInteractionViewTestActivity updated with new realtime error implemenatations and submit button enable and disable. * submit button issue fix. * interactionVieewModelModule code fix for on continue button click for multiplechoice issue/crash. * nit * setting error on submit is moved to stateFragmentPresenter * nit * InteractionAnswerErrorReceiver * 3rd approach * fraction input validation final approach * nit * nit * nit * nit * New fraction submit time error "None of the numbers of the fraction should be larger than 7 digits." * test-case to check long number, nit changes ,kDoc * nit
oppia · Jan 22, 2020 · 14ed668 · 14ed668
1 parent e3e5040
commit 14ed668
Show file tree

Hide file tree

Showing 25 changed files with 430 additions and 64 deletions.
diff --git a/app/src/main/java/org/oppia/app/customview/interaction/FractionInputInteractionView.kt b/app/src/main/java/org/oppia/app/customview/interaction/FractionInputInteractionView.kt
@@ -26,12 +26,12 @@ class FractionInputInteractionView @JvmOverloads constructor(
   attrs: AttributeSet? = null,
   defStyle: Int = android.R.attr.editTextStyle
 ) : EditText(context, attrs, defStyle), View.OnFocusChangeListener {
-  private val hintText: String
+  private val hintText: CharSequence
   private val stateKeyboardButtonListener: StateKeyboardButtonListener
 
   init {
     onFocusChangeListener = this
-    hintText = (hint ?: "").toString()
+    hintText = (hint ?: "")
     stateKeyboardButtonListener = context as StateKeyboardButtonListener
   }
 

diff --git a/app/src/main/java/org/oppia/app/customview/interaction/NumericInputInteractionView.kt b/app/src/main/java/org/oppia/app/customview/interaction/NumericInputInteractionView.kt
@@ -27,16 +27,20 @@ class NumericInputInteractionView @JvmOverloads constructor(
   defStyle: Int = android.R.attr.editTextStyle
 ) : EditText(context, attrs, defStyle), View.OnFocusChangeListener {
   private val stateKeyboardButtonListener: StateKeyboardButtonListener
+  private val hintText: CharSequence
 
   init {
     onFocusChangeListener = this
+    hintText = (hint ?: "")
     stateKeyboardButtonListener = context as StateKeyboardButtonListener
   }
 
   override fun onFocusChange(v: View, hasFocus: Boolean) = if (hasFocus) {
+    hint = ""
     typeface = Typeface.DEFAULT
     showSoftKeyboard(v, context)
   } else {
+    hint = hintText
     if (text.isEmpty()) setTypeface(typeface, Typeface.ITALIC)
     hideSoftKeyboard(v, context)
   }

diff --git a/app/src/main/java/org/oppia/app/customview/interaction/TextInputInteractionView.kt b/app/src/main/java/org/oppia/app/customview/interaction/TextInputInteractionView.kt
@@ -23,12 +23,12 @@ class TextInputInteractionView @JvmOverloads constructor(
   attrs: AttributeSet? = null,
   defStyle: Int = android.R.attr.editTextStyle
 ) : EditText(context, attrs, defStyle), View.OnFocusChangeListener {
-  private val hintText: String
+  private val hintText: CharSequence
   private val stateKeyboardButtonListener: StateKeyboardButtonListener
 
   init {
     onFocusChangeListener = this
-    hintText = (hint ?: "").toString()
+    hintText = (hint ?: "")
     stateKeyboardButtonListener = context as StateKeyboardButtonListener
   }
 

diff --git a/app/src/main/java/org/oppia/app/databinding/EditTextBindingAdapters.kt b/app/src/main/java/org/oppia/app/databinding/EditTextBindingAdapters.kt
@@ -0,0 +1,11 @@
+package org.oppia.app.databinding
+
+import android.text.TextWatcher
+import android.widget.EditText
+import androidx.databinding.BindingAdapter
+
+/** Binding adapter for setting a [TextWatcher] as a change listener for an [EditText]. */
+@BindingAdapter("app:textChangedListener")
+fun bindTextWatcher(editText: EditText, textWatcher: TextWatcher) {
+  editText.addTextChangedListener(textWatcher)
+}
diff --git a/app/src/main/java/org/oppia/app/parser/StringToFractionParser.kt b/app/src/main/java/org/oppia/app/parser/StringToFractionParser.kt
@@ -1,5 +1,9 @@
 package org.oppia.app.parser
 
+import android.content.Context
+import androidx.annotation.StringRes
+import org.oppia.app.R
+import org.oppia.app.customview.interaction.FractionInputInteractionView
 import org.oppia.app.model.Fraction
 import org.oppia.domain.util.normalizeWhitespace
 
@@ -8,14 +12,59 @@ class StringToFractionParser {
   private val wholeNumberOnlyRegex = """^-? ?(\d+)$""".toRegex()
   private val fractionOnlyRegex = """^-? ?(\d+) ?/ ?(\d+)$""".toRegex()
   private val mixedNumberRegex = """^-? ?(\d+) (\d+) ?/ ?(\d+)$""".toRegex()
+  private val invalidCharsRegex = """^[\d\s/-]+$""".toRegex()
+  private val invalidCharsLengthRegex = "\\d{8,}".toRegex()
 
-  fun getFractionFromString(text: String): Fraction {
+  /**
+   * Returns a [FractionParsingError] for the specified text input if it's an invalid fraction, or
+   * [FractionParsingError.VALID] if no issues are found. Note that a valid fraction returned by this method is guaranteed
+   * to be parsed correctly by [parseRegularFraction].
+   *
+   * This method should only be used when a user tries submitting an answer. Real-time error detection should be done
+   * using [getRealTimeAnswerError], instead.
+   */
+  fun getSubmitTimeError(text: String): FractionParsingError {
+    if (invalidCharsLengthRegex.find(text) != null)
+      return FractionParsingError.NUMBER_TOO_LONG
+    val fraction = parseFraction(text)
+    return when {
+      fraction == null -> FractionParsingError.INVALID_FORMAT
+      fraction.denominator == 0 -> FractionParsingError.DIVISION_BY_ZERO
+      else -> FractionParsingError.VALID
+    }
+  }
+
+  /**
+   * Returns a [FractionParsingError] for obvious incorrect fraction formatting issues for the specified raw text, or
+   * [FractionParsingError.VALID] if not such issues are found.
+   *
+   * Note that this method returning a valid result does not guarantee the text is a valid fraction--
+   * [getSubmitTimeError] should be used for that, instead. This method is meant to be used as a quick sanity check for
+   * general validity, not for definite correctness.
+   */
+  fun getRealTimeAnswerError(text: String): FractionParsingError {
+    val normalized = text.normalizeWhitespace()
+    return when {
+      !normalized.matches(invalidCharsRegex) -> FractionParsingError.INVALID_CHARS
+      normalized.startsWith("/") -> FractionParsingError.INVALID_FORMAT
+      normalized.count { it == '/' } > 1 -> FractionParsingError.INVALID_FORMAT
+      normalized.lastIndexOf('-') > 0 -> FractionParsingError.INVALID_FORMAT
+      else -> FractionParsingError.VALID
+    }
+  }
+
+  /** Returns a [Fraction] parse from the specified raw text string. */
+  fun parseFraction(text: String): Fraction? {
     // Normalize whitespace to ensure that answer follows a simpler subset of possible patterns.
     val inputText: String = text.normalizeWhitespace()
     return parseMixedNumber(inputText)
-      ?: parseFraction(inputText)
+      ?: parseRegularFraction(inputText)
       ?: parseWholeNumber(inputText)
-      ?: throw IllegalArgumentException("Incorrectly formatted fraction: $text")
+  }
+
+  /** Returns a [Fraction] parse from the specified raw text string. */
+  fun parseFractionFromString(text: String): Fraction {
+    return parseFraction(text) ?: throw IllegalArgumentException("Incorrectly formatted fraction: $text")
   }
 
   private fun parseMixedNumber(inputText: String): Fraction? {
@@ -29,7 +78,7 @@ class StringToFractionParser {
       .build()
   }
 
-  private fun parseFraction(inputText: String): Fraction? {
+  private fun parseRegularFraction(inputText: String): Fraction? {
     val fractionOnlyMatch = fractionOnlyRegex.matchEntire(inputText) ?: return null
     val (_, numeratorText, denominatorText) = fractionOnlyMatch.groupValues
     // Fraction-only numbers imply no whole number.
@@ -53,4 +102,18 @@ class StringToFractionParser {
   }
 
   private fun isInputNegative(inputText: String): Boolean = inputText.startsWith("-")
+
+  /** Enum to store the errors of [FractionInputInteractionView]. */
+  enum class FractionParsingError(@StringRes private var error: Int?) {
+    VALID(error = null),
+    INVALID_CHARS(error = R.string.fraction_error_invalid_chars),
+    INVALID_FORMAT(error = R.string.fraction_error_invalid_format),
+    DIVISION_BY_ZERO(error = R.string.fraction_error_divide_by_zero),
+    NUMBER_TOO_LONG(error = R.string.fraction_error_larger_than_seven_digits);
+
+    /** Returns the string corresponding to this error's string resources, or null if there is none. */
+    fun getErrorMessageFromStringRes(context: Context): String? {
+      return error?.let(context::getString)
+    }
+  }
 }
diff --git a/app/src/main/java/org/oppia/app/player/state/StateFragment.kt b/app/src/main/java/org/oppia/app/player/state/StateFragment.kt
@@ -7,11 +7,14 @@ import android.view.View
 import android.view.ViewGroup
 import org.oppia.app.fragment.InjectableFragment
 import org.oppia.app.model.UserAnswer
+import org.oppia.app.player.state.answerhandling.InteractionAnswerErrorReceiver
+import org.oppia.app.player.state.answerhandling.InteractionAnswerHandler
 import org.oppia.app.player.state.answerhandling.InteractionAnswerReceiver
 import javax.inject.Inject
 
 /** Fragment that represents the current state of an exploration. */
-class StateFragment : InjectableFragment(), InteractionAnswerReceiver {
+class StateFragment : InjectableFragment(), InteractionAnswerReceiver, InteractionAnswerHandler,
+  InteractionAnswerErrorReceiver {
   companion object {
     /**
      * Creates a new instance of a StateFragment.
@@ -46,6 +49,10 @@ class StateFragment : InjectableFragment(), InteractionAnswerReceiver {
 
   fun handleKeyboardAction() = stateFragmentPresenter.handleKeyboardAction()
 
+  override fun onPendingAnswerError(pendingAnswerError: String?) {
+    stateFragmentPresenter.updateSubmitButton(pendingAnswerError)
+  }
+
   fun setAudioBarVisibility(visibility: Boolean) = stateFragmentPresenter.setAudioBarVisibility(visibility)
 
   fun scrollToTop() = stateFragmentPresenter.scrollToTop()

diff --git a/app/src/main/java/org/oppia/app/player/state/StateFragmentPresenter.kt b/app/src/main/java/org/oppia/app/player/state/StateFragmentPresenter.kt
@@ -38,10 +38,11 @@ import org.oppia.app.model.EphemeralState
 import org.oppia.app.model.Interaction
 import org.oppia.app.model.State
 import org.oppia.app.model.SubtitledHtml
-import org.oppia.app.player.audio.AudioButtonListener
 import org.oppia.app.model.UserAnswer
+import org.oppia.app.player.audio.AudioButtonListener
 import org.oppia.app.player.audio.AudioFragment
 import org.oppia.app.player.audio.AudioUiManager
+import org.oppia.app.player.state.answerhandling.InteractionAnswerErrorReceiver
 import org.oppia.app.player.state.answerhandling.InteractionAnswerReceiver
 import org.oppia.app.player.state.itemviewmodel.ContentViewModel
 import org.oppia.app.player.state.itemviewmodel.ContinueInteractionViewModel
@@ -100,7 +101,7 @@ class StateFragmentPresenter @Inject constructor(
   /**
    * A list of view models corresponding to past view models that are hidden by default. These are intentionally not
    * retained upon configuration changes since the user can just re-expand the list. Note that the first element of this
-   * list (when initialized), will always be the previous answers header to help locate the items in the recycler view
+   * list (when initialized), will always be the previous answer's header to help locate the items in the recycler view
    * (when present).
    */
   private val previousAnswerViewModels: MutableList<StateItemViewModel> = mutableListOf()
@@ -109,6 +110,7 @@ class StateFragmentPresenter @Inject constructor(
    * configuration changes since the user can just re-expand the list.
    */
   private var hasPreviousResponsesExpanded: Boolean = false
+  private lateinit var stateNavigationButtonViewModel: StateNavigationButtonViewModel
 
   fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? {
     explorationId = fragment.arguments!!.getString(STATE_FRAGMENT_EXPLORATION_ID_ARGUMENT_KEY)!!
@@ -399,7 +401,7 @@ class StateFragmentPresenter @Inject constructor(
     Handler().postDelayed({
       binding.congratulationTextview.clearAnimation()
       binding.congratulationTextview.visibility = View.INVISIBLE
-    },2000)
+    }, 2000)
   }
 
   /** Helper for subscribeToAnswerOutcome. */
@@ -440,7 +442,8 @@ class StateFragmentPresenter @Inject constructor(
 
   fun handleKeyboardAction() {
     hideKeyboard()
-    handleSubmitAnswer(viewModel.getPendingAnswer())
+    if (stateNavigationButtonViewModel.isInteractionButtonActive.get()!!)
+      handleSubmitAnswer(viewModel.getPendingAnswer())
   }
 
   override fun onContinueButtonClicked() {
@@ -473,7 +476,7 @@ class StateFragmentPresenter @Inject constructor(
   ) {
     val interactionViewModelFactory = interactionViewModelFactoryMap.getValue(interaction.id)
     pendingItemList += interactionViewModelFactory(
-      explorationId, interaction, fragment as InteractionAnswerReceiver
+      explorationId, interaction, fragment as InteractionAnswerReceiver, fragment as InteractionAnswerErrorReceiver
     )
   }
 
@@ -559,7 +562,7 @@ class StateFragmentPresenter @Inject constructor(
     hasGeneralContinueButton: Boolean,
     stateIsTerminal: Boolean
   ) {
-    val stateNavigationButtonViewModel =
+    stateNavigationButtonViewModel =
       StateNavigationButtonViewModel(context, this as StateNavigationButtonListener)
     stateNavigationButtonViewModel.updatePreviousButton(isEnabled = hasPreviousState)
 
@@ -611,4 +614,13 @@ class StateFragmentPresenter @Inject constructor(
   }
 
   private fun isAudioShowing(): Boolean = viewModel.isAudioBarVisible.get()!!
+
+  /** Updates submit button UI as active if pendingAnswerError null else inactive. */
+  fun updateSubmitButton(pendingAnswerError: String?) {
+    if (pendingAnswerError != null) {
+      stateNavigationButtonViewModel.isInteractionButtonActive.set(false)
+    } else {
+      stateNavigationButtonViewModel.isInteractionButtonActive.set(true)
+    }
+  }
 }
diff --git a/app/src/main/java/org/oppia/app/player/state/StateViewModel.kt b/app/src/main/java/org/oppia/app/player/state/StateViewModel.kt
@@ -5,6 +5,7 @@ import androidx.databinding.ObservableList
 import androidx.lifecycle.ViewModel
 import org.oppia.app.fragment.FragmentScope
 import org.oppia.app.model.UserAnswer
+import org.oppia.app.player.state.answerhandling.AnswerErrorCategory
 import org.oppia.app.player.state.answerhandling.InteractionAnswerHandler
 import org.oppia.app.player.state.itemviewmodel.StateItemViewModel
 import org.oppia.app.viewmodel.ObservableArrayList
@@ -34,7 +35,10 @@ class StateViewModel @Inject constructor() : ObservableViewModel() {
 
   // TODO(#164): Add a hasPendingAnswer() that binds to the enabled state of the Submit button.
   fun getPendingAnswer(): UserAnswer {
-    return getPendingAnswerHandler(itemList)?.getPendingAnswer() ?: UserAnswer.getDefaultInstance()
+    return if (getPendingAnswerHandler(itemList)?.checkPendingAnswerError(AnswerErrorCategory.SUBMIT_TIME) != null) {
+      UserAnswer.getDefaultInstance()
+    } else
+      getPendingAnswerHandler(itemList)?.getPendingAnswer() ?: UserAnswer.getDefaultInstance()
   }
 
   private fun getPendingAnswerHandler(itemList: List<StateItemViewModel>): InteractionAnswerHandler? {

diff --git a/...src/main/java/org/oppia/app/player/state/answerhandling/InteractionAnswerErrorReceiver.kt b/...src/main/java/org/oppia/app/player/state/answerhandling/InteractionAnswerErrorReceiver.kt
@@ -0,0 +1,15 @@
+package org.oppia.app.player.state.answerhandling
+
+/**
+ * A handler for interaction answer's error receiving to update submit button.
+ * Handlers can either require an additional user action before the submit button UI can be updated.
+ */
+interface InteractionAnswerErrorReceiver {
+
+  /**
+   * Called when an error was detected upon answer submission. Implementations are recommended to prevent further answer
+   * submission until the pending answer itself changes. The interaction is responsible for displaying the error provided
+   * here, not the implementation.
+   */
+  fun onPendingAnswerError(pendingAnswerError: String?) {}
+}
diff --git a/app/src/main/java/org/oppia/app/player/state/answerhandling/InteractionAnswerHandler.kt b/app/src/main/java/org/oppia/app/player/state/answerhandling/InteractionAnswerHandler.kt
@@ -14,8 +14,15 @@ interface InteractionAnswerHandler {
    */
   fun isExplicitAnswerSubmissionRequired(): Boolean = true
 
+  /** Return the current answer's error messages  if not valid else return null. */
+  fun checkPendingAnswerError(category: AnswerErrorCategory): String? {
+    return null
+  }
+
   /** Return the current answer that is ready for handling. */
-  fun getPendingAnswer(): UserAnswer
+  fun getPendingAnswer(): UserAnswer? {
+    return null
+  }
 }
 
 /**
@@ -25,3 +32,11 @@ interface InteractionAnswerHandler {
 interface InteractionAnswerReceiver {
   fun onAnswerReadyForSubmission(answer: UserAnswer)
 }
+
+/** Categories of errors that can be inferred from a pending answer.  */
+enum class AnswerErrorCategory {
+  /** Corresponds to errors that may be found while the user is trying to input an answer.  */
+  REAL_TIME,
+  /** Corresponds to errors that may be found only when a user tries to submit an answer.  */
+  SUBMIT_TIME
+}