Support just-in-time measured content

bvaughn commented 6 years ago

In order to avoid adding cost to existing list and grid components, create a new variant (e.g. DynamicSizeList and DynamicSizeGrid). This variant should automatically measure its content during the commit phase.

MVP

The initial implementation of this could work similarly to how CellMeasurer works in react-virtualized:

Content is measured only if no current measurements exists.
Measurements need to be reset externally (imperatively) if something changes.
Cells at a given index can only be positioned after all cells before that index have been measured.

Goal

This component could perform better if we removed the third constraint above, allowing random access (by either item index or scroll offset) without measuring the preceding items. This would make react-window much more performant for use cases like chat applications.

This would also unlock the ability to use a ResizeObserver (via react-measure) to automatically detect item sizing and remove the position and measurements cache entirely. This would remove the need for imperatively resetting cached measurements and dramatically improve the API.

In order for the above to be possible, the dynamic list/grid components would need to use a dramatically different approach for mapping offset to index and vice versa. (This comment about "scroll anchoring" in react-virtualized has some nice visuals.) Essentially, we would need to do something like this:

Estimate total size based on the number of items multiplied by a estimatedItemSize prop. (This estimated size won't need to be adjusted, since the mapping described below doesn't is fuzzy.)
When scroll position changes, compare the new offset to the previous offset. If the delta is greater than some [to be determined] threshold, set the new offset as the "scroll anchor". Map the offset to an estimated index (e.g. divide the offset by total estimated scrollable size and multiply that by the number of items in the collection). Store this mapped index as the "anchor index". For example, if the list described by the image below had 250 items, the "anchor index" would be 132.

When scroll position changes, if the delta is less than the threshold, choose which new items to render relative to the anchor index. Position these items relative to the previously positioned items. Continuing with the example above, if the list was scrolled by a small amount (200px) then 200px worth of additional rows would need to be appended below the previously positioned items:

The above approach has only one major downside: aligning items correctly at list boundaries. If item indices are estimated (as described above) then they likely won't line up exactly with the beginning or end of the scrollable area.

The end could potentially be accounted for by adjusting the total estimated size as the user scrolls closer to the end (although this might make scrolling feel janky).
The start of the list is harder to handle, since the first item needs to align with offset zero while still appearing to connect contiguously with items from some offset greater than zero. Perhaps another threshold could be used, a "safe zone", near the start of the list (e.g. if the scroll offset is less than some absolute value) which would force the list to measure all cells up to that point so they align correctly. The cost of this forced measurement would be relatively low, since it would only be a small number of items.

The one case that would still not be handled correctly with the above approach would be a scroll anchor that is set outside of the "safe zone" but a current scroll that goes inside of the safe zone (as shown below). If the user scrolls slowly back toward the beginning of the list, it may be difficult to align the first cell with zero without introducing scroll janky.

piecyk commented 5 years ago

@bvaughn was playing around over the weekend with the idea position the items with anchor index and scrolling the list to align what user is seeing... First impressions that it could work 😅 For simplicity the list is rendered without any overscan elements, also we need change how cache is working ( in this wip it's cleared always ) ... What do you think ?

poc implementation https://github.com/piecyk/react-window/pull/2/files here is the code sandbox running the build of the branch https://codesandbox.io/s/4x1q1n6nn9

piecyk commented 5 years ago

uh-oh firefox can't keep up with rendering when fast scrolling like chrome does using this approach 🤷‍♂️

tsirlucas commented 5 years ago

I'm trying to add auto-scroll in a chatbox-style app and I'm running into this issue: DynamicSizeList does not support scrolling to items that have not yet measured. scrollToItem() was called with index 111 but the last measured item was 110.

I'm trying to call scrollToItem(items.length-1) in componentDidUpdate

Is this intended behavior?

Kinda late but I'm trying to solve that problem with this react hook.

Not the best solution but I'm still working on it. Maybe we can work to make it more generic (and simple) and create a package?

Gif to show how this is working right now: https://cl.ly/87ca5ac94deb

Maybe we can open a new issue to talk about a solution for this chat-like behaviour?

simeonoff commented 5 years ago

uh-oh firefox can't keep up with rendering when fast scrolling like chrome does using this approach

Looks like it works in Firefox Nightly version 68.

simeonoff commented 5 years ago

@bvaughn I am using the DynamicSizeList with the InfiniteLoader and Autosizer components to try to create a feed. I like what I am seeng so far, keep it up :)

In terms of API and behavior:

Can we get the data object as a second argument in the itemKey callback, the way it is in the FixedSizeList?
Can we reset the the inner container height when the data object is filtered/modified.

tony-scio commented 5 years ago

I've been using DynamicSizeList from 1.6.0-alpha.1 with InfiniteLoader for a while now and it has been working great. Thanks for the awesome feature!

However, now I have a requirement to use scrollToItem(). Similar to an above comment, but distinct because nothing at all is measured yet, I get:

DynamicSizeList does not support scrolling to items that yave not yet measured. scrollToItem() was called with index 9 but the last measured item was -1.

It doesn't appear to be timing related as I tried calling it after a long setTimeout. So, can I force a measurement? Or are there any workarounds?

EDIT: Nevermind, initialScrollOffset did what I needed.

p2227 commented 5 years ago

why use findDOMNode?

WillSquire commented 5 years ago

Do type definitions exist for the @next branch?

martynaskadisa commented 5 years ago

@WillSquire no, they do not. I'm also unsure if it's possible to release them at @next tag in DefinitelyTyped (I might be mistaken). Also, I think it would be better to wait till this branch is finally merged and new version released since API could still change.

brunolemos commented 5 years ago

@bvaughn I've been willing to use this at devhub, but before I invest on integrating it do you think it's already on good shape or still has some blocker issues?

bvaughn commented 5 years ago

@brunolemos My todo list on #102 is pretty accurate I think. If you've tested and feel like it's in a good shape for the browsers you're targeting, then keep me posted!

brunolemos commented 5 years ago

keep me posted!

~Tried to use it but haven't succeed so far.~

Got it working but performance is not good. Maybe my item renders are expensive or other thing.

https://github.com/devhubapp/devhub/pull/152

mmoyles87 commented 5 years ago

It looks like something may have broke in the latest version when using AutoSizer with a container height of 100%

Edit: I was able to find a workaround using 100vh and calc()

bvaughn commented 5 years ago

something may have broke

Too vague to be actionable. Share a repro along with expected behavior?

Gajesh-LH commented 5 years ago

I am using DynamicSizeList from 1.6.0-alpha.1 with different filters and some complex card view. But we need to wrap with the hight and width. It is working fine for me.

Thank you Guys

graphee-gabriel commented 5 years ago

Hello, I'm trying out DynamicSizeList, but all my items are rendered on top of each other. I do pass the style attribute to the rendered item, but if i log style, i see that height is undefined for each item, top and left are always 0. What did I miss here :-) ?

Bessonov commented 5 years ago

@graphee-gabriel you miss a reproduction example.

bvaughn commented 5 years ago

Height is intentionally not specified initially, in order to let the items render with their natural size...

graphee-gabriel commented 5 years ago

Ok I do understand the logic, then this is not the reason the items are rendering on top of each other. I will provide as much data as I can, I thought that maybe i made a common mistake or missed an obvious step that leads to this issue (such as not passing down the style object).

import React from 'react'
import { DynamicSizeList as List } from 'react-window'

import Layout from '../Layout'
import Text from '../Text'
import withScreenDimensions from '../withScreenDimensions'

class ListView extends React.Component {
  state = {
    availableHeight: 0
  }

  componentDidMount() {
    const checkForHeightChange = () => {
      if (this.containerDiv) {
        const { offsetHeight } = this.containerDiv
        if (this.offsetHeight !== offsetHeight) {
          this.offsetHeight = offsetHeight
          this.setState({ availableHeight: offsetHeight })
        }
      }
    }
    checkForHeightChange()
    this.intervalId = setInterval(checkForHeightChange, 10)
  }

  componentWillUnmount() {
    clearInterval(this.intervalId)
  }

  render() {
    const {
      data,
      renderItem,
      emptyText,
      dimensions,
    } = this.props
    const { width } = dimensions
    const { availableHeight } = this.state
    return (
      <Layout
        centerVertical
        style={{
          height: '100%',
        }}>
        {data && data.length > 0 ? (
          <div
            style={{
              display: 'flex',
              flex: 1,
              height: '100%',
              backgroundColor: 'red',
              alignItems: 'stretch',
              justifyContent: 'stretch',
            }}
            ref={ref => this.containerDiv = ref}
          >
            <List
              height={availableHeight}
              itemCount={data.length}
              width={width > 800 ? 800 : width}
            >
              {({ index, style }) => {
                console.log('style', style)
                return (
                  <div style={style}>
                    {renderItem({
                      item: data[index], index
                    })}
                  </div>
                )
              }}
            </List>
          </div>
        ) : (
            <Text bold center padding accent>{emptyText}</Text>
          )}
      </Layout>
    )
  }
}

export default withScreenDimensions(ListView)

bvaughn commented 5 years ago

Share a Code Sandbox where I can see this?

I would suggest not using an inline function for your item renderer because it causes unnecessary remounts due to the fact that it changes each time the parent is rendered. (This is why all of the examples avoid inline functions.)

brunolemos commented 5 years ago

@graphee-gabriel I've had this as well, the docs don't mention it but you need to make your row component support receiving a ref:

Row = React.forwardRef(
      (row, ref) => (
        <div ref={ref} style={row.style}>
          {renderItem({
            item: data[row.index],
            index: row.index,
          })}
        </div>
      ),
    )

graphee-gabriel commented 5 years ago

Hello, here you go:

https://codesandbox.io/s/sweet-sea-irxl8?fontsize=14

Yes indeed for the inline functions, it's a really old code i tried to migrate to react-window, and didn't take the time to improve it here as well. Thanks for your feedback. Let me know what you find in this sandbox.

graphee-gabriel commented 5 years ago

Annnnnd I should have checked @brunolemos message before doing that sandbox hehe. This was the missing step, and fixed my issue, thank you!

graphee-gabriel commented 5 years ago

It raised another issue, I updated the sand box so you can reproduce @bvaughn Find it here: https://codesandbox.io/s/sweet-sea-irxl8

Now the list works fine, but weirdly it seems to grow with the content, and never shrinks again.

Meaning:

If I show 6 items, scroll until the end, everything is fine.
Change the content to show 20 items, scroll until the end, all good.
Change back the content to 6 items, scroll until the last item, but I can continue scrolling on white space that seems to be left from the previous content (the 20 item data set).

edgars-sirokovs commented 5 years ago

It raised another issue, I updated the sand box so you can reproduce @bvaughn Find it here: https://codesandbox.io/s/sweet-sea-irxl8

Now the list works fine, but weirdly it seems to grow with the content, and never shrinks again.

Meaning:

If I show 6 items, scroll until the end, everything is fine.

Change the content to show 20 items, scroll until the end, all good.

Change back the content to 6 items, scroll until the last item, but I can continue scrolling on white space that seems to be left from the previous content (the 20 item data set).

I am having the same issue

FranziAndFinn commented 5 years ago

This is exactly what I needed. I have just tested the DynamicSizeList from 1.6.0-alpha.1 in combination with react-virtualized-auto-sizer and react-window-infinite-loader with very good results. Anything I can help with to make this move forward?

graphee-gabriel commented 5 years ago

Hello @bvaughn, did you have time to check the codesandbox from https://github.com/bvaughn/react-window/issues/6#issuecomment-509213284 ?

shoNagai commented 5 years ago

Hi, I'm using DynamicSizeList for my chat app. Thanks for the awesome feature.

However, I have problems with poor performance and lots of script processing. When scrolling, CPU always rises and often reaches 100%. Do you have any ideas for solutions?

import { useChatList } from '../../../hooks/chat/useChatList';
import LoadingSpinner from '../../../utils/LoadingSpinner';

import dynamic from 'next/dynamic';
import * as React from 'react';
import AutoSizer from 'react-virtualized-auto-sizer';
// @ts-ignore
import { DynamicSizeList as List } from 'react-window';
import { Message } from 'src/app/typings';

const { useRef, useCallback } = React;

const RowItem = React.memo(
  ({ forwardedRef, style, index, data }: any) => {
    const item = data[index] as Message;
    if (item) {
      return (
        <div id={item.messageId} ref={forwardedRef} style={style}>
          {item.text && item.text.plainText}
        </div>
      );
    }
    return null;
  },
  (prevProps, newProps) => {
    const { index, data } = prevProps;
    const { index: newIndex, data: newData } = newProps;
    let isMemo = true;
    isMemo = isMemo && index === newIndex;
    isMemo = isMemo && data.length === newData.length;
    return isMemo;
  }
);

function ChatList() {
  const listRef = useRef<HTMLInputElement>();

  const { formatMessages: messages, isLoadingMessages } = useChatList();

  const keyCreator = useCallback((index: number) => `ChatList/RowItem/${messages[index].messageId}`, [messages]);

  if (isLoadingMessages && (!messages || messages.length <= 0)) {
    return <LoadingSpinner />;
  }
  return (
    <div style={{ flex: 1, height: '100%', width: '100%' }}>
      <AutoSizer>
        {({ height, width }) => (
          <List
            ref={(lref: HTMLInputElement) => {
              if (lref !== null) {
                listRef.current = lref;
              }
            }}
            itemCount={messages.length}
            itemData={messages}
            itemKey={keyCreator}
            height={height}
            width={width}
            overscanCount={10}
          >
            {React.forwardRef((props, ref) => (
              <RowItem forwardedRef={ref} {...props} />
            ))}
          </List>
        )}
      </AutoSizer>
    </div>
  );
}

export default ChatList;

alex-worker commented 5 years ago

Hello @bvaughn, nice work! What is correct way to recalculate item heights on resizing workplace ? With help of comments above i'm make some demo https://codesandbox.io/s/angry-hill-tcy2m When width of workplace changing by mouse, I need recalculating all item height ( and may be clearing internal height cache of VariableSizeList component ) ...

ShivamJoker commented 5 years ago

is there any solution now for dynamic list i just spent 4 hours trying to get it working huh after reading all the comments i am done 😠 If you guys have any working solution please provide me

bvaughn commented 5 years ago

There is no "you guys" on this project. It's maintained by a single person.

And it's kind of inconsiderate to leave comments on multiple issues in the same day complaining about the same thing. By all means, please just use react-virtualized.

ShivamJoker commented 5 years ago

I am really sorry for leaving comment like this brian. I found this issue later and then read all the comments and I thought others are helping you atleast

graphee-gabriel commented 5 years ago

@ShivamJoker I nearly got it to work, the only thing left for it to be perfectly working is the total height that keeps growing but never shrinks after changing the content to less items. If this gets solved, in my use case it would seem to work perfectly.

@bvaughn would you have time to check the example in the sandbox from https://github.com/bvaughn/react-window/issues/6#issuecomment-509213284 ?

=> https://codesandbox.io/s/sweet-sea-irxl8

It starts with the shorter list, therefore:

scroll down to the end of the list
scroll back up and click on Show Long List
scroll down to the end
scroll back up and click on Show Short List
Finally scroll down one last time to see that the short list is now followed by a lot of blank spaces.

Thank you!

simeonoff commented 5 years ago

@ShivamJoker I nearly got it to work, the only thing left for it to be perfectly working is the total height that keeps growing but never shrinks after changing the content to less items. If this gets solved, in my use case it would seem to work perfectly.

@bvaughn would you have time to check the example in the sandbox from #6 (comment) ?

=> https://codesandbox.io/s/sweet-sea-irxl8

It starts with the shorter list, therefore:

scroll down to the end of the list

scroll back up and click on Show Long List

scroll down to the end

scroll back up and click on Show Short List

Finally scroll down one last time to see that the short list is now followed by a lot of blank spaces.

Thank you!

Yeah, basically the inner container should reset it's height upon data filtering/change.

ShivamJoker commented 5 years ago

@ShivamJoker I nearly got it to work, the only thing left for it to be perfectly working is the total height that keeps growing but never shrinks after changing the content to less items. If this gets solved, in my use case it would seem to work perfectly.

@bvaughn would you have time to check the example in the sandbox from #6 (comment) ?

=> https://codesandbox.io/s/sweet-sea-irxl8

It starts with the shorter list, therefore:

scroll down to the end of the list

scroll back up and click on Show Long List

scroll down to the end

scroll back up and click on Show Short List

Finally scroll down one last time to see that the short list is now followed by a lot of blank spaces.

Thank you!

thanks i have implemented the same hoping it to work fine for other users also and yes i also saw that the height issue with scroll

ShivamJoker commented 5 years ago

@graphee-gabriel but I am having also one more issue my app bar used to hide on scroll down which is not happening now what can be the solution ?

divate commented 5 years ago

Good day. I used DinamycSizeList in my demo project. Everything was fine, but after a while I noticed that the list component began to work incorrectly. At first I thought it was some library on which react-window depends. But looking at your demo https://react-window-next.now.sh/#/examples/list/dynamic-size noticed about the same result, although in the past it also worked perfectly. Could you suggest the cause of these bugs?

ShivamJoker commented 5 years ago

@simeonoff

Yeah, basically the inner container should reset it's height upon data filtering/change.

how should i do it i tried putting a state height but it doesnt seems to work any solution ?

simeonoff commented 4 years ago

@simeonoff

Yeah, basically the inner container should reset it's height upon data filtering/change.

how should i do it i tried putting a state height but it doesnt seems to work any solution ?

It must be implemented internally by the library.

mohemohe commented 4 years ago

@ShivamJoker I nearly got it to work, the only thing left for it to be perfectly working is the total height that keeps growing but never shrinks after changing the content to less items. If this gets solved, in my use case it would seem to work perfectly.

@bvaughn would you have time to check the example in the sandbox from #6 (comment) ?

=> https://codesandbox.io/s/sweet-sea-irxl8

It starts with the shorter list, therefore:

scroll down to the end of the list

scroll back up and click on Show Long List

scroll down to the end

scroll back up and click on Show Short List

Finally scroll down one last time to see that the short list is now followed by a lot of blank spaces.

Thank you!

You can set key to prevent blank spaces. https://codesandbox.io/s/blissful-voice-mzjsc

Sauco82 commented 4 years ago

Hello @bvaughn thank you for this awesome project!

Out of need I have been spending quite some time in DynamicLists code trying to figure out how to scroll anywhere without any limitations and how scroll to any item, measured and not.

I managed to do so here https://github.com/bvaughn/react-window/compare/issues/6...Sauco82:issues/6 by getting rid of lastMeasuredIndex and checking per item instead with itemIsMeasured, which obviously in turn requires of many other changes.

I have never participated in an open source project nor used Flow so I'm not quite sure if this is useful or worth to include, nor if I should directly try a PR or discuss it here.

I'm happy to help and tweak, so if you need anything please let me know.

Kashkovsky commented 4 years ago

Hey, here's much simpler solution (no need to append ghost elements to DOM to calculate their size): Codesandbox

josh-axy commented 4 years ago

Hey, here's much simpler solution (no need to append ghost elements to DOM to calculate their size): Codesandbox

Awesome! It works for me. Nice job.

dmitryshelomanov commented 4 years ago

@Kashkovsky calculated size if was update and itemSize call once

kirill-konshin commented 4 years ago

@userbq201 @Kashkovsky great workaround! Somehow in my case it did not work out of the box, I had to modify the code of ChatHistory.js like so:

    const listRef = useRef(); // added
    const sizeMap = useRef({});
    const setSize = useCallback((index, size) => {
        sizeMap.current = {...sizeMap.current, [index]: size};
        listRef.current.resetAfterIndex(index); // added
    }, []);
    const getSize = useCallback(index => sizeMap.current[index] || 60, []);
    // ...
    <List ref={listRef}

Otherwise all items are rendered with the same default height. In your case it works as is, I can't explain why there's a difference... maybe @bvaughn can.

pathikdevani commented 4 years ago

@bvaughn what if we have to enable similar behaviour for SSR? any hint for that side?

roelzkie15 commented 4 years ago

@bvaughn It is me or your demo is not working?

willsmanley commented 4 years ago

Nice implementation @Kashkovsky @kirill-konshin and @userbq201 !

How would you approach using memoization with this solution?

I tried wrapping ChatMessage in a memo with areEqual, but React still re-renders each object every time the a message is added to the list.

In my other FixedSizedLists, the memoization works fine with that memo/areEqual wrapper, but maybe the ref={root} is affecting it?

You can easily tell if React is re-rendering the components by sticking a

I forked the example here: https://codesandbox.io/s/dynamic-size-of-react-window-list-items-nb038

EDIT: I fixed the memoization - it is meant to wrap the initial functional component with memo - not the as I had done. Here is a memoized solution in case anyone has a complex DOM structure inside their lists (or if they have to map videos as I do): https://codesandbox.io/s/dynamic-size-of-react-window-list-items-errt4

EDIT 2: It turns out that the initial rendering of the rows is completely fine, but calling resetAfterIndex is very difficult to manage. My data changes (for instance when the user selects a different conversation). But the real problem is that resetAfterIndex seems to run before the new setSize can finish. So it does successfully clear the cached style, but then it just generates a new cache of the old styles again because the browser hasn't finished re-sizing the content.

Not sure if that makes sense in writing but I have to put a pin in this one for now. Please let me know if anyone is successfully changing their content dynamically and maintaining updated height styles.

I have lots of other react-window implementations so I would really prefer to find a good solution here instead of using react-virtualized for this one use case.

EDIT 3: This will be my final edit since I have finally come up with a semi-elegant solution.

1.) If the itemData needs to change (i.e. when the conversation is changing), unmount the entire component and then re-mount it with the new itemData. This can be done using a setState call back. This ensures that old height styles are not carried over when you change your data.

2.) I passed {sizeMap, setSize, listRef} to ChatMessage via ChatContext. That way, instead of just setting the size blindly, I can tell ChatMessage to set the size AND compare it to the old size. If the old size is different from the new size, then call a resetAfterIndex starting at index and passing true to force update.

So my new ChatMessage looks something like this:

const ChatMessage = ({ message, index }) => {
    const { setSize, sizeMap, listRef } = useContext(ChatContext);
    const root = React.useRef();
    useEffect(() => {
        let oldSize = sizeMap[index];
        let newSize = root.current.getBoundingClientRect().height;
        setSize(index, newSize);
        if(newSize !== oldSize){
            listRef.current.resetAfterIndex(index,true);
        }
    }, [sizeMap, setSize, listRef]);

    return (
        <div ref={root}
             {message.body}
        </div>
    );
};
export default ChatMessage;

3.) I added what is basically a listener in the component that waits for the newly mounted to be rendered. Once rendered, then it scrolls to the bottom. This fixes the problem with trying to put the scrollTo function directly in componentDidMount because it's never rendered prior to that method being called. So looks like this:

class Chat extends Component {
    constructor(props) {
        super(props);
        this.listRef = createRef();
        this.state = {
            initialScrollComplete: false,
            interval: null
        };
    }

    componentDidMount(){
        // Create interval to check if list is ready. Once ready, scroll to bottom of list.
        this.setState({interval: setInterval(()=>{
            if(this.listRef.current && !this.state.initialScrollComplete){
                this.listRef.current.scrollToItem(this.props.chatHistory.length - 1, "end");
                this.setState({initialScrollComplete: true});
            }
        },25)});
    }

    componentDidUpdate(prevProps, prevState) {

        // Clear interval if scroll has been completed
        if(!prevState.initialScrollComplete && this.state.initialScrollComplete){
            clearInterval(this.state.interval);
            this.setState({interval: null});
        }

        // If new message is transmitted, scroll to bottom
        if(prevProps.chatHistory && this.props.chatHistory && prevProps.chatHistory.length !== this.props.chatHistory.length){
            this.listRef.current.scrollToItem(this.props.chatHistory.length - 1, "end");
        }

    }

    render() {
        return <ChatHistory listRef={this.listRef} chatHistory={this.props.chatHistory}/>;
    }
}

tastyqbit commented 4 years ago

There seems to be a really weird behaviour on safari.

The height of the DynamicSizedList div and the height or margin-top of the rows does not work correctly content starts to overlap.

When window width/content of the row changes, the new height is not recalculated or changed to work with the new height of the content.

Something that seems to fix it is if the content is scrollable, scrolling to a point where the row is not visible anymore, scrolling back up to it makes it render correctly.

The content does not render correctly on the first page load so refreshing doesn't seem to fix the problem :/

It all works absolutely fine on other browsers including chrome and firefox. Safari is aweful but unfortunately the app still needs to work for people that use it.

I could really use some help!

afreix commented 4 years ago

@tastyqbit I also ran into the same issue on Safari and Internet Explorer. As you mentioned, scrolling past the expanded row and then scrolling back allows it render correctly, but of course that doesn't really solve the problem.

If you find any workarounds please let me know!

bvaughn / react-window

Support just-in-time measured content #6

MVP

Goal