Skip to content

Introduce Option to control Scroll Speed #3076

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

nheinbaugh
Copy link

Description

My company is wanting to use gridstack, but early user testing revealed frustration with how fast the grid can be scrolled when in large dashboards. Instead of trying to locally patch the behavior we wanted to contribute to the library.

Checklist

  • Created tests which fail without the change (if possible) (NOTE: new tests were added since the only tests in the repo were "bogus tests". This required refactoring the logic to more easily test)
  • All tests passing (yarn test) (NOTE: all tests were written against the original logic before any scroll speed changes were added in order to preserve existing behavior)
  • Extended the README / documentation, if necessary

@adumesny
Copy link
Member

adumesny commented Jun 20, 2025

... instead of trying to locally patch the behavior we wanted to contribute to the library.

thank you for doing the right thing. I'll take a look this weekend when I have time.

Will need to see why larger grids would be worse and if we can't fix this generally without a maxSpeed which is hard to gage for users. Granted mouse scroll wheel usually have a speed setting as well, but I woud like it work better out of the box.

@nheinbaugh
Copy link
Author

Alain: I think that the fact that the scroll speed is zoomier in large grids has nothing to do with the code and more to do with human behavior. When users are moving an item from the top of a large grid to the bottom they move their mouse really fast and the code takes that distance into account (the distance calculated being the top of the item compared to the pointer location). After it happens once it just puts users into a place where they expect it to happen.

It's kinda interesting to play around with; that's why I chose to also include a new demo page. Even if you decide it isn't something you'd like to have a demo for it is really useful to mess with and try to see the change in behavior. I felt like ~40 was a good baseline speed, anything less than that felt too sluggish.

I understand the hesitation about introducing a new option for speed and having it as just an integer. Since this was completely unsolicited I didn't want to start with more involved solutions, but I also considered making it a numberOrString and having the unconstrained number for "expert" users and a couple of friendly names like:

type ScrollSpeeds = number | 'fast' | 'medium' | 'slow';

That sorta solves the problem of making it more approachable, but it doesn't resolve your (really good) point that the number values 5 or 40 or 100 are impossible to understand without playing with it. If you want to try and dig into the code to understand what the numbers correlate to.... go for it :) Even understanding what the value was calculated from didn't help me too much. I would need a better understanding of how often that method is called.

As I was typing this long essay (sorry, I do ramble) I realized that maybe one way to think about scrolling speed is to have it be anchored to the size of rows. You likely don't want to be scrolling much faster than rows can appear (and my row height is ~40 iirc) so that might work

nheinbaugh pushed a commit to time-loop/gridstack.js that referenced this pull request Jun 25, 2025
@nheinbaugh nheinbaugh mentioned this pull request Jun 25, 2025
3 tasks
nheinbaugh added a commit to time-loop/gridstack.js that referenced this pull request Jun 25, 2025
Co-authored-by: Nick Heinbaugh <nheinbaugh@clickup.com>
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.

2 participants