-
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 #3905: A11y flow fix for ProfileProgressHeader #3931
Conversation
Hi @rt4914. Apologies, will need to look at this tomorrow. |
Sorry, I'll need to review this on Monday. |
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 @rt4914! This seems sensible--is there any way to add tests for the changes, though? Also, any idea on the test failure; is it related to this PR?
@BenHenning I have tried adding test cases and I am unable to think of any other ways we can test it but at least this strategy makes sure if something changes the test case fails. All tests are passing now. |
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 @rt4914. I think the new tests are a bit change-detectory, but they're probably the best we can do since there isn't a way to simulate a11y flows in Robolectric so far as I know. This will for sure ensure that we don't regress the actual fix, but won't necessarily prevent other failures occurring in the future. We'll need to rely on manual testing for those.
Merging since everything seems addressed now. Thanks @rt4914! |
Explanation
Fix #3905: A11y flow fix for ProfileProgressHeader
Earlier the flow for profile progress was really bad. This PR tries to improve that experience.
Issue
incorrect_flow.mp4
New Flow
device-2021-10-14-233854.mp4
Espresso Tests are passing
Essential Checklist
For UI-specific PRs only
If your PR includes UI-related changes, then: