I.L. FE Mobile: Initial Focus management between screens

laflannery commented 1 year ago

Description

Possibly related to this other focus management issue

I couldn't test the focus on mobile until the updates got to staging but on my phone the focus is acting a bit odd:

The first time I go through the app (nothing has been filled in, I haven't gone through any of the screens yet), the focus is going to the continue button each time I move to the next question.
Once I get to the Review page, the behavior becomes correct and I'm hearing the H1 as I should
Then when I start to interact with the Edit flow, I continue to hear the H1 for each screen properly, even when using the Back/Continue buttons.

User story

AS A Veteran using a screen reader on my phone I WANT the H1 to be announced when navigating between screens SO THAT I can understand the page/question I am on within the Income Limits flow for the first time.

Engineering notes / background

I tested the example form I had put in the other related ticket just to confirm what behavior this had and it was not acting like Income limits. First time going through the form, I was hearing the H2s which is expected.

Analytics considerations

Quality / testing notes

I was testing on an iPhone with Safari and VoiceOver

Acceptance criteria

[x] When using a mobile device, the H1 of the page is announced when navigating between screens within the Income Limits flow for the first time.

randimays commented 1 year ago

Not able to reproduce this on my Android, so I reached out to @laflannery to see if we had any cross-platform testing tools so I can try on an iPhone. She said we do not currently have a solution for this (Slack thread for context) but she's going to look into it tomorrow. Worst case, we may just need someone with access to an iPhone to work on this.

This is likely a simple fix, but with the unknown of not being able to reproduce, I padded it at 5.

dsasser commented 1 year ago

I tested this on my iPhone 13 with voiceover enabled and the problem isn't presenting for me. Testing steps:

Using Safari, visit https://staging.va.gov/health-care/income-limits-temp (which redirects to health-care/income-limits-temp/year)
Enable VoiceOver with Siri
Siri starts to announce the page contents: "web page loaded, flag"
Enter the year in the dropdown and click the 'continue' button
Siri announces the h1 "Donec id elit vitae sapien finibus sagittis...heading level 1"
Hit 'back' button, Siri announces the h1
Hitting continue, Siri announces the h1

laflannery commented 1 year ago

This is where I'm at:

I was able to test this on another iPhone and on my iPhone in both Safari and Chrome and am getting the same results (when initially going through the app the focus is not behaving as expected)
I have tested other forms and the focus does behave correctly, the same way it does on desktop
I've been looking at the code for both Income Limits and other forms trying to see how this was done and I don't think it's done the same way which perhaps is something we could look into
- Income limits is using useEffect but I can't find that being used anywhere in the other forms. The forms I'm looking at are in vets-website here
- I have found a platform utilities folder for the forms library that speaks to what we are trying to do I think, however I don't think I see those other forms utilizing this folder, I can't see it being imported so I don't think this is how they are managing focus
- I also can't figure out how those other forms are managing focus, I can't find any mention of focus anywhere in the files
- I tried to see if there are any known issues with useEffect and iOS but nothing stood out to me but if the other forms are doing this a different way maybe we should try what they are doing?

Does this help in any way? Would we be able to look at how the other forms are managing focus to see if it's done differently and if so can we try and emulate what they are doing and if that resolves the issue?

randimays commented 1 year ago

@laflannery Thank you for the update. The code examples from the other forms shouldn't be different in functionality (we are both using the focusElement function provided by platform), but maybe this is a very specific iOS quirk with useEffect. I'd prefer to keep our method of code because it's newer/cleaner syntax than the other forms, but if we switch over and it works, it's fine by me.

randimays commented 1 year ago

Some progress notes:

I don't have an iPhone and wasn't able to reproduce this issue on my phone. I asked @dsasser to try to reproduce on his iPhone and he wasn't able to either. Laura is still seeing the issue on the same version of iPhone on Safari and Chrome so it's not immediately clear what exactly the problem is.

I don't want to blindly make changes until we hit the right solution, but it might be the only way for now. Laura can't view review instances or Tugboat on her phone as they both require SOCKS. Daniel suggested we use ngrok.io to create a testable environment for an iPhone that I could send to Laura to test (again, this would be blindly iterating).

Alternatively, I'm looking for a way to test this on an iPhone device locally. I tried Xcode / Device Simulator and that did not pan out because it does not support accessibility features. They do have an XCode Accessibility Inspector (separate app) but that doesn't support focus features.

I have a request out to Platform for a seat on the VA BrowserStack account. They haven't responded yet. More to come.

jilladams commented 1 year ago

Talked to Randi today:

Browserstack has iOS screenreader feature in dev, no timeline
xCode does screenreader but doesn’t have a flow of reading – doesn’t navigate the DOM, so cannot help verify fix for this issue when navigating between pages.

So right now we don't have a way to repro in order to resolve. Randi will ask Platform re: any other recommendations on tools.

@laflannery while we try to figure this out, can you clarify if this issue is launch blocking?

jilladams commented 1 year ago

I also have an iPhone 13, testing too to see if I can repro:

🟢 Enable VoiceOver with Siri 🟢 Using Safari, visit https://staging.va.gov/health-care/income-limits (which redirects to health-care/income-limits/year) -- siri reads the H1 🟢 Enter the year in the dropdown and click the 'continue' button 🔴 Siri announces the h1 "Donec id elit vitae sapien finibus sagittis...heading level 1" -- nope

FOr me, Siri says "Contact us, collapsed" on the Zip code screen. Not sure why. 🟢 Hit 'back' button, Siri announces the h1 🔴 Hitting continue, Siri announces the h1 -- nope
Siri says "Contact us, collapsed"

🔴 The first time I go through the app (nothing has been filled in, I haven't gone through any of the screens yet), the focus is going to the continue button each time I move to the next question.

For me, H1 is getting announced on each new screen

🟢 Once I get to the Review page, the behavior becomes correct and I'm hearing the H1 as I should 🔴 Then when I start to interact with the Edit flow, I continue to hear the H1 for each screen properly, even when using the Back/Continue buttons.

For me, once I click edit, I start to hear either "Contact Us" collapsed, or "Feedback" depending on the screen.

I am remembering an issue Bryan Paronto ran into at some point where the order of the DOM with JS loading after was affecting what was available on page for the screen reader to say first. I can't recall exactly what the issue was or how we fixed it. Maybe it'll come back to me overnight.

randimays commented 1 year ago

@jilladams I think maybe you got a cached version of the app. It likely won't make a difference in your test; just making a note here. An introduction screen has been added, so navigating to https://staging.va.gov/health-care/income-limits should not redirect you to /year automatically anymore.

laflannery commented 1 year ago

Adding a couple comments here that I also put in slack from my recent testing with Randi:

The focus is still not quite working right however, looking at Jill's comment (above) and looking at this more like a possible loading issue maybe this makes sense? One of the times I tested the current flow, I got a situation where the H1 for the zip screen was read and then suddenly VoiceOver was interrupted with "Webpage loaded" and the reader started reading the top of the page again with "skip to content" and the other elements that are at the very top in the header.
Another instance on the intro page where I was sort of "stuck" when trying to swipe to reach the 2 links for the flows and couldn't get to them - it was almost like the screen reader didn't think anything was on the screen that it was able to go to. So maybe the app wasn't loaded or something before I started trying to interact with it? (edited)

@jilladams focus management is classified as a defect-1 and because of the confusion that this is causing on mobile (because it seems to be throwing focus to odd places) this would be considered launch-blocking

randimays commented 1 year ago

@jilladams You may have been referring to this discussion w/ Bryan Paronto? I read through it just in case; not sure it's an applicable solution. But continuing to research today to see what I can find.

jilladams commented 1 year ago

Talked to DaveC about this in mid-sprint checkin. Notes:

Is this unique to our app? Could we try to repro on another React app using the sub-task pattern within a bigger form flow to see if they have similar load / h1 read issues?
- We change the H1 on each screen. Other forms>sub-tasks keep a consistent H1 and change the H2 on every screen. We should consider whether this is a pattern that would fix for us to unblock launch.
Re: being launch blocking, need to stay in comms with DaveC on this in the event that we can't figure out a consistent repro in order to drive a fix.

laflannery commented 1 year ago

@jilladams I can't find another place where I can test this - I have looked at other forms using a version of the subtask pattern but not completely like we are. These forms do not have unique H1s, instead they have unique h2s and the focus was working properly on desktop and mobile. Do you know of an example that is using the full subtask pattern with unique H1s like we are that I can test?

I mentioned I was looking at some other forms in vet-website in this previous comment and I just checked another one of these again just to be sure I didn't miss anything or nothing changed and they are still behaving properly on my phone

jilladams commented 1 year ago

We don't - and it may not exist. :/

@randimays @wesrowe Added a scrum topic for tomorrow to talk about this. If we add H2s, that's content we'll need - not sure how it sounds to y'all, but maybe it's worth throwing in a Lorem h2 just to prove out whether that approach fixes our issue?

laflannery commented 1 year ago

A possible custom property I found and mentioned in slack that maybe will help?

randimays commented 1 year ago

@jilladams I got an answer from Platform about other testing tools: https://dsva.slack.com/archives/CBU0KDSB1/p1687903716287529?thread_ts=1687813567.819449&cid=CBU0KDSB1

They said they have used SauceLabs for cross-platform testing but there's no access to real devices there; it just runs the simulator on a macOS box which will be the same result as Xcode.

I'm going to look into @laflannery's suggestion when I get back to this ticket and see where that takes me.

randimays commented 1 year ago

Some more updates:

I tried adding an <h2> to each page of the flow to see if that helped with focus. Laura tested and said the behavior was actually worse than before
I tried adding a manual check to see if the <h1> actually exists before trying to focus on it. Laura tested and said the behavior is the same as before
I posted a thread in [the design system channel(https://dsva.slack.com/archives/C01DBGX4P45/p1688072497765459) to see if anyone had insights. They referred me to #accessibility-help.
Laura and I were actively testing together so she posted in #accessibility-help here. We got a couple suggestions but the one that ended up working for us was swapping out our use of focusElement with waitForRenderThenFocus. It's not clear to me why other apps that use focusElement do not have this problem, but it solves the issue for now.

More context: I recently discovered that the subtask pattern actually has a code implementation in Platform. I thought "subtask" was just referring to a design pattern / guidance. I have a ticket open for the current sprint to start translating everything over to the right code implementation. This might fix the focus problem so we can revert back to focusElement, but it might be worth it just to keep the current solution so we don't have to go back and forth again.

Pull request for the final fix is here: https://github.com/department-of-veterans-affairs/vets-website/pull/24723

Thank you @laflannery for working with me to win the battle, and @jilladams for your testing help and insights!!

jilladams commented 1 year ago

NICE WORK.

randimays commented 1 year ago

I tested Chrome, Safari and Firefox on desktop, and Chrome on Android once the code got into staging. @laflannery also tested on iOS and desktop. https://dsva.slack.com/archives/C52CL1PKQ/p1688398524200229 (thread for confirmation).

This is ready to close!

wesrowe commented 1 year ago

🎉 Big win!! I think I saw that you circulated this info back to the DST, that's a solid team play.

department-of-veterans-affairs / va.gov-cms