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: skip-to-content scroll behavior #2401

Merged
merged 3 commits into from Apr 17, 2024
Merged

Conversation

jhildenbiddle
Copy link
Member

@jhildenbiddle jhildenbiddle commented Apr 11, 2024

Summary

Fixes an issue the new (unreleased) "Skip to Content" link that caused an immediate jump to to the content (instead scrolling) and the main content to scroll down (instead of being static) into place.

Example of bug (develop branch preview w/ Chrome 123)

The incorrect behavior is the result of three separate things occurring at the same time:

  1. By default, browsers will automatically scroll any offscreen element that receives focus into view. This is the correct behavior for accessibility purposes because the focused item should be in view.
  2. Clicking the "Skip to Content" link places the focus on the main content area. This is the correct behavior for accessibility purposes because the focused item should be in view. However, by placing the focus on an off-screen element, the browser's default behavior (see above) causes the scroll position to jump to the content instead of smoothly scrolling to it.
  3. Docsify's "smooth scroll" behavior aligns the main content container element to the top edge of the viewport.
Example-Bug.mp4

Example of fix (PR changes w/ Chrome 123)

The fix if simple thanks to modern APIs: set the preventScroll option to true when calling the browser's native focus() method:

Example-fix.mp4

Related issue, if any:

None

What kind of change does this PR introduce?

Bugfix

For any code change,

N/A

Does this PR introduce a breaking change?

No

Tested in the following browsers:

  • Chrome
  • Firefox
  • Safari

Copy link

vercel bot commented Apr 11, 2024

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

Name Status Preview Comments Updated (UTC)
docsify-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 17, 2024 5:57am

@jhildenbiddle jhildenbiddle added this to the 5.x milestone Apr 11, 2024
@jhildenbiddle jhildenbiddle self-assigned this Apr 11, 2024
@jhildenbiddle jhildenbiddle requested a review from a team April 12, 2024 14:17
@Koooooo-7
Copy link
Member

Koooooo-7 commented Apr 13, 2024

Hi @jhildenbiddle , could plz more details on it? (Much thx that if you could provide a screenshot/recording)
sorry, that I don't full understand the behaviors.

As per the early discuss and my understanding, thats what I did:
I open 2 preview, with and without this fix.

On the preview site, press Tab, click the Skip to content is fine.
Then, I tabs Tab multi times, and click or switch sections.

@jhildenbiddle
Copy link
Member Author

jhildenbiddle commented Apr 15, 2024

@Koooooo-7 --

I've added descriptions and videos demonstrating the bug and the fix to PR summary (above).

Copy link
Member

@Koooooo-7 Koooooo-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, and nice experience improvement.

Copy link
Member

@sy-records sy-records left a comment

Choose a reason for hiding this comment

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

LGTM. Good job!

@sy-records sy-records merged commit 2d986fe into develop Apr 17, 2024
9 checks passed
@sy-records sy-records deleted the fix-skip-link-scroll branch April 17, 2024 06:58
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.

None yet

3 participants