Skip to content
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

Fix smooth scrolling + add scroll on reply click #1326

Merged
merged 2 commits into from
Apr 6, 2023
Merged

Conversation

gdbroman
Copy link
Contributor

@gdbroman gdbroman commented Apr 4, 2023

Demo

CleanShot.2023-04-04.at.21.11.32-converted.mp4

Explanation

react-virtualized (which I based our WindowedList component on) is fucked. It doesn’t support our use case with chat. I got smooth bottom-up scrolling working with CSS hackery, but not together with programmatic scroll to arbitrary row. The bottom line is that the react-virtualized algorithm is fundamentally a mismatch for the chat use-case, it was built for grids. The author even testified to this and that he won’t have time to fix it (issue still open).

Luckily there is another virtualization library built for chat, which out of the box supports:

  1. Bottom-up scrolling
  2. Variable-sized rows without manual re-measure
  3. Programmatic scroll to arbitrary row
  4. It is also actively maintained and has excellent docs

It’s called react-virtuoso and I’ve implemented it in this PR. Long term we might want to rebuild a new WindowedList based on it, but I can’t justify the time spend for it now.

All in all, scrolling UX is excellent now.


Side note: I've left the scrollbar visible on all lists. I thought it was helpful UI, but if you think it's ugly @drunkplato it's an easy removal.

@gdbroman gdbroman requested a review from drunkplato April 4, 2023 20:31
@gdbroman gdbroman linked an issue Apr 4, 2023 that may be closed by this pull request
44 tasks
@gdbroman gdbroman force-pushed the virtuoso branch 3 times, most recently from c637665 to a86f44e Compare April 4, 2023 20:56
@gdbroman gdbroman mentioned this pull request Apr 5, 2023
44 tasks
Copy link
Contributor

@drunkplato drunkplato left a comment

Choose a reason for hiding this comment

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

I think leaving the scrollbar is fine as long as we can make the style of the scrollbar consistent across windows, mac, and linux.

  • The track should be fully transparent.
  • The scrollbar should be the Mac like pill as it currently appears.
  • The scrollbar should darken slightly on hover.
  • We should offset the scrollbar to appear a few pixels more to the right. Maybe translate the x position?
  • Also noticed the footer needs more top padding (before it was fine)
    • image
    • image
/* width */
::-webkit-scrollbar {
  
}

/* Track */
::-webkit-scrollbar-track {
  
}

/* Handle */
::-webkit-scrollbar-thumb {
  
}

/* Handle on hover */
::-webkit-scrollbar-thumb:hover {
  
}

@gdbroman
Copy link
Contributor Author

gdbroman commented Apr 5, 2023

We got a custom scrollbar now @drunkplato

CleanShot.2023-04-05.at.23.46.35.mp4

@gdbroman gdbroman requested a review from drunkplato April 5, 2023 21:51
@drunkplato drunkplato merged commit 9896da1 into master Apr 6, 2023
@drunkplato drunkplato deleted the virtuoso branch April 6, 2023 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New chat TODO list
2 participants