react-virtualized icon indicating copy to clipboard operation
react-virtualized copied to clipboard

Two <WindowScroller>s on a page causes the second one to not render properly

Open apelsinet opened this issue 6 years ago • 9 comments

When I'm trying to render multiple lists (2 in my case) after each other, both using <AutoSizer> and <WindowScroller>, the second one does not render properly. It takes up the correct amount of space on the page, but the rows are gone except for a few at the bottom of the page. If you scroll up, a few more of them appears. This issue does not happen after resizing your window, or if you set the width of your <List> component to a static value.

See codesandbox example.

apelsinet avatar Feb 08 '19 14:02 apelsinet

@apelsinet, did you make any progress on this? I'm dealing with the same problem right now. I've noticed though that it does seem to work after once resizing the window. 🤔

virtualized

nicolaslohrer avatar Mar 28 '19 16:03 nicolaslohrer

I was having this issue on a project, and I ended up putting everything inside a single <List> and created a function to handle the height of everything, instead of having to use multiple autosizers and lists

michellemdaraujo avatar Mar 28 '19 17:03 michellemdaraujo

Nope, I ended up not using virtualised lists on pages with multiple lists unfortunately.

apelsinet avatar Mar 28 '19 19:03 apelsinet

I've done some more digging. I'm not familiar at all with the codebase, but maybe my findings still help:

WindowScroller has an updatePosition() method which measures the dom element's offset and stores it in _positionFromTop and _positionFromLeft. updatePosition() is called in componentDidMount(), but at the time the offsets are stored, the lists themselves haven't been painted yet. For the first one, that isn't a problem because _positionFromTop is correct no matter its height. But the second list does depend on what has been rendered above. So what seems to be happening is that an incorrect _positionFromTop is stored during the initial render, breaking virtualization in the second list. Resizing the window triggers updatePosition() to be called again from _onResize(), which fixes the incorrect offsets - hence the behavior demonstrated in my comment above.

Another place where updatePosition() is called is in registerChild. A (rather unsatisfying) quick fix for the problem seems to be doing something like this:

<WindowScroller>
  {({ height, registerChild, scrollTop }) => (
    <div>
      <AutoSizer disableHeight>
        {({ width }) => (
          <div ref={el => registerChild(el)}> // <-- relevant line
            <List
              autoHeight
              height={height}
              rowCount={list1.length}
              rowHeight={32}
              rowRenderer={rowRenderer1}
              scrollTop={scrollTop}
              width={width}
            />
          </div>
        )}
      </AutoSizer>
    </div>
  )}
</WindowScroller>

Note that it only works when recreating the function passed to ref on every render, i.e. ref={el => registerChild(el)} instead of ref={registerChild}. I've tweaked @apelsinet's example accordingly: https://codesandbox.io/s/jjnr1v3r3w?fontsize=14

It's obviously a hack and probably not the right long-term solution. I'm not sure at all about possible negative implications.

But at least it seems like RV isn't very far from supporting this use case. :)

nicolaslohrer avatar Mar 29 '19 13:03 nicolaslohrer

@nicolasschabram thank you so much for doing the research and supplying this solution! It's definitely better than not using RV at all 😄

apelsinet avatar Apr 02 '19 11:04 apelsinet

One more workaround that seems to work: wrapping the WindowScrollers in <div style={{height: rowCount * rowHeight}}>. Obviously that works only with fixed-height rows.

Rogach avatar Apr 30 '19 13:04 Rogach

@nicolasschabram Thank you so much for this. Hack or not, it works for my use case without any apparent implications.

dmitrygalperin avatar Sep 14 '20 19:09 dmitrygalperin

@nicolasschabram your hack works like a charm for a default window scroller. However, once you set a scrollElement (and you need to do so if you have some nested components) it stops working.

Does anyone have a solution with 2 lists on top, a window scroller, and custom scroll element?

uffou avatar Feb 01 '22 04:02 uffou

OK, I figured out what the problem was with 2 WindowScrollers and the same scrollElement provided to each one, the registerScrollListener function (utils/onScroll) has a check which prevents the adding a scroll listener.

When I commented the if statement it works perfectly. I am not really sure what this check is for, probably something to prevent too many event listeners to the same component.

export function registerScrollListener(
  component: WindowScroller,
  element: Element,
) {
  // if (
  //   !mountedInstances.some(instance => instance.props.scrollElement === element)
  // ) {
  element.addEventListener('scroll', onScrollWindow);
  // }
  mountedInstances.push(component);
}

This only works thanks to @nicolasschabram relevant line ;)

<div ref={el => registerChild(el)}> // <-- relevant line

@bvaughn First of all. Big thanks to you for your incredible work! I know in your mind we are using the lib in the wrong way but for my case (React Beautiful DND) I really need multiple lists on top of each other and can't really combine it into one. I am sure many more have this same issue so maybe someone can help with this use case.

WindowScroller is used by many in conjunction with react-window as well. So solving this use case would really be helpful to the community.

uffou avatar Feb 01 '22 05:02 uffou