Add a scroll to top bottom #77
Conversation
| window.scrollTo({top: 0, behavior: 'smooth'}); | ||
| }; | ||
|
|
||
| window.addEventListener('scroll', checkScrollTop); |
There was a problem hiding this comment.
Can we put the addEventListener into a useEffect? here are two examples:
https://gist.github.com/JoaoTMDias/b5aeb6e02be7e6d25d5fd7f10826fc8a
There was a problem hiding this comment.
yea , I will use useEffect to do that.
| return ( | ||
| <div className="scroll-to-top" onClick={scrollToTop}> | ||
| {showScroll && | ||
| <div > |
There was a problem hiding this comment.
nit - fix formatting and indentation
There was a problem hiding this comment.
I am not sure how to format here
There was a problem hiding this comment.
Instead of this
<div>
<Fab color="primary" aria-label="scroll back to top">
<ArrowUpIcon fontSize="large" />
</Fab>
</div>
more like this
<div>
<Fab color="primary" aria-label="scroll back to top">
<ArrowUpIcon fontSize="large" />
</Fab>
</div>
You can also run the command npm run lint:fix to format everything
| import "./ScrollToTop.scss"; | ||
|
|
||
| const ScrollArrow = () =>{ | ||
|
|
src/components/App.jsx
Outdated
| import './App.scss'; | ||
|
|
||
|
|
|
Is it ok that I push the button in the middle of the page? |
|
Instead of the middle, is it possible to keep it at the bottom, but above the other buttons? |
to prevent overlapping on the smallest screen, the lowest position is here. I am not sure it's prettier than the middle. |
Maybe I can add some responsive CSS, let me try |
| width: 56px; | ||
| height: 56px; | ||
| bottom: 22%; | ||
| right: 5%; |
There was a problem hiding this comment.
media query is great! I played with the numbers and I think these make the position a little lower and centered above the other button. Can you try this?
bottom: 11%;
right: 7.5%
There was a problem hiding this comment.
sure, but I can't do it today. I will fix them tmr.
| import ArrowUpIcon from '@material-ui/icons/ArrowUpward'; | ||
| import './ScrollToTop.scss'; | ||
|
|
||
| const ScrollArrow = () => { |
There was a problem hiding this comment.
could we rename to ScrollToTopButton?
| const ScrollArrow = () => { | ||
| const [showScroll, setShowScroll] = useState(false); | ||
|
|
||
| const checkScrollTop = () => { |
There was a problem hiding this comment.
could we rename to getShouldShowButton?
|
Thanks @tianlangwu ! |
Fixed #76
Added a scrollToTop component with
ScrollToTop.jsxandScrollToTop.scss.Imported and rendered ScrollToTop component in App component.
Let me know if I did anything incorrectly. Thanks for your consideration.