Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat: Add "Skip to main content" link and update nav behavior#2253

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
sy-records merged 24 commits intodevelopfrom494-skip-link
Nov 30, 2023

Conversation

@jhildenbiddle
Copy link
Member

@jhildenbiddlejhildenbiddle commentedOct 7, 2023•
edited
Loading

Summary

These updates align with accessibility best practices. Theskip link and the updated navigation behavior allow for more efficient navigation via the keyboard and screen readers.

  1. Add a "Skip to main content" button as the first element in the DOM with options.
    • Includes configuration options to disable, modify the label, and specify localized labels based on the route path.
    • Dynamically updates label text on route path change (use "Translations" menu in preview for demo)
  2. Updated navigation behavior:
    • When loading a Docsify site:
      • If the URL does not contains a headingid, focus will remain on the<body> element. This is the browser's default behavior, which allows the skip link to appear when visitors pressTab one time.
      • If the URL contains a headingid, focus will be moved to the linked heading.
    • When navigating to a new page, focus will move to the first heading in the content area.
    • When navigating to a heading, focus will move to the linked heading.
  3. Updated documentation to includeskipLink configuration options

This bug also addresses a separate issue where caused Docsify to treat successfully loaded but empty markdown files as 404 errors.

Related research

Screenshots

Safari 17 on macOS 14.0

CleanShot 2023-10-07 at 17 28 38@2x

CleanShot 2023-10-07 at 17 30 03@2x

Related issue, if any:

#494
#495
#2294 (Fixes loading of empty markdown files)

What kind of change does this PR introduce?

  • Feature
  • Docs
  • Tests

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?

No

Tested in the following browsers:

  • Chrome
  • Firefox
  • Safari

sy-records reacted with thumbs up emojipaulhibbitts reacted with hooray emojiKoooooo-7 reacted with heart emoji
Allows skip link text to be updated dynamically based on the route path. See docisfy site’s “Translations” menu as an example of why this is necessary.
@vercel
Copy link

vercelbot commentedOct 7, 2023•
edited
Loading

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

NameStatusPreviewCommentsUpdated (UTC)
docsify-preview✅ Ready (Inspect)Visit Preview💬Add feedbackNov 30, 2023 4:49am

@jhildenbiddlejhildenbiddle changed the titlefeat: Add "Skip to main content" linkfeat: Add "Skip to main content" link and update nav behaviorOct 8, 2023
@jhildenbiddlejhildenbiddle changed the titlefeat: Add "Skip to main content" link and update nav behaviorFeat: Add "Skip to main content" link and update nav behaviorOct 16, 2023
trusktr
trusktr previously approved these changesOct 22, 2023
Co-authored-by: Joe Pea <trusktr@gmail.com>
# Conflicts:#src/core/render/index.js
Copy link
Member

@Koooooo-7Koooooo-7 left a comment

Choose a reason for hiding this comment

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

Can not show the btn locally or on thevercel Preview, although the dom is insert success.
Is there any style issue?

@jhildenbiddle
Copy link
MemberAuthor

jhildenbiddle commentedNov 22, 2023•
edited
Loading

@Koooooo-7 --

TL;DR: PressTab after the initial site load.

  1. Click thepreview link
  2. PressTab

Or...

  1. Navigate directly to apreview page
  2. PressTab

Can not show the btn locally or on thevercel Preview, although the dom is insert success. Is there any style issue?

The Vercel preview is working correctly.

The "Skip to main content" link is intended to be the first "focusable" element in the DOM. It is hidden by default and only visible when it receives focus. The intention is for it to be hidden for visitors navigating with a pointing device, but easily accessible to keyboard navigators and assistive device users by pressingTab after the initial site load.

If you are wondering why pressingTab does not result in the skip link being displayed after subsequent page loads, the following explanation taken fromthis comment explains:

For SPAs, there is no definitively "correct" behavior regarding where to move the focus after new content has loaded, although the majority opinion based on my research is that moving focus to the first heading in the content area is the most desirable behavior. The alternative is to set the focus back on the<body> to mimic the page load behavior of "standard" web site (i.e. non-SPA sites), but this is a far less practical behavior that requires users to manually navigate to the content area after every new page is loaded. Less than a minute testing with the keyboard or a screen reader makes choosing between these two options easy.

sy-records reacted with thumbs up emoji

sy-records
sy-records previously approved these changesNov 22, 2023
Copy link
Member

@sy-recordssy-records left a comment

Choose a reason for hiding this comment

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

Good jobs!

@Koooooo-7
Copy link
Member

Hi@jhildenbiddle , sorry for the misunderstanding here.
Hit theTab, I can see it now !

Based on our site here, about themulti langs support, I found that it only works when fresh the page ( as you mentioned, trigger after theinitial Load), so every time fresh pages gonna trigger theskipLink bytab as well .
My concern is when we fresh the page, or in multi langs, we may already on themain content, it seems we don't need this btn.
Should we only make it works on the coverage showing?

@jhildenbiddle
Copy link
MemberAuthor

Hi@jhildenbiddle , sorry for the misunderstanding here. Hit theTab, I can see it now !

No worries,@Koooooo-7. Happy to hear it's working as expected for you!

My concern is when we fresh the page, or in multi langs, we may already on themain content, it seems we don't need this btn. Should we only make it works on the coverage showing?

👍 I've made a small update to the PR. Focus will now move to the linked heading when loading a URL with a linked headingid (e.g.,?id=foo). This behavior mimics the standard hash behavior in standard web pages (e.g,#foo).

A complete summary of the new navigation behavior is available in the summary and below:

  1. Updated navigation behavior:
    • When loading a Docsify site:
      • If the URL does not contains a headingid, focus will remain on the<body> element. This is the browser's default behavior, which allows the skip link to appear when visitors pressTab one time.
      • If the URL contains a headingid, focus will be moved to the linked heading.
    • When navigating to a new page, focus will move to the first heading in the content area.
    • When navigating to a heading, focus will move to the linked heading.

Given the above behavior, I believe your concerns are addressed:

  • If a user loads a Docsify URL that contains a headingid, focus will move to the linked heading.
  • If a user navigates to a page using the "Translations" menu ondocsify.js.org, focus will move to the first heading in the content area.

In both scenarios above, pressingTab will not display the skip link.

Koooooo-7 reacted with thumbs up emoji

Koooooo-7
Koooooo-7 previously approved these changesNov 23, 2023
Copy link
Member

@Koooooo-7Koooooo-7 left a comment

Choose a reason for hiding this comment

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

LGTM. thx@jhildenbiddle for you ideas and contributions. 👍

jhildenbiddle reacted with heart emoji
# Conflicts:#src/core/render/index.js#src/core/render/tpl.js
@jhildenbiddle
Copy link
MemberAuthor

@sy-records@Koooooo-7 --

Would you mind approving this PR again? I had to mergedevelop which cleared your previous approvals. Thx!

Koooooo-7 reacted with rocket emoji

@sy-recordssy-records changed the titleFeat: Add "Skip to main content" link and update nav behaviorfeat: Add "Skip to main content" link and update nav behaviorNov 30, 2023
@sy-recordssy-records merged commit50b84f7 intodevelopNov 30, 2023
@sy-recordssy-records deleted the 494-skip-link branchNovember 30, 2023 05:11
@sy-recordssy-records mentioned this pull requestMay 5, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@Koooooo-7Koooooo-7Koooooo-7 approved these changes

@sy-recordssy-recordssy-records approved these changes

@trusktrtrusktrAwaiting requested review from trusktr

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Feature: Add response status toroute object and plugins [FEATURE REQUEST] Add option to generate skip link for screen reader users

5 participants

@jhildenbiddle@Koooooo-7@trusktr@sy-records
[8]ページ先頭
©2009-2025 Movatter.jp