Skip to content

Conversation

@marissahuysentruyt
Copy link
Collaborator

@marissahuysentruyt marissahuysentruyt commented Oct 17, 2025

Description

This PR implements visually hidden dismiss buttons in the <sp-tray> component to improve mobile screen reader accessibility, particularly for VoiceOver on iOS. The dismiss buttons are automatically rendered before and after the tray's slotted content, allowing mobile screen reader users to easily dismiss the overlay from either end without requiring manual implementation by developers.

Changes include:

  1. Added dismissHelper getter to Tray.ts that returns a visually hidden button template
  2. Updated render method to wrap slotted content with dismiss helpers
  3. Enhanced CSS to support .visually-hidden class for shadow DOM content
  4. Comprehensive documentation in README with explanations and tabbed examples

The implementation follows the same pattern used in the <sp-picker> component, ensuring consistency across the design system.

Motivation and context

Mobile screen reader users, particularly those using VoiceOver on iOS, navigate through interactive elements sequentially (by swiping). Without dismiss buttons at the beginning and end of tray content, users who navigate past all menu items have difficulty discovering how to close the overlay - they must either navigate backward through all items or close the app entirely.

This gap affects any tray usage with menu content that lacks a built-in dismiss mechanism.

Related issue(s)

Screenshots (if appropriate)

Screenshot 2025-10-20 at 2 52 25 PM

Author's checklist

  • I have read the CONTRIBUTING and PULL_REQUESTS documents.
  • I have reviewed the Accessibility Practices for this feature, see: Aria Practices
  • I have added automated tests to cover my changes.
  • I have included a well-written changeset if my change needs to be published.
  • I have included updated documentation if my change required it.

Reviewer's checklist

  • Includes a Github Issue with appropriate flag or Jira ticket number without a link
  • Includes thoughtfully written changeset if changes suggested include patch, minor, or major features
  • Automated tests cover all use cases and follow best practices for writing
  • Validated on all supported browsers
  • All VRTs are approved before the author can update Golden Hash

Manual review test cases

  • Verify dismiss buttons are accessible to VoiceOver on Mac [@cdransf]

    1. Enable VoiceOver (Cmd + F5 or from within the Accessibility/VoiceOver menus in System Settings)
    2. Open a tray with menu content using the examples in the documentation
    3. Navigate forward with Control + Option + Right Arrow
    4. Expect: VoiceOver announces "Dismiss, button" before the first menu item
    5. Continue navigating through all menu items
    6. Expect: VoiceOver announces "Dismiss, button" after the last menu item
    7. Activate either dismiss button with Control + Option + Space
    8. Expect: Tray closes successfully
Screenshot 2025-10-17 at 4 31 18 PM

Verify dismiss buttons don't interfere with keyboard navigation [@cdransf]

1. Open a tray with menu content
2. Press Tab key to navigate through interactive elements
3. Expect: Tab navigation skips the visually hidden dismiss buttons (due to tabindex="-1")
4. Expect: Tab moves directly from trigger to menu items

  • Verify dismiss buttons are keyboard accessible

    1. Open a tray with menu content (e.g., using the "Content has no buttons" example from README)
    2. Press Tab key repeatedly to navigate through interactive elements
    3. Expect: Focus moves to the first visually hidden dismiss button
    4. Expect: Press Enter to close the tray
    5. Re-open the tray and tab through all content
    6. Expect: Focus reaches the second dismiss button after the last menu item
    7. Press Enter to close the tray from the second button
  • Verify auto-detection correctly identifies existing buttons

    1. Open a tray with dialog content that has `dismissable` attribute ("Content has buttons" example)
    2. Inspect the tray element in DevTools
    3. Expect: No visually hidden dismiss buttons are rendered
    4. Open a tray with menu content only ("Content has no buttons" example)
    5. Expect: Two dismiss buttons are rendered (before and after content)
  • Verify manual override property works correctly

    1. Open a tray with `has-keyboard-dismiss` attribute set ("Manual override" example)
    2. Inspect the tray element
    3. Expect: No visually hidden dismiss buttons are rendered, even though content lacks buttons
    4. Verify: You are responsible for providing your own dismiss mechanism
  • Verify dismiss buttons work with dialog content [@cdransf]

    1. Open a tray with dismissable dialog content
    2. Enable VoiceOver
    3. Navigate through the tray content
    4. Expect: Both the tray's dismiss helpers and dialog's close button are accessible
    5. Verify either method successfully closes the overlay
Screenshot 2025-10-17 at 4 25 46 PM
  • Verify dismiss buttons are visually hidden [@cdransf]

    1. Open a tray with menu content in a browser
    2. Inspect the tray element
    3. Expect: Dismiss button divs exist in DOM with .visually-hidden class
    4. Expect: No visual indication of these buttons (1px size, positioned absolutely, clipped)

Device review

  • Did it pass in Desktop?
  • Did it pass in (emulated) Mobile?
  • Did it pass in (emulated) iPad?

Note: Primary testing should be done with VoiceOver on iOS/iPadOS devices or simulators where the original accessibility issue was discovered.

@marissahuysentruyt marissahuysentruyt self-assigned this Oct 17, 2025
@changeset-bot
Copy link

changeset-bot bot commented Oct 17, 2025

🦋 Changeset detected

Latest commit: 661de4a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 84 packages
Name Type
@spectrum-web-components/tray Minor
@spectrum-web-components/picker Minor
@spectrum-web-components/bundle Minor
@spectrum-web-components/action-menu Minor
@spectrum-web-components/custom-vars-viewer Minor
example-project-rollup Patch
example-project-webpack Patch
@spectrum-web-components/story-decorator Minor
documentation Patch
@spectrum-web-components/breadcrumbs Minor
@spectrum-web-components/eslint-plugin Minor
@spectrum-web-components/accordion Minor
@spectrum-web-components/action-bar Minor
@spectrum-web-components/action-button Minor
@spectrum-web-components/action-group Minor
@spectrum-web-components/alert-banner Minor
@spectrum-web-components/alert-dialog Minor
@spectrum-web-components/asset Minor
@spectrum-web-components/avatar Minor
@spectrum-web-components/badge Minor
@spectrum-web-components/button-group Minor
@spectrum-web-components/button Minor
@spectrum-web-components/card Minor
@spectrum-web-components/checkbox Minor
@spectrum-web-components/clear-button Minor
@spectrum-web-components/close-button Minor
@spectrum-web-components/coachmark Minor
@spectrum-web-components/color-area Minor
@spectrum-web-components/color-field Minor
@spectrum-web-components/color-handle Minor
@spectrum-web-components/color-loupe Minor
@spectrum-web-components/color-slider Minor
@spectrum-web-components/color-wheel Minor
@spectrum-web-components/combobox Minor
@spectrum-web-components/contextual-help Minor
@spectrum-web-components/dialog Minor
@spectrum-web-components/divider Minor
@spectrum-web-components/dropzone Minor
@spectrum-web-components/field-group Minor
@spectrum-web-components/field-label Minor
@spectrum-web-components/help-text Minor
@spectrum-web-components/icon Minor
@spectrum-web-components/icons-ui Minor
@spectrum-web-components/icons-workflow Minor
@spectrum-web-components/icons Minor
@spectrum-web-components/iconset Minor
@spectrum-web-components/illustrated-message Minor
@spectrum-web-components/infield-button Minor
@spectrum-web-components/link Minor
@spectrum-web-components/menu Minor
@spectrum-web-components/meter Minor
@spectrum-web-components/modal Minor
@spectrum-web-components/number-field Minor
@spectrum-web-components/overlay Minor
@spectrum-web-components/picker-button Minor
@spectrum-web-components/popover Minor
@spectrum-web-components/progress-bar Minor
@spectrum-web-components/progress-circle Minor
@spectrum-web-components/radio Minor
@spectrum-web-components/search Minor
@spectrum-web-components/sidenav Minor
@spectrum-web-components/slider Minor
@spectrum-web-components/split-view Minor
@spectrum-web-components/status-light Minor
@spectrum-web-components/swatch Minor
@spectrum-web-components/switch Minor
@spectrum-web-components/table Minor
@spectrum-web-components/tabs Minor
@spectrum-web-components/tags Minor
@spectrum-web-components/textfield Minor
@spectrum-web-components/thumbnail Minor
@spectrum-web-components/toast Minor
@spectrum-web-components/tooltip Minor
@spectrum-web-components/top-nav Minor
@spectrum-web-components/underlay Minor
@spectrum-web-components/vrt-compare Minor
@spectrum-web-components/base Minor
@spectrum-web-components/grid Minor
@spectrum-web-components/opacity-checkerboard Minor
@spectrum-web-components/reactive-controllers Minor
@spectrum-web-components/shared Minor
@spectrum-web-components/styles Minor
@spectrum-web-components/theme Minor
@spectrum-web-components/truncated Minor

Not sure what this means? Click here to learn what changesets are.

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

@github-actions
Copy link
Contributor

github-actions bot commented Oct 17, 2025

📚 Branch Preview

🔍 Visual Regression Test Results

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

Deployed to Azure Blob Storage: pr-5814

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

@marissahuysentruyt marissahuysentruyt added bug Something isn't working a11y Issues related to accessibility Component: Tray labels Oct 17, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Oct 17, 2025

Tachometer results

Chrome

tray permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 563 kB 95.08ms - 97.75ms - faster ✔
8% - 12%
8.93ms - 12.92ms
branch 553 kB 105.85ms - 108.81ms slower ❌
9% - 14%
8.93ms - 12.92ms
-
Firefox

tray permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 563 kB 164.82ms - 170.02ms - faster ✔
7% - 11%
12.77ms - 20.63ms
branch 553 kB 181.18ms - 187.06ms slower ❌
8% - 12%
12.77ms - 20.63ms
-

@coveralls
Copy link
Collaborator

coveralls commented Oct 17, 2025

Pull Request Test Coverage Report for Build 18786444697

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 105 of 111 (94.59%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.01%) to 97.953%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/tray/src/Tray.ts 105 111 94.59%
Totals Coverage Status
Change from base Build 18730825611: -0.01%
Covered Lines: 34353
Relevant Lines: 34888

💛 - Coveralls

@marissahuysentruyt
Copy link
Collaborator Author

marissahuysentruyt commented Oct 20, 2025

TODO questions:

  • Should the visually-hidden dismiss buttons extend past just mobile trays? (like not just for small screens, but at all screen sizes) edit: trays are intended for small screens. i think sp-dialog-wrapper is the large screen equivalent.
  • Should the dismiss buttons be tied to any dismissible property? For instance, if there's a visible & accessible close button in the dialog content, do we still or should we still have visually-hidden buttons before and after the rest of the content?
  • If DialogWrapper also doesn't support visually-hidden dismiss buttons but it should- should those buttons go on DialogBase since DialogWrapper extends DialogBase? What about regular Dialog without the dismissible property? (edit: adding the dismissHelper to sp-dialog itself/Dialog.ts seems to flow how I am expecting.)

EDIT 10/21/25: after pairing with Nikki, reaching out to James Nurthen might be a good idea, just to validate the approach you'll see in the PR right now:

  • implemented dismissHelper into sp-tray, sp-dialog-base, and sp-dialog-wrapper (I'm open to feedback on where the dismissHelper needs to be defined!)
  • because you can force "mobile" behavior, we opted to render the tray at all screen sizes with visually hidden buttons before and after the slot content.
  • in dialog instances, there is a conditional for the second visually-hidden button. If the dialog is dismissable, meaning it renders a visible close button, the second dismissHelper button doesn't render.

EDIT: 10/23/25: we do not have to render any close button with non-dismissible dialogs. That work has been reverted.

  • the tray should auto-detect if it's content has buttons
    • if the content has buttons, no visually-hidden buttons are rendered
    • if the content has no buttons, visually-hidden buttons are rendered
    • users can manually override so that no visually-hidden buttons are rendered. There's a note in the docs about then needing to do custom keyboard-accessible dismiss functionality (that's up to the consumer to do)

@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/swc-274-tray-visually-hidden-bug branch from 642619c to a7363f4 Compare October 21, 2025 18:47
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/swc-274-tray-visually-hidden-bug branch from a7363f4 to 225faf7 Compare October 21, 2025 19:12
@marissahuysentruyt marissahuysentruyt marked this pull request as ready for review October 21, 2025 19:22
@marissahuysentruyt marissahuysentruyt requested a review from a team as a code owner October 21, 2025 19:22
@marissahuysentruyt marissahuysentruyt added the Status: Ready for review PR ready for review or re-review. label Oct 21, 2025
@marissahuysentruyt marissahuysentruyt marked this pull request as draft October 22, 2025 21:30
@marissahuysentruyt marissahuysentruyt added Status: WIP PR is a work in progress or draft and removed Status: Ready for review PR ready for review or re-review. labels Oct 22, 2025
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/swc-274-tray-visually-hidden-bug branch from dd22eca to a8be2ba Compare October 23, 2025 13:14
@marissahuysentruyt marissahuysentruyt marked this pull request as ready for review October 23, 2025 15:26
…pers

we do not need to provide a close/visually hidden button for
non-dismissible diaologs
…tons

we do not need to provide a close/visually hidden button for
non-dismissible dialogs
we do not need to provide a close/visually hidden button for
non-dismissible dialogs
- adds several properties and methods to tray API to support more
flexibile dismissible behavior.
- queries for keyboard-accessible dismiss buttons in the tray's slot
content
- adds a state property to track if dismiss buttons are needed
- adds manual override for dismissible behavior
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/swc-274-tray-visually-hidden-bug branch from a8be2ba to 542fb4d Compare October 23, 2025 16:29
@marissahuysentruyt marissahuysentruyt added Status: Ready for review PR ready for review or re-review. and removed Status: WIP PR is a work in progress or draft labels Oct 23, 2025
Copy link
Contributor

@nikkimk nikkimk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I loved all the regressions tests and the descriptive PR with reviewer manual testing instructions. Don't forget to add keyboard testing of this button as screen reader users need to access this button via keyboard.

For example VoiceOver is reading the button on the example in the first tab here https://swcpreviews.z13.web.core.windows.net/pr-5814/docs/components/tray/#accessibility, but I can't access the button via keyboard, so I still can't dismiss the tray.

return html`
<div class="visually-hidden">
<button
tabindex="-1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenreader users need to be able to access this via keyboard, so a tabindex should not exist here.

Suggested change
tabindex="-1"

});

describe('Dismiss helpers', () => {
it('renders visually-hidden dismiss helpers when no buttons detected', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also text that this button can be accessed via keyboard to dismiss the tray.


expect(el.open).to.be.false;
});

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love all the regression tests that were added here. :chefs_kiss:

@nikkimk nikkimk self-requested a review October 24, 2025 16:21
></sp-underlay>
<div
class="tray modal"
tabindex="-1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because tray-modal has a tabindex="-1", I still can't get to the dismiss buttons with a keyboard

Image
Suggested change
tabindex="-1"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if I'm still missing anything here. Without removing the tabindex="-1" on the .tray.modal, I can still tab to the buttons.

Screen.Recording.2025-10-24.at.2.11.49.PM.mov

Did we want some sort of focus indicator? What's the expectation if the button is visually hidden, but keyboard accessible? 🤔
Screenshot 2025-10-24 at 2 25 54 PM

);
expect(buttons.length).to.equal(2);
expect(buttons[0].getAttribute('aria-label')).to.equal('Dismiss');
expect(buttons[0].getAttribute('tabindex')).to.be.null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's test this by tabbing to the button, pressing Enter and checking if the tray closes. It is a more accurate way to test if this works.

In rthe PR description, we should also ask reviews to test with a keyboard as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description should be updated (sorry I missed that earlier!).

I also have 2 more tests now- one to make sure the button is tabbable, and the other to make sure enter closes the tray once the button is tabbed to. Thanks for that!

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

Labels

a11y Issues related to accessibility bug Something isn't working Component: Tray Status: Ready for review PR ready for review or re-review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug][a11y]: Tray component lacks visually hidden dismiss button to close the overlay on mobile

4 participants