feat(v2): add back to top button #4912
Conversation
|
✔️ [V2] 🔨 Explore the source changes: 382c15f 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/6101b70a1473920007c17731 😎 Browse the preview: https://deploy-preview-4912--docusaurus-2.netlify.app |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4912--docusaurus-2.netlify.app/ |
|
Size Change: +49.4 kB (+6%) 🔍 Total Size: 860 kB
|
There was a problem hiding this comment.
Some comments:
- What about using native smooth scrolling on browsers supporting it? And cancel the animation on the polyfill?
- The behavior is different from v1. Isn't it confusing to require scrolling up to show the button? This makes the new TOC less accessible (scroll up + press instead of just press)
- This comp does not depend too much on Infima, what about moving it to theme-common so that we can reuse it later in other themes?
- We should support iOS 15 new URL bar to fix iOS 15 Safari URL bar covers floating secondary nav button #4927
| const currentScroll = document.documentElement.scrollTop; | ||
|
|
||
| if (currentScroll > 0) { | ||
| requestAnimationFrame(smoothScrollToTop); |
There was a problem hiding this comment.
Is this recursion solution for Safari?
Where does it come from?
What about using the native scrollTo(0,{behavior: "smooth"}) for other browsers?
Apparently we can test for support with 'scrollBehavior' in document.documentElement.style
Also the recursion should stop if user attempts to scroll down again, otherwise the user scroll and the current animation conflict 🤪
There was a problem hiding this comment.
No, this is such cross-browser generic way to make smooth scrolling, since the call scrollTo(0,{behavior: "smooth"} with scroll-behavior does not guarantee it.
| transform: scale(1); | ||
| } | ||
|
|
||
| /* Temporary, for test on mobiles */ |
There was a problem hiding this comment.
I guess we can remove it now?
As I understand it, native smooth scrolling is rather unreliable thing. On Linux/Windows I was unable to get it to work with built-in API, so the
Yes and no. If the user wants to scroll to the top of page, then we display back-to-top button with which they can do this more easily, although end-user does not always need to scroll to the very top of a page. Anyway, I like this pattern in that the user focuses on the content (also something like is done in Material for MkDocs). I propose to leave this behavior, and if there are many dissatisfied users with this, then we will change to more traditional option.
Alright.
Since the back-to-top button is not displayed immediately on first rendering, only when scrolling up, doesn't that fix this "bug"? |
lex111
force-pushed
the
lex111/back-to-top-button
branch
from
5f1d26f
to
8b610f5
Compare
|
This button styles actually rely on Infima variables. I've also now applied new utility class |
…a bit backToTop button
Motivation
Resolves #3513 ℹ️ must be merged after #4273 approved.
To distract end user less and let them focus on the content, the back-to-top button is intentionally shown only when they scroll up.
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Preview
Related PRs
(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)