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 10 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
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
BenHenning marked this conversation as resolved.
Show resolved Hide resolved
)
}
}
errorMessage.addOnPropertyChangedCallback(callback)
isAnswerAvailable.addOnPropertyChangedCallback(callback)
// Apply defaults:
BenHenning marked this conversation as resolved.
Show resolved Hide resolved
errorOrAvailabilityCheckReceiver.onPendingAnswerErrorOrAvailabilityCheck(
null,
true
BenHenning marked this conversation as resolved.
Show resolved Hide resolved
)
}

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
1 change: 1 addition & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@
<string name="fraction_error_invalid_format">Please enter a valid fraction (e.g., 5/3 or 1 2/3)</string>
<string name="fraction_error_divide_by_zero">Please do not put 0 in the denominator</string>
<string name="fraction_error_larger_than_seven_digits">None of the numbers in the fraction should have more than 7 digits.</string>
<string name="fraction_error_empty_input">Enter a fraction to continue</string>
BenHenning marked this conversation as resolved.
Show resolved Hide resolved
<string name="number_error_starting_with_floating_point">Please begin your answer with a number (e.g.,”0” in 0.5).</string>
<string name="number_error_invalid_format">Please enter a valid number.</string>
<string name="number_error_larger_than_fifteen_characters">The answer can contain at most 15 digits (0–9) or symbols (. or -).</string>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ class FractionParsingUiErrorTest {
.toUiError()
.getErrorMessageFromStringRes(activity.appLanguageResourceHandler)
assertThat(errorMessage)
.isEqualTo("Please enter a valid fraction (e.g., 5/3 or 1 2/3)")
.isEqualTo("Enter a fraction to continue")
adhiamboperes marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.oppia.android.util.math

import android.text.TextUtils
import org.oppia.android.app.model.Fraction
import org.oppia.android.util.extensions.normalizeWhitespace

Expand All @@ -24,6 +25,9 @@ class FractionParser {
* detection should be done using [getRealTimeAnswerError], instead.
*/
fun getSubmitTimeError(text: String): FractionParsingError {
if (TextUtils.isEmpty(text)) {
BenHenning marked this conversation as resolved.
Show resolved Hide resolved
return FractionParsingError.EMPTY_INPUT
}
if (invalidCharsLengthRegex.find(text) != null) {
return FractionParsingError.NUMBER_TOO_LONG
}
Expand Down Expand Up @@ -130,6 +134,9 @@ class FractionParser {
* Indicates that at least one of the numbers present in the string is too long to be
* precisely represented in a fraction.
*/
NUMBER_TOO_LONG
NUMBER_TOO_LONG,

/** The input text was empty. */
adhiamboperes marked this conversation as resolved.
Show resolved Hide resolved
EMPTY_INPUT
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ class FractionParserTest {
}

@Test
fun testSubmitTimeError_emptyString_returnsInvalidFormat() {
fun testSubmitTimeError_emptyString_returnsEmptyInput() {
val error = fractionParser.getSubmitTimeError("")
assertThat(error).isEqualTo(FractionParser.FractionParsingError.INVALID_FORMAT)
assertThat(error).isEqualTo(FractionParser.FractionParsingError.EMPTY_INPUT)
}

@Test
Expand Down
Loading