-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: a11y: search component submit accessible naming #2737
Conversation
…viding an accessible name to small search submit, and a unique accessible name to the other variants. Sets default value to string of search, based on USWDS docs.
…e button naming issue on pages that may have multiple searches
…e is still support for internationalization
Taking a look now! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ANDI output works as expected. No visual changes in the design. Great work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non blocking, but minor code organizational nit recommended in review comments
4bb23da
Co-authored-by: Brandon Lenz <[email protected]>
Co-authored-by: Brandon Lenz <[email protected]>
Co-authored-by: Brandon Lenz <[email protected]>
c147d9d
to
2d89748
Compare
@shkeating is the tooltip supposed to be always visible, as Happo's showing? |
i don't know why that is happening. Is that happening on other branches? I didn't touch the tooltip code. And it looks like it is isolated to Safari? 🤔 |
… was working correctly
…e' into 2736-shk-small-button-search-name
…sts test to account for that
de0959f
to
740b873
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used yarn link as described in our docs here to import this branch into our project and the aria-label shows up for search buttons as expected when I run it all locally.
Summary
adds support for accessible name for search submit button in small search variant, and enhances naming capabilities on all search variants submit button to prevent duplicate naming issues on pages with multiple search components. Example of this scenario in USSF portal provided in screen shots.
Related Issues or PRs
How To Test
check output in ANDI, which should reflect the fallback name of "Search". Note, the aria label added to the non-small variants will not conflict with the typed name button text in the default instances, because the aria label will override the button text in the accessibility tree. This will allow us to have "search" appear visually for users who will have visual context, but more precise labeling for users of assistive tech.
I encourage reviewers to be vigilant in the changes made to the Search test file. My intention is to pull this into the USSF portal repo to ensure it works as expected.
Screenshots (optional)
Example of multiple searches in a single interface
ANDI outputs in local storybook for react-uswds
Buttons with inner text
Small Search: Buttons without Inner Text