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

Remove / search shortcut #2025

Merged
merged 9 commits into from
Jul 5, 2024

Conversation

HiDeoo
Copy link
Member

@HiDeoo HiDeoo commented Jun 18, 2024

Description

This PR removes the / search shortcut for accessibility reasons.

We cannot entirely rely on aria-keyshortcuts to indicate the presence of the search shortcut due to its poor support in screen readers. The markup was slightly updated to get the best out of it.

Here is a video of the behavior with VoiceOver on macOS:

voiceover.mp4

Here is a video of the behavior with NVDA on Windows:

nvda.mp4

Copy link

changeset-bot bot commented Jun 18, 2024

🦋 Changeset detected

Latest commit: aa92b0c

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

This PR includes changesets to release 1 package
Name Type
@astrojs/starlight 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

Copy link

vercel bot commented Jun 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
starlight ✅ Ready (Inspect) Visit Preview Jul 5, 2024 9:10am
starlight-i18n 🔄 Building (Inspect) Visit Preview Jul 5, 2024 9:10am

Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Thanks for the research and work on this @HiDeoo! I had a couple of small questions/suggestions, but generally seems really solid. I appreciate the screen reader videos!

cc @markteekman on this one too as the original issue author

.changeset/big-buttons-shop.md Show resolved Hide resolved
package.json Show resolved Hide resolved
packages/starlight/components/Search.astro Outdated Show resolved Hide resolved
packages/starlight/components/Search.astro Outdated Show resolved Hide resolved
@delucis delucis added the 🌟 minor Change that triggers a minor release label Jul 4, 2024
HiDeoo and others added 3 commits July 4, 2024 15:53
Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

LGTM! Suggesting one small translation change, but all the code changes seem spot on. Thank you @HiDeoo 🚀

packages/starlight/translations/de.json Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌟 core Changes to Starlight’s main package 🌟 minor Change that triggers a minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Accessibility issue with the search navigation keyboard shortcut
3 participants