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 202 forks source link

[508-defect-2] Tablist on compensation claims are links styled as a tablist without keyboard functionality #41449

Closed joshkimux closed 1 year ago

joshkimux commented 2 years ago

508-defect-2 [KEYBOARD, COGNITION] Tablist on compensation claims are links styled as a tablist without keyboard functionality

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

Details

Styling links as tabs is deceptive – when a link is clicked, the content is shown after a page refresh with the focus moving back to the top of the viewport.

But tabs don’t work like this – when a tab is clicked, the content is shown instantly without a page refresh and with the focus remaining on the tab.

Keyboard users may expect to be able to use the left and right arrow keys in order to switch tabs. This wouldn’t be expected with a list of links.

And (sighted) screen reader users may expect certain functionality, like using a shortcut to move focus to the associated tab panel or announcing which tab is selected.

Tabs should only look like tabs if they behave like tabs otherwise it can be in disorienting and confusing for users.

Avoiding tab styles for navigation, Adam Silver (gov.uk)

Current experience

https://user-images.githubusercontent.com/14154792/168847941-dda1f857-a82c-4b14-9433-02e5108b97bb.mov

Left and right arrow keys navigates characters , not tabs

Expected experience

https://user-images.githubusercontent.com/14154792/168848450-bf222124-4ccf-47ea-b242-516218640877.mov

Left and right arrow keys navigate between tabs OR these are written and styled as links.

Testing

Edge / Windows (Keyboard only)

JAWs / Edge / Windows

Chrome / Windows (Keyboard only)

NVDA / Chrome / Windows

Next steps

Steps to Recreate

  1. Enter https://staging.va.gov/track-claims/your-claims/600219085/files in browser
  2. Start preferred screen reading device
  3. Log in as user 233
  4. Navigate to the tab list
  5. Confirm right and left arrows do not navigate tab to tab

Tab List

Reference MDN doc to add keyboard functionality

window.addEventListener('DOMContentLoaded', () => {
  const tabs = document.querySelectorAll('[role="tab"]');
  const tabList = document.querySelector('[role="tablist"]');

  // Add a click event handler to each tab
  tabs.forEach(tab => {
    tab.addEventListener('click', changeTabs);
  });

  // Enable arrow navigation between tabs in the tab list
  let tabFocus = 0;

  tabList.addEventListener('keydown', e => {
    // Move right
    if (e.keyCode === 39 || e.keyCode === 37) {
      tabs[tabFocus].setAttribute('tabindex', -1);
      if (e.keyCode === 39) {
        tabFocus++;
        // If we're at the end, go to the start
        if (tabFocus >= tabs.length) {
          tabFocus = 0;
        }
        // Move left
      } else if (e.keyCode === 37) {
        tabFocus--;
        // If we're at the start, move to the end
        if (tabFocus < 0) {
          tabFocus = tabs.length - 1;
        }
      }

      tabs[tabFocus].setAttribute('tabindex', 0);
      tabs[tabFocus].focus();
    }
  });
});

function changeTabs(e) {
  const target = e.target;
  const parent = target.parentNode;
  const grandparent = parent.parentNode;

  // Remove all current selected tabs
  parent
    .querySelectorAll('[aria-selected="true"]')
    .forEach(t => t.setAttribute('aria-selected', false));

  // Set this tab as selected
  target.setAttribute('aria-selected', true);

  // Hide all tab panels
  grandparent
    .querySelectorAll('[role="tabpanel"]')
    .forEach(p => p.setAttribute('hidden', true));

  // Show the selected panel
  grandparent.parentNode
    .querySelector(`#${target.getAttribute('aria-controls')}`)
    .removeAttribute('hidden');
}

Link List

Visual design

Before

After

Code

Before


<ul class="va-tabs claims-status-tabs" role="tablist">
  <li role="presentation">
    <a id="tabStatus" aria-controls="tabPanelStatus" aria-selected="true" role="tab" class="va-tab-trigger va-tab-trigger--current" href="/track-claims/your-claims/600307936/status">Status</a>
  </li>
  <li role="presentation">
    <a id="tabFiles" aria-selected="false" role="tab" class="va-tab-trigger" href="/track-claims/your-claims/600307936/files">Files</a>
  </li>
  <li role="presentation">
    <a id="tabDetails" aria-selected="false" role="tab" class="va-tab-trigger" href="/track-claims/your-claims/600307936/details">Details</a>
  </li>
</ul>
<div role="tabpanel" id="tabPanelStatus" aria-labelledby="tabStatus">
  <div class="va-tab-content claim-tab-content">
    <div>
      <h3 class="vads-u-visibility--screen-reader">Claim status</h3>
...

After

<ul>
  <li><a href="/track-claims/your-claims/600307936/status">Status</a></li>
  <li><a href="/track-claims/your-claims/600307936/files">Files</a></li>
  <li><a href="/track-claims/your-claims/600307936/details">Details</a></li>
</ul>
<h1>Claim status</h1>
...

Interaction

WCAG or Vendor Guidance (optional)

MDN Tab Role Avoiding tab styles for navigation, Adam Silver (gov.uk)

joshkimux commented 2 years ago

@jerekshoe if you have time I'd love to sync with you on this! Just talked to the design team and determined that we probably won't be able to do any deeper fixes until later, so we should focus on temporary fixes in the meantime 😄

Action items for me:

joshkimux commented 2 years ago

Update for folks here @jerekshoe and @skylerschain for awareness. We've decided to go with the latter fix of removing the tab styling and role from this pattern (essentially turning them into a straightforward list of links). Important note here that this is still meant to be an intermediary fix. We may need to change this later after more research is conducted. I'll update the acceptance criteria to match this in a bit.

After reviewing this in more detail, I think I'll want to spend more time outlining the necessary changes. Using a link list in this instance will require more design, content, and IA changes including, but not limited to:

On the flip side, introducing keyboard navigation to the existing a elements will:

joshkimux commented 2 years ago

After talking with @skylerschain we've decided to hold off on this until more technical discovery is done here 😄

jerekshoe commented 2 years ago

@joshkim I actually did find something interesting while looking at the code for the tabs. It turns out that you can switch between these via keyboard input. It probably doesn't work the way that it should, but currently on mac if you use option+1, 2, or 3, it switches between the tabs

joshkimux commented 2 years ago

@jerekshoe nice find! I tested on Chrome the other day and they seemed to work, so I'll see if I can dig a bit deeper into why it's not working on mac/safari/voiceover given it works the way it should on MDN's demo.

joshkimux commented 2 years ago

@jerekshoe tested again this morning on both CST tab panel and MDN's tab panel; seems like this is an issue that's universal to most screen reader browser combinations.

If it's not too much of a lift and doesn't feel like too much of a hack (we'll need to document this either way), we could resolve this ticket in the meantime if:

Alternatively, if @RLHecht and @jsokohl are both confident that these are meant to be separate pages (and not sections of a page), we could look into removing the tab panel functionality all together in favor of pure, semantic links. This could mean introducing a new pattern for the design system and could likely require a collab cycle review.

And alternatively to that, neither may even be required if a wholesale redesign is done to CST where the concept of a panel may not be needed at all (e.g. the former team's redesign concept featured accordions).

That all being said, @skylerschain and I agreed to hold off on solutioning until more research was done given how complex we anticipated all of the above could be 😆

Edge / Windows (Keyboard only)

JAWs / Edge / Windows

Chrome / Windows (Keyboard only)

NVDA / Chrome / Windows

jerekshoe commented 2 years ago

@joshkimux Just to provide a bit more context, as far as the code is concerned, the different tabs are separate pages. They each have their own entry in the router and clicking one of the tabs goes to a different route, we are not utilizing hashes.

joshkimux commented 2 years ago

That's awesome context, thanks @jerekshoe ! These should be full fledged links then (without the tab role) which means we'll need to do alot of work to get them (and their surroundings) back into accessible shape

jacobworrell commented 2 years ago

This would become a default level 3 if the left and right arrow keys worked to switch focus between tabs. And make sure space key selects the link.

jacobworrell commented 1 year ago

We are looking at addressing this defect with some design changes to CST part of a future release. @joshkimux . We haven't forgot about it.

HeatherWidmont commented 1 year ago

@joshkimux since we are currently working on fixing this in the new CST Designs, is there anything left to do for this ticket or can we close it out?

joshkimux commented 1 year ago

@HeatherWidmont great question! We can close this out once it is in production. I'll need to test it one more time on staging when it's implemented to ensure the fix has been done.