-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[docs] Improve UX with back to top #32896
[docs] Improve UX with back to top #32896
Conversation
import useScrollTrigger from '@mui/material/useScrollTrigger'; | ||
import Box from '@mui/material/Box'; | ||
import Fab from '@mui/material/Fab'; | ||
import Tooltip from '@mui/material/Tooltip'; | ||
import KeyboardArrowUpRoundedIcon from '@mui/icons-material/KeyboardArrowUpRounded'; | ||
import Zoom from '@mui/material/Zoom'; | ||
import Fade from '@mui/material/Fade'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace the Zoom with a Fade animation to fix the broken tooltip animation and to reduce the visual distraction when scrolling
sx={[ | ||
{ position: 'fixed', bottom: 24, right: 24, zIndex: 10 }, | ||
...(Array.isArray(sx) ? sx : [sx]), | ||
]} | ||
> | ||
<Tooltip title="Scroll to top"> | ||
<Fab | ||
color="primary" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change the color of the Fab color to a less distracting color, primary to grey
@@ -248,7 +247,6 @@ export default function MyApp(props) { | |||
|
|||
return ( | |||
<AppWrapper emotionCache={emotionCache} pageProps={pageProps}> | |||
<BackToTop /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move the component to only be visible on the docs pages, having it in https://mui.com/material-ui/getting-started/templates/blog/ feels deceptive, it's not what the template includes
data-ga-event-category="docs" | ||
data-ga-event-action="click-back-to-top" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add ga event to see if it's used at scale (>0.01%)
window.scrollTo({ top: 0, behavior: 'smooth' }); | ||
onClick?.(event); | ||
const handleClick = () => { | ||
window.scrollTo({ top: 0 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pain point this button solve is being able to quickly move back to the top. I assume that making the scroll to top transition instant is helping to better solve the problem.
a839a13
to
5693f06
Compare
I like it! 👍 This will make a big difference on massive pages like the migration guides! |
@samuelsycamore Thanks for the feedback, let's wait for Danilo to come back from holiday, it's ultimately about UX, so its core area of ownership. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed just a couple of styling adjustments-other than that, looking great! Good improvements!
Rebased on HEAD |
3766305
to
c447d77
Compare
@danilo-leal Perfect, I have pushed two last commits:
Before Screen.Recording.2022-06-02.at.11.44.29.movAfter Screen.Recording.2022-06-02.at.11.44.07.mov |
This is an attempt to iterate on #30441 (comment)
To compare the UX: