va-accordion: remove aria-controls and aria-expanded from expand all button

[1Copenut]

Commit log

• Removed two ARIA attributes from Expand all button.
• Removed ARIA attributes from va-accordion e2e test.

Chromatic

https://patch-tpierce-accordionExpandAll--65a6e2ed2314f7b8f98609d8.chromatic.com

Reasoning for this change

I opted to remove the aria-expanded attribute from the <va-accordion> expand all button because it was announcing as "Collapse all accordions, expanded" in screen reader testing. This felt contradictory and potentially confusing. I felt the change in button text alone made it very clear that I had expanded 1 to N accordions as a group.

This change triggered an axe-core error for the aria-controls attribute. I did some more digging and determined that JAWS is still the only screen reader adopting aria-controls. JAWS uses this attribute to set virtual cursor on the "controlled" content, which here would be multiple things and not especially helpful. I used the following resources to validate this decision:

• ARIA controls vs ARIA owns
• Support for aria-controls attribute (aria)

---

Configuring this pull request

• Link to any related issues in the description so they can be cross-referenced.
• Add the appropriate version patch label (major, minor, patch, or ignore-for-release).
  • See How to choose a version number for guidance.
  • Use ignore-for-release if files haven't been changed in a component library package. (ie. only Storybook files)
• DST Only: Increment the /packages/core version number if this will be the last PR merged before a release.
• Complete all sections below.
• Delete this section once complete

Description

Closes department-of-veterans-affairs/va.gov-team#96354

QA Checklist

• Component maintains 1:1 parity with design mocks
• Text is consistent with what's been provided in the mocks
• Component behaves as expected across breakpoints
• Accessibility expert has signed off on code changes (if applicable. If not applicable provide reason why)
• Designer has signed off on changes (if applicable. If not applicable provide reason why)
• Tab order and focus state work as expected
• Changes have been tested against screen readers (if applicable. If not applicable provide reason why)
• New components are covered by e2e tests; updates to existing components are covered by existing test suite
• Changes have been tested in vets-website using Verdaccio (if applicable. If not applicable provide reason why)

Screenshots

---

---

Acceptance criteria

• QA checklist has been completed
• Screenshots have been attached that cover desktop and mobile screens

Definition of done

• Documentation has been updated, if applicable
• A link has been provided to the originating GitHub issue (or connected to it via ZenHub)

[1Copenut]

This block was no longer needed after the aria-controls attribute was removed. TypeScript was complaining so I opted to remove it.

[rsmithadhoc]

Hi @1Copenut,

This was initially developed the way you have it, using the button text to signal the status of the accordions, but aria-expanded was added because JAWS works a little differently from other screen readers. VoiceOver and NVDA announce button text changing, but JAWS does not. Here's my results from testing:

• Current iteration - Toggling back and forth between "Expand all/Collapse all" will announce "expanded/collapsed" each time. But does not re-read the button text when going back to "Expand all +".
• Proposed changes - Toggling back and forth between "Expand all/Collapse all" will not announce "expanded/collapsed" each time. And it does not re-read the button text when going back to "Expand all +". So you miss out on hearing the status.

So I don't believe we should remove aria-expanded from the Expand/Collapse All button because of that missing button text announcement. I didn't find it too confusing on the other screen readers that read out "Expand all, collapsed" and "Collapse all, expanded".

Can you let me know your thoughts?

Thanks!

[1Copenut]

Thank you for testing with JAWS @rsmithadhoc. I'm surprised that JAWS isn't announcing the changed text when you toggle back and forth, but maybe I shouldn't be. :)

I still believe that removing the aria-expanded is the right call, but wonder if we could add an aria-live region on the container DIV and retest. With a "polite" setting it shouldn't double up in NVDA and VoiceOver, and might smooth out the JAWS experience.

[1Copenut]

/cc @1Copenut

TODO: Add a data-cytest="NAME" attribute here and update the Cypress expandAccordions() helper. Could add an optional argument to accept a string value, but definitely need to ensure the helper doesn't break or tests start failing.

[rsmithadhoc]

Thank you for testing with JAWS @rsmithadhoc. I'm surprised that JAWS isn't announcing the changed text when you toggle back and forth, but maybe I shouldn't be. :)

I still believe that removing the aria-expanded is the right call, but wonder if we could add an aria-live region on the container DIV and retest. With a "polite" setting it shouldn't double up in NVDA and VoiceOver, and might smooth out the JAWS experience.

I believe aria-live was also tested, with a similar result: department-of-veterans-affairs/vets-design-system-documentation#1663 (comment)

[rsmithadhoc]

Thank you for testing with JAWS @rsmithadhoc. I'm surprised that JAWS isn't announcing the changed text when you toggle back and forth, but maybe I shouldn't be. :)
I still believe that removing the aria-expanded is the right call, but wonder if we could add an aria-live region on the container DIV and retest. With a "polite" setting it shouldn't double up in NVDA and VoiceOver, and might smooth out the JAWS experience.

I believe aria-live was also tested, with a similar result: department-of-veterans-affairs/vets-design-system-documentation#1663 (comment)

@1Copenut How do you feel about removing the aria-controls for now, but keeping aria-expanded? It looks like aria-controls can't even see the IDs that it's referencing through the shadow DOM, so it will always trigger an Axe error. Thanks!

[1Copenut]

@1Copenut How do you feel about removing the aria-controls for now, but keeping aria-expanded? It looks like aria-controls can't even see the IDs that it's referencing through the shadow DOM, so it will always trigger an Axe error. Thanks!

@rsmithadhoc I went ahead and removed the aria-controls and retested in Storybook and on vets-website with NVDA and Narrator on Win11 (Chrome, Edge, Firefox). The button is working as expected. I also added a data-testid hook in case we decide to remove the aria-expanded attribute later. This won't prevent the breaking change in the Cypress expandAccordions.js file, but will make fixing it significantly easier.

I still hold to my personal desire to remove the aria-expanded attribute but haven't gotten access to JAWS to properly test alternatives, so this feels like the best solution in the moment.

[rsmithadhoc]

Looks good, thanks!