fix: remove sidebar scroll behaviour #3383
Conversation
|
CLA Assistant Lite All Contributors have signed the CLA. |
| @@ -85,7 +85,7 @@ const SafeListItem = ({ | |||
|
|
|||
| useEffect(() => { | |||
| if (isCurrentSafe && shouldScrollToSafe) { | |||
| safeRef?.current?.scrollIntoView({ behavior: 'smooth' }) | |||
| safeRef?.current?.scrollIntoView() | |||
There was a problem hiding this comment.
You can still pass block: 'center' for better positioning.
There was a problem hiding this comment.
But is it not then at the top of the page?
There was a problem hiding this comment.
block: 'center' is supposed to keep the viewport where it is if the item is already visible.
The default is block: start, and it will always scroll the page so that the item is on top of it.
There was a problem hiding this comment.
Need to experiment though, I'm not 100% sure it will be better.
There was a problem hiding this comment.
I can happily change it. What are your thoughts @liliiaorlenko? Open the sidebar so the currently open Safe is at the top of the list or in the middle?
There was a problem hiding this comment.
I think In a middle would be the best (to not mess with the order?)
There was a problem hiding this comment.
I will alter it now. (It doesn't change the order @liliiaorlenko, just the scroll position)
ESLint Summary View Full Report
Report generated by eslint-plus-action |
Pull Request Test Coverage Report for Build 1771964391
💛 - Coveralls |
|
E2E Tests Failed Failed tests:
|
|
Looks good to me. |
What it solves
Auto-scroll in long sidebars.
How this PR fixes it
The scrollbar no longer scrolls to the Safe when opening the sidebar. Instead, positioning the Safe in the middle of the sidebar.
How to test it
Screenshot