oppia / oppia-android

A free, online & offline learning platform to make quality education accessible for all.
https://www.oppia.org
Apache License 2.0
300 stars 503 forks source link

Improve flow in onboarding screen #4655

Open vrajdesai78 opened 1 year ago

vrajdesai78 commented 1 year ago

Describe the bug When we click next button in onboarding screen, focus will remain in next button instead of announcing current screen.

To Reproduce Steps to reproduce the behavior:

  1. Clear app's data if Oppia app is already installed
  2. Turn on Talkback.
  3. Click on 'Next Button'.

Expected behavior When we click next button, talkback reader should read screen name.

Demonstration

https://user-images.githubusercontent.com/43074241/195674699-2c58a48a-f04f-47ef-8acc-ca3476da5c57.mp4

Environment

iampradeep-hr commented 1 year ago

Hi Can I work on this issue ?

BenHenning commented 1 year ago

This is actually a really challenging problem @iampradeep-hr as it's an issue with view pager, not Oppia code. I suggest looking at other open issues, instead.

iampradeep-hr commented 1 year ago

@BenHenning I have worked with ViewPager2 and I have some experience. I had commented on all the issues which i guess i able to solve.

BenHenning commented 1 year ago

@iampradeep-hr before I assign this to you, I think it’d be helpful to get an idea on how you’d approach it. Could you provide your thoughts on how this issue might be fixed?

iampradeep-hr commented 1 year ago

Ok Thanks, @BenHenning So I think we need to send AccessibilityEvents manually whenever the page is scrolled. The focus is not going to the next screen viewPager instead staying on next button itself. So in the onPageScrolled() callback I need to add the code which sends AccessiblityEvents focus changes to the system. This would solve it i guess.

BenHenning commented 1 year ago

That might work @iampradeep-hr but it'll depend on which events you send. We have to be careful to not re-route the user at a time the view pager itself might be trying to change the flow, and we don't want to interrupt anything being read out at that moment. If you think your solution can address these points as well, I'm happy to assign the issue over to you.

iampradeep-hr commented 1 year ago

@BenHenning

https://user-images.githubusercontent.com/81664507/202836668-6614dc9e-263c-4506-970a-3409f5199e8e.mp4

Yes, if we send only focus change events on the ViewPager then it would read only the ACTION_ACCESSIBILITY_FOCUS changes and it dosen't redirect / re-route the user anywhere. So here on pressing the next button the focus changes to ViewPager and announces the page number. once the user taps on the current page it announces the views.

BenHenning commented 1 year ago

Interesting! This looks like a really good UX. It may be worth checking how that flow behaves on a different screenreader (like Samsung's), but otherwise I think this is a really promising start, thanks @iampradeep-hr! Assigning this over to you.

Edit: Also, I suggest enabling "speech output" in the developer settings of Talkback as this can make it easier to see what Talkback is actually trying to verbalize (and works better when showing a11y flows as then people don't need to listen to the video).

iampradeep-hr commented 1 year ago

Hi @BenHenning, Is this the way it should work ?

https://user-images.githubusercontent.com/81664507/202853732-4ed5a364-0541-4977-bbed-78ec5bd92f8c.mp4

BenHenning commented 1 year ago

@iampradeep-hr I think that looks really good! That's exactly how I'd expect it to work, yep.

iampradeep-hr commented 1 year ago

@iampradeep-hr I think that looks really good! That's exactly how I'd expect it to work, yep.

Cool then, I would update the code changes in all the related layout files and create a PR.

iampradeep-hr commented 1 year ago

@BenHenning , here's the PR onboarding flow improved

BenHenning commented 1 year ago

Thanks @iampradeep-hr will take a look.

adhiamboperes commented 1 year ago

Unassigning @iampradeep-hr due to inactivity.