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.

Sign up for GitHub

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

Jump to bottom

Max Wait on _.debounce #166

Closed
gianalvarezblanes opened this issue Apr 17, 2018 · 2 comments
Closed

Max Wait on _.debounce #166

gianalvarezblanes opened this issue Apr 17, 2018 · 2 comments

Comments

@gianalvarezblanes
Copy link

Due to the _.debounce of the recalculate function, the scrollbar can get into a state where the scroller doesn't update and you see a "blank" vertical bar. This happens because there is no "maxWait". If for some reason recalculate is triggered more often than the debounce interval, the function call will not fire until everything is done. This can lead to a pretty poor user experience.

The particular workflow I saw this was a "lazy" loading/display scenario. We are iterating over a large list of records and displaying in "batch" 10-25 records at a time at a fairly quick interval/rate. Each iteration happens in a timeout to give some time back to the browser (you can probably reproduce with a debounce as well).

We've locally modified the library to simply add a maxWait parameter to the debounce (same value as the debounce interval) so that the function call doesn't go beyond 100ms. It fixed our issue.

Another workaround is to make sure your lazy load interval is longer than the debounce interval, but depending on the amount of items being added it gets hard to give a good "smooth" scroll experience.

Recommend either adding by default a max wait time, or allow us to configure one.
@Grsmto
Copy link
Owner

Grsmto commented Apr 19, 2018

It seems like we are talking about throttle here! Maybe we should use it instead of debounce actually.
Your explanation makes perfect sense, I'll take a look into this!

@gianalvarezblanes
Copy link
Author

Yes you are correct. They are equivalent.

Thanks a lot!

PS: Great library btw. Thanks for maintaining it and addressing comments/issues!

@Grsmto Grsmto closed this as completed in b52e5d5 Jul 14, 2018
Grsmto pushed a commit that referenced this issue Jul 14, 2018
Merge branch 'next'
81ed171 
* next: (72 commits)
  Add normal "horizontal" exemple
  Fix rtl test not working anymore after switch to "translate" instead of "left"
  Switch from debounce to throttle so method is called at regular time (fix #166)
  Fix horizontal scrollbar not positionned properly in RTL mode
  Make sure to position scrollbar properly on init
  Update Babel and Rollup to latest versions
  Fix implementation of RAQ not performance efficient
  Add sourcemap generation to Rollup
  Do not call showScrollbar() when scrollbar already visible (avoids adding/cancelling settimeout on every scroll event)
  Use RAQ when animating for performances optimisation
  Use translate3d instead of top/left to avoid paint flashing
  Switch from bind to transform class properties syntax
  Add support for scroll on track (thanks Facebook Messenger for the trick :) )
  Add Bable transform-class-properties plugin so we can use the () => {} form and don't have to .bind() everything (easier for events adding/removal)
  Add Babel-eslint
  Lerna ≠ npm link!!!!!
  Travis plz
  Let try to update Yarn on Travis and Babel to latest version (build still fails...)
  Rename simplebar-react main output file
  Try a different way of importing modules as Travis doesn't find them
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Grsmto @gianalvarezblanes