Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

fix: remove sidebar scroll behaviour #3383

Merged
merged 2 commits into from
Jan 31, 2022
Merged

fix: remove sidebar scroll behaviour #3383

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0981b38
fix: remove sidebar scroll behaviour
iamacook Jan 28, 2022
9371e96
fix: scroll to center
iamacook Jan 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/SafeListSidebar/SafeList/SafeListItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ const SafeListItem = ({

useEffect(() => {
if (isCurrentSafe && shouldScrollToSafe) {
safeRef?.current?.scrollIntoView({ behavior: 'smooth' })
safeRef?.current?.scrollIntoView()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can still pass block: 'center' for better positioning.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But is it not then at the top of the page?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to experiment though, I'm not 100% sure it will be better.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think In a middle would be the best (to not mess with the order?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will alter it now. (It doesn't change the order @liliiaorlenko, just the scroll position)

}
}, [isCurrentSafe, shouldScrollToSafe])

Expand Down