search

ampproject / amp-react-prototype

A scratch pad to experiment with React rendered AMP Components
Apache License 2.0
36 stars 9 forks source link

Scroll events and component state #41

Open dvoytenko opened 4 years ago

dvoytenko commented 4 years ago

Context: https://github.com/ampproject/amp-react-prototype/blob/c4c7373542f30feda4a1b539db1781f88bdc5063/src/amp-react-carousel.js#L65

Scroll event management is difficult for carousels, especially with smooth scrolling: the container can scroll slowly from slide to slide and at some point we need to update the state to the new "current" slide. Nuances:

  1. The scroll events are generally aligned with rAF so if we update state on each scroll event, we will see a significant number of re-renderings.
  2. Waiting for scrolling to end is complicated and often introduces some lag. It can also introduce some possibility of lost sync. This could be improved a lot with the new scrollend event.
  3. We can throttle scroll events and that'd generally be acceptable performance-wise.
  4. Smooth scrolling introduces another nuance: with single-pass rendering, onScroll recalculates the current slide and updates the component's state. In turn, the state change issues a side-effect to update the container's scrollLeft. This is a weird cyclical (and possibly buggy) behavior. All-in-all it might be better to just implement the scrollend.
dvoytenko commented 4 years ago

The recommendation: useScrollEndEffect(ref, callback, deps). Maybe also the API could have thresholds for <>0.5 and such.

dvoytenko commented 4 years ago

/cc @jridgewell updated the recommendation ^^^.