Skip to content

Conversation

@joshwooding
Copy link
Contributor

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Sep 15, 2025

⚠️ No Changeset found

Latest commit: b94b747

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Sep 15, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
saltdesignsystem Ready Ready Preview, Comment Jan 7, 2026 2:14pm

@jake-costa
Copy link
Contributor

jake-costa commented Sep 27, 2025

Hi @joshwooding I appreciate your patience. Blow our my findings from my most recent review of tabs. Feel free to let me know if you have any questions or want to jump on a call to discuss.

Tab A11y Review:

  • Overflow dialog: Recommend adding aria-modal="true" if possible.

  • Overflow tab aria-haspopup: I recommend using true instead of dialog.

  • Closable tabs — recommend:

    • Add the option to remove the close button from the tab order and hide it from AT (drop the aria-label).
    • Support the Delete key to close a tab. This aligns with APG, reduces tab stops, and clears the Axe aria-required-children flag.
    • Set aria-description to: “Press Delete to remove the tab.”
    • Keep the close icon button clickable for mouse users.
    • Note:
      • If we add nested menus, also support “Shift+F10” to open the menu.
      • While this change would better align with the APG, this will likely create a challenge for mobile AT. Maybe we add a way to allow these nested options to receive focus but add to the docs that this should only be used for mobile support.

Examples:

I believe these are example-specific, not component-level. Correct me if wrong.

  • Add aria-hidden="true" to decorative icons (e.g., “With Icon”). Is this something we should also add to the documentation or is it something we can bake in at the component level

  • Reflow: Storybook tabs reflow at 320×256 px. Vercel site does not.

  • Badge “1” accessible name reads “One updates.” Expected: “One update.”

  • Tablist name: add an accessible name to the main tablist (aria-label). For the overflow tablist, use aria-labelledby that points to the dialog label.

    • We can add accessible-naming guidance to the docs. I can work on drafting it.

Vercel overflow example:

  • VoiceOver/Safari: announces “Checks, tab 4 of 4.” Overflow tab not counted. Unsure if this is fixable. Checking if this is a known issue.
  • NVDA/Firefox: announces “Checks, tab 4 of 5” (includes overflow).
  • JAWS/Chrome: announces “Checks, tab 4 of 5” (includes overflow).

Comments:

When a tab is chosen from overflow it moves to the main list and gets focus. Selecting another main-list tab then pushes that tab back into overflow. Do we want this, or should surfaced tabs remain in the main list until overflow recalculates?

@joshwooding
Copy link
Contributor Author

Hi @joshwooding I appreciate your patience. Blow our my findings from my most recent review of tabs. Feel free to let me know if you have any questions or want to jump on a call to discuss.

Tab A11y Review:

  • Overflow dialog: Recommend adding aria-modal="true" if possible.

  • Overflow tab aria-haspopup: I recommend using true instead of dialog.

  • Closable tabs — recommend:

    • Remove the close button from the tab order and hide it from AT (drop the aria-label).

    • Support the Delete key to close a tab. This aligns with APG, reduces tab stops, and clears the Axe aria-required-children flag.

    • Set aria-description to: “Press Delete to remove the tab.”

    • Keep the close icon button clickable for mouse users.

    • Note:

      • If we add nested menus, also support “Shift+F10” to open the menu.
      • While this change would better align with the APG, this will likely create a challenge for mobile AT. Maybe we add a way to allow these nested options to receive focus but add to the docs that this should only be used for mobile support.

Examples:

I believe these are example-specific, not component-level. Correct me if wrong.

  • Add aria-hidden="true" to decorative icons (e.g., “With Icon”). Is this something we should also add to the documentation or is it something we can bake in at the component level

  • Reflow: Storybook tabs reflow at 320×256 px. Vercel site does not.

  • Badge “1” accessible name reads “One updates.” Expected: “One update.”

  • Tablist name: add an accessible name to the main tablist (aria-label). For the overflow tablist, use aria-labelledby that points to the dialog label.

    • We can add accessible-naming guidance to the docs. I can work on drafting it.

Vercel overflow example:

  • VoiceOver/Safari: announces “Checks, tab 4 of 4.” Overflow tab not counted. Unsure if this is fixable. Checking if this is a known issue.
  • NVDA/Firefox: announces “Checks, tab 4 of 5” (includes overflow).
  • JAWS/Chrome: announces “Checks, tab 4 of 5” (includes overflow).

Comments:

When a tab is chosen from overflow it moves to the main list and gets focus. Selecting another main-list tab then pushes that tab back into overflow. Do we want this, or should surfaced tabs remain in the main list until overflow recalculates?

Thanks for the reply, your recommendations for deleted tabs change the previous spec so we'll have to raise this with the rest of the team.

@joshwooding
Copy link
Contributor Author

For the overflow tablist, use aria-labelledby that points to the dialog label.

@jake-costa what is the dialog label in this example?

@jake-costa
Copy link
Contributor

For the overflow tablist, use aria-labelledby that points to the dialog label.

@jake-costa what is the dialog label in this example?

I was referring to aria-label="Overflow Menu" on dialog. That said, since it is already announcing correctly, we would need to validate that this works correctly. While it would align with the APG, this is more of an edge case and I am unsure if the label would be announced twice. If it does announce twice I would recommend we keep the aria-label on the dialog but not add aria-labelledby to tablist.

@jake-costa
Copy link
Contributor

@joshwooding Let me know if you want to jump on a call for this.

To confirm, are we going to plan to add support for the Delete key as well as the ability to support the current focus interaction with the delete button?

@jake-costa
Copy link
Contributor

A11y Review Items

  • Can we rename the overflow tab label to just “Overflow” instead of “13 tabs hidden”? Right now screen readers announce: “13 tabs hidden tab collapsed subMenu 5 of 5”. With “Overflow” it would be: “Overflow tab collapsed subMenu 5 of 5”.
  • Can we support the Delete key to close a tab when the tab itself has focus, while still keeping the close button in the tab order?
  • Not sure why, but on Mac + Safari in the closable example, after closing a tab it looks like focus gets lost. I’m not seeing this with JAWS/NVDA. (@karlgoldstraw, @joshwooding Can you reproduce?)
  • Should I open a ticket to also create a scrollable tablist, or do we have one already?

Notes

  • We’re still going to have aXe flag the closable example because of the close button but we do call this out on the Tabs a11y page.
  • VoiceOver announces “4 tabs” while JAWS/NVDA announce “5 tabs.” This isn’t a violation. VoiceOver appears to use heuristics to exclude the overflow control (treating it as a menu trigger), while Windows screen readers count every role="tab" literally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants