FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: enhance accessibility for sidebar toggle button by sy-records · Pull Request #2604 · docsifyjs/docsify · GitHub

  1. docsifyjs
  2. docsify

Repository navigation

fix: enhance accessibility for sidebar toggle button - #2604

Merged
sy-records merged 3 commits into
docsifyjs:developfrom
sy-records:fix/2599
Sep 18, 2025
ConversationCommits (3)ChecksFiles changed
Merged

fix: enhance accessibility for sidebar toggle button#2604
sy-records merged 3 commits into
docsifyjs:developfrom
sy-records:fix/2599

Conversation

sy-records commented Sep 15, 2025

Copy link
Copy Markdown
Member

Summary

Related issue, if any:

Fix #2599

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

For any code change,

  • Related documentation has been updated, if needed
  • Related tests have been added or updated, if needed

Does this PR introduce a breaking change?

  • Yes
  • No

Tested in the following browsers:

  • Chrome
  • Firefox
  • Safari
  • Edge

vercel Bot commented Sep 15, 2025 •
edited
Loading

Copy link
Copy Markdown

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

Project Deployment Preview Comments Updated (UTC)
docsify-preview Ready Preview Comment Sep 17, 2025 9:08am

sy-records commented Sep 15, 2025

Copy link
Copy Markdown
Member Author

@msftedad Could you please test it at https://docsify-preview-git-fork-sy-records-fix-2599-docsifyjs.vercel.app/preview/#/?

msftedad commented Sep 16, 2025 •
edited
Loading

Copy link
Copy Markdown

@sy-records, As verified this issue in docsify. We observed that when the screen reader first focuses on the control, it announces 'Toggle Primary navigation button '. After pressing Enter or Space to hide or show the left navigation, the screen reader announces, 'Show primary navigation/Hide primary navigation, button, '.

As discussion with the PWD team, they recommended that when the screen reader initially focuses on the control, it should announce the button's accessible label as 'Hide primary navigation', indicate its role as a button, and provide the shortcut key hint, such as 'Use shortcut key '.

Please let us know if you need any further information. Thanks

sy-records commented Sep 16, 2025

Copy link
Copy Markdown
Member Author

Thank you for your feedback. I've made the changes.

paulhibbitts previously approved these changes Sep 16, 2025
View reviewed changes

paulhibbitts left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks @sy-records @msftedad , testing on Mac OS and VoiceOver things look good and hopefully @msftedad can confirm the latest change.

msftedad commented Sep 17, 2025

Copy link
Copy Markdown

Hi @paulhibbitts,
Currently, the screen reader announces: "Hide primary navigation, Use shortcut key , button, " because the shortcut hint is included within the aria-label along with aria-keyshortcuts.

To fix this, set them separately as:
aria-label="Hide primary navigation"
and
aria-keyshortcuts="Use shortcut key "

sy-records commented Sep 17, 2025

Copy link
Copy Markdown
Member Author

Shouldn't it be aria-keyshortcuts="Use shortcut key \"?

msftedad commented Sep 17, 2025

Copy link
Copy Markdown

@sy-records, Yes, it should be. I noticed that there’s an issue when adding comments — the \ is automatically removed.

Currently, the screen reader announces: "Hide primary navigation, Use shortcut key \ , button \ , " because the shortcut hint is included within the aria-label along with aria-keyshortcuts.

To fix this, set them separately as:
aria-label="Hide primary navigation \ "
and
aria-keyshortcuts="Use shortcut key \ "

Please let us know if any further information is required.

sy-records commented Sep 17, 2025

Copy link
Copy Markdown
Member Author

Okay, I've made the changes. Please retest.

msftedad commented Sep 17, 2025

Copy link
Copy Markdown

@sy-records, As verified this issue on docsify, and it appears to be resolved. Now, when the screen reader focus moves to the Left navigation pane show/hide button, it announces 'Hide primary navigation, button, Use shortcut key \ '. When Enter or Space is pressed, the screen reader announces, 'Show primary navigation, button, Use shortcut key \ '.

Thank you.

paulhibbitts approved these changes Sep 17, 2025
View reviewed changes

paulhibbitts left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks very much @msftedad for confirming these changes, I am learning as we go through this very helpful process!

sy-records merged commit 3014945 into docsifyjs:develop Sep 18, 2025
8 checks passed
sy-records deleted the fix/2599 branch September 18, 2025 02:39
trusktr added a commit that referenced this pull request Nov 17, 2025
Merge branch 'develop' into type-definitions
c860c65 
* develop: (126 commits)
  refactor: improve blockquoteCompiler logic (#2624)
  docs: add DeployHQ deployment instructions (#2627)
  docs: fix broken link to issues in CONTRIBUTING.md (#2625)
  chore: bump rimraf from 5.0.7 to 6.1.0 (#2619)
  chore: bump @rollup/plugin-commonjs from 28.0.1 to 29.0.0 (#2618)
  chore: bump actions/setup-node from 5 to 6 (#2620)
  chore: bump actions/upload-artifact from 4 to 5 (#2621)
  chore: bump stefanzweifel/git-auto-commit-action from 6 to 7 (#2622)
  test: improve file embed & code fragment tests (#2617)
  test: add test for file embed and code fragments (#2616)
  feat: add fallback default language support (#2607)
  chore: bump actions/setup-node from 3 to 5 (#2609)
  chore: bump actions/checkout from 4 to 5 (#2608)
  docs: Update intro paragraph to help improve first impressions and broaden audience (#2602)
  feat: enhance embed handling for table cells (#2606)
  fix: enhance accessibility for sidebar toggle button (#2604)
  [release] 5.0.0-rc.3
  [release] 5.0.0-rc.2
  fix: normalize slugs to NFC and remove emoji variation selector (#2597)
  feat: GitHub style callouts (#2487)
  ...
sy-records mentioned this pull request Mar 11, 2026
Merged
sy-records mentioned this pull request Jul 3, 2026
Merged

This branch was successfully deployed

1 active deployment
Preview — 4cf45eef Deployed Sep 17, 2025 by vercel[bot]
View deployment
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

paulhibbitts paulhibbitts approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Incorrect name and no state is defined for the ‘Hamburger’ menu button.

3 participants

Back | FazBrowse Home | New Git URL