search

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

Public resources for building on and in support of VA.gov. Visit complete Knowledge Hub:
https://depo-platform-documentation.scrollhelp.site/index.html
282 stars 203 forks source link

sitewide 508-defect-2 [COGNITION, KEYBOARD]: Focus MUST not be hidden by the sticky profile menu #19779

Closed joshkimux closed 3 years ago

joshkimux commented 3 years ago

508-defect-2

Feedback framework

Definition of done

  1. Review and acknowledge feedback.
  2. Fix and/or document decisions made.
  3. Accessibility specialist will close ticket after reviewing documented decisions / validating fix.

Point of Contact

VFS Point of Contact: Josh

User Story or Problem Statement

As a keyboard user using magnification or on a smaller viewport, I always want to be able to see and track focus so I can navigate and interact with the page.

Details

At smaller viewports or when magnified, the profile menu will become sticky on the page. This results in it occasionally covering focusable elements. Keyboard users who rely on visible focus to interact with the page will struggle to do so as shown in research done by the gov.uk team.

Keyboard users often navigate web pages using the TAB key to move through focusable elements, such as links and input boxes. The SHIFT + TAB key combination can be used to move through those elements backwards. We found that where the sticky header overlapped the page, navigating backwards through elements often left the focused element obscured by the sticky element.

This was an accessibility barrier for users. The sticky element was preventing a commonly used navigation technique from working properly, leaving users unable to see or read the element they had focused on.

Does this mean that sticky functionality should never be implemented? No, but it must be considered and tested carefully, particularly if the sticky element in any way overlaps other content. A safer use would be a sticky element positioned over an empty part of the page, or to have the content scroll in a way that it is never overlapped by the sticky element.

I believe this also exists on public websites, so I have marked this as a sitewide ticket. @caw310, @1Copenut , & @rtwell , this may be worth your review. This issue is a sibling to 19784.

Acceptance Criteria

Environment

Steps to Recreate

  1. Enter https://staging.va.gov/profile in browser
  2. Zoom by 400%
  3. Tab all the way through the page
  4. Shift tab all the way back up the page
  5. Confirm focus is hidden by the sticky profile menu

Proposed Solution

Stickiness is OK provided it does not cover focusable elements. If a design solution can be found where the menu does not overlap with focusable items on the page, then the ticket can be closed.

Otherwise, it may be simpler to remove stickiness and allow it to remain relatively positioned at the top. To provide an equivalent experience to stickiness, designers could consider:

WCAG or Vendor Guidance (optional)

Screenshots or Trace Logs

https://user-images.githubusercontent.com/14154792/107564618-a44f2b00-6bb0-11eb-8b7d-dc926ff98d17.mov

https://user-images.githubusercontent.com/14154792/107564656-b16c1a00-6bb0-11eb-91da-f13527a58ac2.mov

Screen Shot 2021-02-10 at 1 49 44 PM Screen Shot 2021-02-10 at 3 28 42 PM
mshea0606 commented 3 years ago

@sporeboy @erikphansen this is a ticket that will require a conversation with the platform team

mshea0606 commented 3 years ago

@Samara-Strauss This sticky menu is currently used anywhere there is a secondary nav on static content.

Samara-Strauss commented 3 years ago

Got it. I've got this in our sprint plan to review in a couple sprints when we have more design bandwidth.

Samara-Strauss commented 3 years ago

@andaleliz I'm adding this to next sprint in case you have time to work on it. It's not clear to me whether this has impact outside of the profile, so that will be something to look into. If this affects menus outside of the profile, we might have to bring in the site wide team.

erikphansen commented 3 years ago

It does affect the mobile nav trigger on the entire site. But it's potentially worse on Profile when the mobile nav is expanded.

andaleliz commented 3 years ago

This also happens anywhere the In This Section menu is used on content pages - e.g. on the education How to Apply page (screenshot below). I think we need to bring in the site wide team.

When tabbing down through the page, the page jumps as needed so the focused element is in the viewport. I don't notice this issue of the sticky element covering the focused element when tabbing _down. But, this doesn't always happen going back up, which is where I see the focused element being hidden.

@erikphansen, thoughts on programmatically setting the distance of the focused element from the top of the viewport so that it'll always clear the height of the sticky menu? Probably an overly simplistic solution...

image

erikphansen commented 3 years ago

@andaleliz that was my initial thought as well. I think we'd have to add listeners on every single focusable-object on the page so that when they got focus we could scroll up a bit if needed. Which would work but also feels icky.

It might also be possible to essentially crop the main page content to a box that starts just below the sticky nav trigger. Which sounds harder to accomplish well but would make this problem go away.

We should definitely raise this to as many FE devs as possible. Someone might have an elegant solution for this. I'll also be on the lookout for solutions to this in the wild.

joshkimux commented 3 years ago

It would be awesome if we can find a non-hacky solution that doesn't require js to function, so I'd recommend we bring in design/content folks as well.

Is a sticky menu really necessary, or could we explore shortening the amount of content on pages? Do users really need fast access to the menu at all times, or could it just remain at the top and bottom of the pages?

We should examine other issues of stickiness as well beyond just compliance too. For zoomed in screens and mobile, a sticky menu takes up a significant portion of the screen. For users with low vision (think older Veterans on a mobile phone with large font), valuable information may be hidden by the sticky menu or they may be severely limited in terms of how much they can see at any given time.

We may want to consider more research on mobile in multiple, realistic contexts.

Edit: that additional floating feedback label isn't helping either 😬

@rtwell this thread might interest you!

Samara-Strauss commented 3 years ago

@joshkimux — @andaleliz is a designer, so she's one of the right people to have on this!

Also, if this ends up being more of a larger, site wide issue, I suggest we bring in the site wide (public websites) team. Our team is really only responsible with how this functions in the profile, so we shouldn't be handling any larger questions around site content or cross-site design elements on our own.

I don't remember at this point what the reason was that we decided to make this a sticky menu instead of a menu that just hangs at the top of the page. @tressaellen can you provide more context here? Hoping your memory is better than mine.

erikphansen commented 3 years ago

I'm certainly open to being convinced otherwise, but I am firmly in the Pro Sticky Nav camp. But obviously it introduces issues. I hope we can figure out something that works for everyone. I don't have a major problem with solving this with JS. The JS it would take to solve it would likely have a tiny, tiny impact on performance and page load compared to React and some of the insane stuff that's happening in the form system. And that all works just fine even on old mobile phones. Also, FWIW, the menu button only gets "sticky" because of JS, so fixing the rough edges with JS doesn't seem objectionable.

andaleliz commented 3 years ago

@erikphansen yes I was a pretty hacky dev so makes perfect sense that my brain went there 😆 Definitely don't want to do something ugly/hacky/non-performant. Good call to raise to other devs - what is the best way to do that? I'm not sure where all this is being used on the site, other than on the content pages.

Is our profile menu the same component as the in this section menu? Either way, this definitely has larger, site wide implications. Something I don't think our team is responsible for solving alone, at all.

@joshkimux

that additional floating feedback label isn't helping either

That should go away once we shift from ForeSee to Medilla. I thought that was slated for earlier this year but must've been pushed back some. It's on the way out though, afaik.

Do users really need fast access to the menu at all times, or could it just remain at the top and bottom of the pages?

I haven't brushed up on the research around these design decisions, but I'm sure Tressa will chime in. If so, using the back to top component instead is a possible non-hacky solution.

joshkimux commented 3 years ago

@Samara-Strauss As discussed with the team and Ryan, we've decided to remove stickiness from this page! @erikphansen will be replicating the menu on Pittsburgh Health Care.

Samara-Strauss commented 3 years ago

I love when stuff gets solved and I didn't even need to be part of this conversations 😂

@erikphansen @tressaellen @andaleliz you all good with this? You may have been part of the original convo, but just making sure in case you weren't.

erikphansen commented 3 years ago

@joshkimux just wanted to confirm that I'll use this ticket to fix the Profile nav. Fixing the global nav is different code, but also a whole lot easier. Probably one point of work to just disable the stickiness.

erikphansen commented 3 years ago

I can remove @andaleliz from this ticket and make this the FE implementation ticket.

joshkimux commented 3 years ago

@erikphansen Yes, let's go ahead and do that! Thank you

andaleliz commented 3 years ago

@brianalloyd pinging you for awareness since the hub pages that include In this section sticky menus have a similar issue

sshein commented 3 years ago

hi all I'm a little late to this, but as I recall, Ryan designed this and didn't really have a strong opinion on whether the menu was sticky or not. Then Erik asked me "hey, should this be sticky?" and I was like "I don't know let's ask Ryan" and Ryan was like "sure." So it wasn't like a very strongly thought through decision. That said, if we're making it not sticky on profile then yes it shouldn't be sticky on any of the benefits content pages so let's make sure someone makes that update.

joshkimux commented 3 years ago

@sshein No worries! All the folks you mentioned were on the call when we made the decision to unsticky it again 🥳

erikphansen commented 3 years ago

To be clear, the offending button has always been sticky. Pittsburgh was the trailblazer in having a non-sticky mobile nav menu trigger.

Fun/sad fact: I did some searching to see how other major sites dealt with this sticky-nav-hiding-focus issue. I got very excited to see that Deque's site uses a sticky nav! But sadly they punted on solving this exact same issue :(

andaleliz commented 3 years ago

I think one of @sshein's concerns is that this gets fixed everywhere, not just in the profile. We talked about our team being the one to test to see how it performs as not sticky, but then I think we decided we didn't need to since this is already on the VAMC pages. Giving it more thought, I don't think this was actually tested. Posted a slack message to verify.

Update: as far as Ryan knows this has not been validated with users.

Do we need to do some usability testing with this before it's changed globally? I personally would feel better validating that the back to top works for people and we're not introducing new pain points by making them scroll back up to get to the profile nav.

If we don't think testing is necessary - shall we create a separate ticket for the public websites team? I'm not sure the best way to make sure this gets done everywhere.

joshkimux commented 3 years ago

@andaleliz Gotcha, that makes sense! If we were to test, we'd need to make sure that we're recruiting our participants strategically e.g. specifically requesting Veterans that view this on mobile or who are primarily keyboard users. Otherwise, we may unintentionally map desktop/ablelist findings to all Veterans.

If we don't think testing is necessary, I think a separate ticket would be the right way to go!

andaleliz commented 3 years ago

As far as anyone knows this hasn't been tested with users, so I think it would be a good idea to get some feedback on this change before we recommend a sitewide change. I don't know that we need a dedicated study just for this change though. Couple of options:

  1. see what studies other teams are working on that require people to log in and see if they can add a task/questions around this. Are we doing more testing w/ MyVA and does it make sense there?
  2. wait until we do some testing for facility changes to profile and work it in there. Timeline is dependent on VAOS team though.
Samara-Strauss commented 3 years ago

As far as anyone knows this hasn't been tested with users, so I think it would be a good idea to get some feedback on this change before we recommend a sitewide change.

Does this change anything with the plan to un-sticky the nav in the profile before making larger site wide changes? I have un-sticking the nav down as a task for Erik for next sprint, so if we want to hold off and do some testing, then I can move that accordingly.

erikphansen commented 3 years ago
  1. This ticket only pertains to the Profile. The site-wide nav is totally different code
  2. I can probably un-stick the Profile menu behind a feature flag so that we can test this with users in Prod without actually releasing the change to all users.
Samara-Strauss commented 3 years ago

This ticket only pertains to the Profile. The site-wide nav is totally different code

@joshkimux just making sure that if this ticket is meant for the profile only, you have the larger site wide issue tracked in another issue. I don't want to close this out and then lose the discussion on the sticky nav site-wide.

I can probably un-stick the Profile menu behind a feature flag so that we can test this with users in Prod without actually releasing the change to all users.

Sounds good. I can keep it on for next sprint, then, and we can always do additional testing if we determine that's needed.

andaleliz commented 3 years ago

I'll need someone w/ more tenure on the team to verify this, but I believe all that's been influenced by testing in the current implementation is the visual design. When it was all blue w/ white text, no one saw it, so we changed it to what we have today because that's what was in place with VAMC.

If that's true, and we aren't un-doing something that came out of testing by removing the stickiness, I think we can make the changes in our profile and not change our plans. Not-sticky is equally valid as sticky at that point.

However, I think:

I know it's not technically the auth exp team's responsibility to validate sitewide changes, but we're in a position to help here given that we're making the change first.

Also, just saw Samara's comment:

@joshkimux just making sure that if this ticket is meant for the profile only, you have the larger site wide issue tracked in another issue. I don't want to close this out and then lose the discussion on the sticky nav site-wide.

I was not able to find any other tickets discussing this issue.

Samara-Strauss commented 3 years ago

If that's true, and we aren't un-doing something that came out of testing by removing the stickiness, I think we can make the changes in our profile and not change our plans. Not-sticky is equally valid as sticky at that point.

Sounds good to me. TBH, I am not sure what testing has or has not been done on this element.

In terms of moving things forward, here is what I am understanding as of now:

  1. We can have @erikphansen remove stickiness from this element in the profile next sprint. It sounds like no additional testing needs to be done for the profile specifically, but let me know if that is not correct.
  2. @joshkimux should create an issue to track this update site wide. Our team is not responsible for site wide updates.
  3. Some level of testing should be done to verify this change site wide, but that is not the responsibility of the auth exp team. However, if we did end up doing testing on the profile because we had time to do so, those conclusions could be applied to site wide changes.

Am I following along accurately at this point? Anything I missed?

andaleliz commented 3 years ago

@Samara-Strauss that's accurate. I do think it's important that this change gets made sitewide if it proves to be a good one, but I don't feel good making recommendations changes across the site that have not been validated.

It sounds like the facilities team has some research coming up in Q2 looking at navigation on the VAMC pages, so they may be able to have this looked at in their study before we can get to it. I will also mention this as a research opportunity to the new public websites team researcher.

Samara-Strauss commented 3 years ago

It sounds like the facilities team has some research coming up in Q2 looking at navigation on the VAMC pages, so they may be able to have this looked at in their study before we can get to it. I will also mention this as a research opportunity to the new public websites team researcher.

This sounds great! Thanks, Liz!

erikphansen commented 3 years ago

@andaleliz @Samara-Strauss @ajakabcin I was about to push up a PR for this change, and then realized I said I was gonna put this behind a feature flag so that we could test it with users before pushing it out to all users in prod. Did you still want to hide this change behind a feature flag so that we can control the rollout?

Samara-Strauss commented 3 years ago

I thought we said we didn't need to do additional testing on this for the profile specifically, though whoever is managing larger sitewide changes should consider testing. Is that accurate, @andaleliz?

andaleliz commented 3 years ago

Yes, that's mostly accurate - we said we would release w/out testing, since this is how it works on VAMC and neither VAMC OR the sticky profile were ever tested with people (so we're not explicitly going against any data). I think we have to do this ASAP anyway, as the defect-2 issues are supposed to be resolved within a month and we're overdue on this.

I'm hopeful we can add a research question to validate this in our next study (likely May), which would probably happen before the sitewide team gets to it.

Samara-Strauss commented 3 years ago

@erikphansen sounds like you don't need to launch behind a feature flags, but thanks for checking in on that!

erikphansen commented 3 years ago

Merged! The un-sticky Profile menu will be on staging later today for validation by... @ajakabcin I guess? @joshkimux might also want to take a took.

Samara-Strauss commented 3 years ago

Yep! And @andaleliz can also validate.

andaleliz commented 3 years ago

Took a look at this and confirmed that the profile menu is no longer hiding focus 🎉

I think we're good to go, but have a couple of other thoughts to share:

joshkimux commented 3 years ago

@andaleliz You beat me to it! Closing this ticket now as it's been resolved 🥳 Thank you @erikphansen !!! 🎆

Samara-Strauss commented 3 years ago

Something that I totally neglected earlier (or perhaps it has changed) is that I don't see the back to top component in profile. We factored that into our decision to go this direction since it would still provide users easy access to the profile menu; I assumed that was already part of profile and never consciously double checked that. If we need to manually add this to profile, we can explore that separately. I don't think the fact that this is missing should block us from rolling out this update. We have never validated that users struggled to get to the top of a page or that the stickiness was helpful in the first place.

Agree on all accounts. @andaleliz do you want to add exploring this to the running list of things you want to assess in profile research at some point?

andaleliz commented 3 years ago

@Samara-Strauss it's already in the doc, but phrased in a way that assumed the back to top button is already there. I'll clarify. Good idea to update it!

Samara-Strauss commented 3 years ago

it's already in the doc

Great! Tbh, I totally did not check the doc before commenting. Glad to hear it's already there.