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

[bug] scroll-behavior: smooth not working together with TOC anchor nav in 2.0.0-beta.15 #6620

Closed
6 of 7 tasks
yangshun opened this issue Feb 6, 2022 · 3 comments · Fixed by #6748
Closed
6 of 7 tasks
Labels
bug An error in the Docusaurus core causing instability or issues with its execution difficulty: starter Issues that are starter difficulty level, e.g. minimal tweaking with a clear test plan. status: accepting pr This issue has been accepted, and we are looking for community contributors to implement this

Comments

@yangshun
Copy link
Contributor

yangshun commented Feb 6, 2022

Have you read the Contributing Guidelines on issues?

Prerequisites

  • I'm using the latest version of Docusaurus.
  • I have tried the npm run clear or yarn clear command.
  • I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • I have tried creating a repro with https://new.docusaurus.io.
  • I have read the console error message carefully (if applicable).

Description

#6317 breaks pages using scroll-behavior: smooth as both JS and CSS are trying to smoothly scroll the page. For sites like about.supabase.com which uses scroll-behavior: smooth (reasonable thing to add IMO), cliking on the the TOCs don't bring you to the right section of the page.

Steps to reproduce

To repro:

  1. Add scroll-behavior: smooth to the element
  2. Click on anchors in TOC
  3. The page fails to scroll to the anchor

Expected behavior

Scroll to the section you clicked on in the TOC.

Actual behavior

Doesn't scroll to the section you clicked on in the TOC.

Your environment

  • Public source code: supabase/supabase
  • Public site URL: about.supabase.com
  • Docusaurus version used: 2.0.0-beta.15
  • Environment name and version (e.g. Chrome 89, Node.js 16.4): Chrome 97.0.4692.99
  • Operating system and version (e.g. Ubuntu 20.04.2 LTS): Windows

Reproducible demo

No response

Self-service

  • I'd be willing to fix this bug myself.
@yangshun yangshun added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Feb 6, 2022
@yangshun
Copy link
Contributor Author

yangshun commented Feb 6, 2022

I think the correct way would be to scrollIntoView() only if the link is not visible. Something like

function isInViewport(el) {
    const rect = el.getBoundingClientRect();
    return (
        rect.top >= 0 &&
        rect.left >= 0 &&
        rect.bottom <= (window.innerHeight || document.documentElement.clientHeight) &&
        rect.right <= (window.innerWidth || document.documentElement.clientWidth)

    );
}
if (!isInViewport(link)) {
    link.scrollIntoView({ block: 'nearest' });
}

@Josh-Cena
Copy link
Collaborator

At this point, I think we should both 1) make this configurable (but default to true because it's good for most users) and 2) apply the fix you proposed 🤔

@Josh-Cena Josh-Cena added difficulty: starter Issues that are starter difficulty level, e.g. minimal tweaking with a clear test plan. status: accepting pr This issue has been accepted, and we are looking for community contributors to implement this and removed status: needs triage This issue has not been triaged by maintainers labels Feb 6, 2022
@Josh-Cena
Copy link
Collaborator

Another thing to consider for someone working on the fix: when the page is pinch-zoomed, the TOC link still tries to scroll to itself, which results in a horizontal scroll. We should only scroll into view if the scroll is purely vertical. This would probably need the visualViewport API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution difficulty: starter Issues that are starter difficulty level, e.g. minimal tweaking with a clear test plan. status: accepting pr This issue has been accepted, and we are looking for community contributors to implement this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants