Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #4135, Fix part of #5070: In FractionInteraction UI, leave submit button enabled when answer is empty. #5224

Merged
merged 28 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
52e1261
In FractionInteraction UI, leave submit button enabled when answer is…
masclot Oct 17, 2023
c89659d
Fix error message
masclot Oct 17, 2023
0ec1bc7
Address PR comments
masclot Nov 13, 2023
44a88c7
Address PR comments
masclot Nov 13, 2023
21e1d91
Address PR comments
masclot Nov 13, 2023
d37362b
Add new error message when the text to be parsed for a fraction is em…
masclot Nov 13, 2023
984d943
Add new error message when the text to be parsed for a fraction is em…
masclot Nov 13, 2023
cb15252
Fix tests for FactionParser.
masclot Nov 13, 2023
8c2c1dd
Fix kdoc format.
masclot Nov 13, 2023
f7d2dda
Fix tests in FractionParsingUiErrorTest
masclot Nov 13, 2023
e8c4331
Update kdoc as per review.
masclot Nov 15, 2023
1c044a1
Merge remote-tracking branch 'upstream/develop' into enable_submit_on…
masclot Nov 21, 2023
dbd8e46
Create a separate test activity and test for fraction input interaction
masclot Nov 21, 2023
2ab3fbf
Add missing class to previous commit
masclot Nov 21, 2023
eb54fb5
Fix formatting
masclot Nov 21, 2023
13a110f
Fix test
masclot Nov 21, 2023
482bc70
Merge remote-tracking branch 'upstream/develop' into enable_submit_on…
masclot Nov 27, 2023
0b228c0
Fix bug in which setting the text of a MathExpression was not always …
masclot Nov 27, 2023
674d631
Fix KDoc errors
masclot Nov 27, 2023
fe54f1d
Add accessibility exemption for new test activity
masclot Nov 27, 2023
8b3a92e
Merge branch 'develop' into enable_submit_on_empty_answer
adhiamboperes Dec 5, 2023
a8da03c
Improve test naming as requested in review
masclot Dec 5, 2023
b7e4bbf
Merge remote-tracking branch 'upstream/develop' into enable_submit_on…
masclot Dec 5, 2023
3f225ab
Merge remote-tracking branch 'origin/enable_submit_on_empty_answer' i…
masclot Dec 5, 2023
b294ccb
Merge remote-tracking branch 'upstream/develop' into enable_submit_on…
masclot Dec 17, 2023
51015e2
Apply PR review changes.
masclot Dec 17, 2023
ea10dc7
Merge remote-tracking branch 'upstream/develop' into enable_submit_on…
masclot Dec 29, 2023
768ebfd
Merge remote-tracking branch 'upstream/develop' into enable_submit_on…
masclot Jan 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
adhiamboperes marked this conversation as resolved.
Show resolved Hide resolved
}
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()
}
BenHenning marked this conversation as resolved.
Show resolved Hide resolved

/**
* 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
Loading