Skip to content
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(docsearch-react): accessibility improvements #1391

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

isner
Copy link

@isner isner commented May 19, 2022

These changes address some of the concerns raised in #1370.

Issues addressed:

  • Incorrect role/attributes on dialog
  • Focus lost/not managed when closing dialog
  • Low-contrast text

Issues not addressed:

  • Adding aria-hidden or inert to the elements underlying the modal dialog (this was a heavier lift than I was prepared to undertake, and adding the aria-modal="true" attribute to the dialog has addressed this issue for some ATs)
  • Incorrect role/attributes on the search input itself (I don't think the input should actually be a combobox as suggested in Accessibility issues with v3 dialog #1370, since its purpose is not to assist the user in completing the field, but rather producing a list of navigation links)
  • Low-contrast icons (I wasn't prepared to create new icons)

@netlify
Copy link

netlify bot commented May 19, 2022

Deploy Preview for docsearch ready!

Name Link
🔨 Latest commit f9c3576
🔍 Latest deploy log https://app.netlify.com/sites/docsearch/deploys/6320a2a2aed0700009b375cf
😎 Deploy Preview https://deploy-preview-1391--docsearch.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 96f713e:

Sandbox Source
Vanilla Configuration

@isner isner changed the title Accessibility improvements fix(docsearch-react): accessibility improvements May 19, 2022
@isner
Copy link
Author

isner commented May 25, 2022

The Netlify deployment was cancelled automatically due to lack of content changes. This PR does introduce changes to the UI behavior and markup, however, so a preview deployment would still be useful. Is there any way to manually deploy from this branch?

@shortcuts
Copy link
Member

Hey, thanks for the contribution 💙, I'll take a look at it ASAP!

a preview deployment would still be useful. Is there any way to manually deploy from this branch?

Indeed, you can update this line to include packages/docsearch-react too!

@isner
Copy link
Author

isner commented Sep 13, 2022

a preview deployment would still be useful. Is there any way to manually deploy from this branch?

Indeed, you can update this line to include packages/docsearch-react too!

I made the change you requested, but I didn't realize that Netlify was only being used to deploy the DocSearch website. I don't think it's necessary to trigger a preview deployment of the website when the contents of the docsearch-react package are changed.

Instead, it would be useful to preview the changes that are introduced by this PR in CodeSandbox. When I follow the link provided by the codesandbox-ci bot I see the build output, which is great, but it's unclear how to access a working preview of that package build. This would be the easiest way for you to review the PR changes in action. Any suggestions?

@jfgreffier
Copy link

@isner @shortcuts this PR is from last year, is it still revelant ? If so, it probably needs a git rebase and solving the conflicts

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