-
Notifications
You must be signed in to change notification settings - Fork 521
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 #4322: Stops Talkback from pronouncing asterisks. #4601
Conversation
@BenHenning, this duplicate causes breaks. But its present in the origin/develop. As seen here https://github.com/oppia/oppia-android/blob/develop/app/src/main/res/values/color_defs.xml |
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.
Hi @Ryggs. I took at the PR and left one follow-up comment.
Regarding the failure, it should go away if you sync to the latest develop. The PR that caused it has been reverted.
Sending this back to you @Ryggs per the comment above, and also converting it to be a draft PR since it's still a work-in-progress. |
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 @Ryggs. Left a few follow-up comments, but this seems close to done. PTAL.
app/src/sharedTest/java/org/oppia/android/app/profile/AddProfileActivityTest.kt
Outdated
Show resolved
Hide resolved
testing/src/main/java/org/oppia/android/testing/espresso/TextInputAction.kt
Outdated
Show resolved
Hide resolved
app/src/sharedTest/java/org/oppia/android/app/profile/AddProfileActivityTest.kt
Show resolved
Hide resolved
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 @Ryggs. Just a couple of follow-ups--PTAL.
Assigning @rt4914 since I think he can take a look at this now. |
I will review it today. |
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.
Nice work @Ryggs Thanks.
Unassigning @rt4914 since they have already approved the PR. |
Assigning @BenHenning for code owner reviews. Thanks! |
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 @Ryggs. This LGTM!
Explanation
Fixes #4322
This PR stops talkback from pronouncing the word asterisks and instead pronounces the word required.
It introduces the usage of the helperText attribute and the removal of all occurrences of the *, in string files. The fields affected in the add profile activity layout file are name, pin, and confirm pin.
Essential Checklist
For UI-specific PRs only
If your PR includes UI-related changes, then:
Demo
ORIGINAL WITHOUT TALKBACK
dev-no-talkback.mp4
ORIGINAL WITH TALKBACK
dev-with-talkback.webm
FIXED PR ISSUE WITHOUT TALKBACK
after-change-non-talkback.mp4
FIXED PR ISSUE WITH TALKBACK
after-change-talkback.mp4
Tests