bvaughn / react-window

React components for efficiently rendering large lists and tabular data
https://react-window.now.sh/
MIT License
15.82k stars 785 forks source link

use scrollEnd event to detect scrollend where possible #760

Closed heikomat closed 1 month ago

heikomat commented 6 months ago

Overview

On every scroll event, animationFrame-based timers are installed and uninstalled to find the moment the user stopped scrolling, so pointer-events: none can be removed.

This installing and uninstalling of animationFrame-callbacks costs time. Not that much, but measurable on slow devices, and for most modern browsers, a better alternative exists: The scrollend-event. (also see this chrome developers blogpost)

This PR aims to replace the timer with the scrollEnd-event where possible.


Changed default

To not break existing custom OuterElements, that onScrollEnd is only supplied and used when the user sets the useScrollEndEvent-property and the browser supports the event. If that property is set however, the user has to call the onScrollEnd when appropriate (just like the onScroll-event)

The way it's currently implemented, these changes switch the default scroll-end detection (for non-custom outer-elements) to the scrollend-event if possible. If that is to drastic, we could change it so that the scrollEnd event is only ever used if useScrollEnd === true.


About this PR

I'm not to deep in the codebase, so some details of the implementation might not be optimal and i would be thankful for feedback.

I will update the docs soon, which is why this is a draft-pr for now, but the code is done as long as there is no feedback. I will still try to update the docs soon, but the setup for the website requirs some older versions of node and some dependencies that are not in the package.json, so updating the docs isn't as simple as i thought.

There is also at least one edge-cases that is not covered (yet!): onScroll being called without the user scrolling. For example when the user has scrolled far down and then the number of items gets reduced. This seemingly triggers a scroll, but no scrollEnd (reminder for myself: maybe the event has info if it was triggered by the mouse/wheel/touch. if that is absent, maybe don't set isScrolling to true)


Performance

Here is scrolling through a extreme simple 5000 items list: (chrome with 6x slowdown on M2 Max)

import { FixedSizeList as List } from "react-window";

const Row = ({ index, style }) => (
  <div style={style}>
    Row {index}
  </div>
);

function App() {
  return (
    <List
      height={300}
      itemCount={5000}
      itemSize={35}
      width={300}
    >
      {Row}
    </List>
  )
}

export default App
with animationframe-timer with scrollend-event
full_old full_new
details_old details_new
heikomat commented 6 months ago

@bvaughn sorry to bother you, but is using the scrollend-event something you'd consider supporting? If so, is there anything i can do to improve the PR?

bvaughn commented 6 months ago

I don't really have the bandwidth to support this library right now. (There are hundreds of issues and it's just a pro-bono side project.) I've learned from experience that making one-off releases for things like this can often lead to regressions or an influx of new bug reports too, so I'm pretty wary to do anything with this at the moment. Sorry :(

heikomat commented 6 months ago

No need to be sorry @bvaughn, that is totally understandable! This feature is not that important and i'm thankful for what you've done for the react ecosystem :) Wish you a nice day

bvaughn commented 6 months ago

Thank you for being so understanding, and for putting together the PR/suggestion. I really do appreciate both.

Take care~