-
Notifications
You must be signed in to change notification settings - Fork 527
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 part of #5070: Display empty answer message in ratio input interaction #5263
Fix part of #5070: Display empty answer message in ratio input interaction #5263
Conversation
@theMr17 is this ready for a full pass or are you looking for a preliminary review on approach? Please convert to ready and assign to me if the former. |
@adhiamboperes, please do a preliminary review of the approach. I will proceed to write tests after that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@theMr17, I have left some suggestions, but the overall approach looks good to me.
...org/oppia/android/app/player/state/itemviewmodel/RatioExpressionInputInteractionViewModel.kt
Outdated
Show resolved
Hide resolved
...org/oppia/android/app/player/state/itemviewmodel/RatioExpressionInputInteractionViewModel.kt
Outdated
Show resolved
Hide resolved
@adhiamboperes PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @theMr17! This LGTM.
Thanks for moving the ratio tests to their own suite as well.
Unassigning @adhiamboperes since they have already approved the PR. |
Assigning @BenHenning for code owner reviews. Thanks! |
Hi @theMr17, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. |
Hi @theMr17, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue. |
@BenHenning, this PR is blocked on codeowner, PTAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @theMr17! Really sorry about the late review here. I just had two small comments, otherwise the PR LGTM.
...c/sharedTest/java/org/oppia/android/app/testing/RatioInputInteractionViewTestActivityTest.kt
Outdated
Show resolved
Hide resolved
...c/sharedTest/java/org/oppia/android/app/testing/RatioInputInteractionViewTestActivityTest.kt
Outdated
Show resolved
Hide resolved
@BenHenning PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @theMr17! Please make sure to respond to addressed comments on future PRs. Otherwise the PR LGTM.
...c/sharedTest/java/org/oppia/android/app/testing/RatioInputInteractionViewTestActivityTest.kt
Outdated
Show resolved
Hide resolved
...c/sharedTest/java/org/oppia/android/app/testing/RatioInputInteractionViewTestActivityTest.kt
Outdated
Show resolved
Hide resolved
Thanks @BenHenning, I will surely keep that in mind. |
Explanation
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.Screen.Recording.mp4
Essential Checklist
For UI-specific PRs only
If your PR includes UI-related changes, then: