Dynamic height + scrolling up (when the upper cells aren't loaded yet)

sunabozu commented 7 years ago

How to reproduce this issue: go to https://bvaughn.github.io/react-virtualized/#/components/CellMeasurer , select dynamic height text, click on the table, press the End key and start scrolling up. You should see that the cells start "jumping" when loading the content.

I'm implementing similar set of components on my own, and I got the same problem. So I wonder if it's technically possible to render it smoothly. I suppose we could adjust the scrolling position and make it look smooth, but I can't wrap my head around it.

bvaughn commented 7 years ago

Unfortunately, I don't know a way to avoid this. In this case, we're essentially bypassing the measurements for all of the cells in the Grid and then scrolling back up evaluating them in backwards order. Depending on how realistic your default height setting for the CellMeasurerCache is...which in the case you linked to, I'd say "not very"...then things are going to jump around a bit.

I'd say this is a bug, but I don't currently know of a way to address it.

PTihomir commented 7 years ago

I was about to open an issue, when I noticed this. Something similar happens to me, but it is not only a visual issue, not just jumping around a bit... If I have a long List, and the prop scrollToIndex is set to something near the end of the list, scrolling up becomes impossible, it constantly jumps back to the selected row. Scrolling below the selected row works just fine. I use CellMeasurerCache and CellMeasurer because the items have dynamic height. If I remove the CellMeasurer part, then it works as expected ( I mean it doesn't jump back, but of course in that case the items height is wrong).

@bvaughn Can You tell from above is this related, or a separate issue? I can try to provide an example, if needed.

bvaughn commented 7 years ago

Seems related enough to stay as part of this issue, @PTihomir. If you have any bandwidth I'd welcome a hand looking into this one. I'm a bit swamped at the moment.

PTihomir commented 7 years ago

@bvaughn I tried some approaches, but as you said, it is not easy... My main idea was to somehow update the scrollTop of the scroll container when some of the rows are rendered (I was only considering List for the start) by the difference of the old and the new offset of the last rendered item. Yeah, it didn't worked, not sure why. Assuming that when I set the scrollTop manually, it disturbed the native scrolling. I can't really work more on this for some time, hopefully somebody will think of a solution.

My half-workaround for now is to make better estimations. I created a custom cache, which will take for defaultHeight a function instead of a number. My list items have types, and all the items with the same type have almost the same height. This way my estimation is much closer to the real height.

mushkab commented 7 years ago

cant upgrade virtualized because of @PTihomir issue. any estimation for solving this ?

mushkab commented 7 years ago

@bvaughn my use case is chat app. i use CellMeasurer + List + AutoSizer i scroll to bottom every time i get new message by changing scroll to index. when this happens the scroll will keep returning to bottom and wont let me keep up. it was working before in 8.11.4

bvaughn commented 7 years ago

any estimation for solving this ?

Nope. I would welcome contributions. 😄

mushkab commented 7 years ago

@bvaughn any direction for me to start ?

bvaughn commented 7 years ago

Not really. It's a complicated issue to work on.

Familiarize yourself with how Grid and CellSizeAndPositionManager work. Then familiarize yourself with the CellMeasurer flow and how it async-measures content and re-renders Grid.

arusakov commented 7 years ago

@mushkab Do you have any ideas? I'm chat app developer too.

bvaughn commented 7 years ago

If there's interest from you both on collaborating on this- I'm supportive! I'd be happy to video chat to discuss questions or ideas about approaches, etc. I just don't have the bandwidth personally right now to work on it.

mushkab commented 7 years ago

@arusakov didnt get to it at all sorry :(

wasd171 commented 7 years ago

@bvaughn is it possible to work around this issue by adding virtual InfiniteScroll, which would measure cells before "loading" them?

bvaughn commented 7 years ago

I'm afraid I don't understand what you mean.

wasd171 commented 7 years ago

@bvaughn never mind, I don't think I had a right idea. However I stumbled upon chrome scroll anchoring but couldn't make it work with RV. Am I missing something or scroll anchoring algorithm is in principle not compatible with RV rendering?

bvaughn commented 7 years ago

I don't think scroll anchoring should impact react-virtualized, except for maybe for the specific combination of CellMeasurer and WindowScroller jumping past content and then scrolling up. (Basically what this issue is about.)

babsonmatt commented 7 years ago

Encountering the same issue as @PTihomir, just wanted to provide a link to an example demonstrating the issue with attempting to scroll up when using CellMeasurer / scrollToIndex

https://codesandbox.io/s/55O0639R

bvaughn commented 7 years ago

Nice repro, @babsonmatt

gordon-rawe commented 7 years ago

I am also developing a chat app @mushkab @arusakov, any later progress you guys made? this issue is really a headache, I performed a weird trick to solve it but not perfect, my solution is to remove the scrollToIndex method and call List.scrollToRow(lastIndex) twice, which will jump to last and avoid jumping, but I am still expecting @bvaughn would bring us a greater react-virtualized, as I am more devoted to android and ios developing work, not so familiar with H5, or I want to contribute my code ^_^

gordon-rawe commented 7 years ago

@mushkab @arusakov I have finished my chat app using react-virtualized, and my situation maybe more complex, my variable content is not just pure words, it can be emoji or stickers inside words, I used the way of off-screen render calculator to calculate the height of every given content, but this may cause efficiency problem, I did some tricks to avoid most calculations and finally it is efficient enough, 1000 nodes cost time less than 50ms, I hope my experience can give u guys some help~

scf4 commented 7 years ago

Do you happen to be using that with InfiniteLoader @gordon-rawe ? I'm in a similar situation where the content is loaded as you scroll up and the height varies a lot. This combination is a real headache!

yuchi commented 7 years ago

I belive I cracked a solution. @wasd171 reference to Scroll Anchoring is totally right, and in fact I strongly believe that RV should implement scroll anchoring. Let me show you why.

Normal scrolling works by keeping a ∆y cursor which represents “how many pixel the content should be shifted upwards”. The reference for that value is usually the top of the content. In fact if you had to implement scrolling you could just change the top value of an absolutely positioned content element inside the container.

In other words we can then say that the y position of the content is simply y = ∆y.

I’ll now try to demonstrate that this simple formula works only when the content is static, and that it’s just the special case where moving the “scroll anchor” would have no effect on the overall scroll.

First of all what’s an “anchor”? You can consider it as the point in the content your are scrolling from. If you want a simple metaphor you can think of touch scrolling. When the user starts scrolling she is “touching” a vertical point in the content and moving it.

This gives us

Where

∆y is just ∆yₜ – ∆y₀,
yᴬ₀ is the y position of the anchor in the content at the start of the scrolling,
yᴬₜ is the y position of the anchor in the content at the end of scrolling,
∆yᴬ is the amount of y movement applied to the anchor by touch scrolling,
∆y₀ is the y position of the content in the container at the start of the scrolling,
∆yₜ is the y position of the content in the container at the end of the scrolling.

When the content never changes so do the y position of the anchor in the content, so yᴬ₀ – yᴬₜ = 0, which simplifyies the equation as the simpler one above. In fact you can choose any arbitrary scroll anchor position, nothing would change at all.

Dynamic Anchor Position

Since dynamic load of rows is introduced in the system the position of the anchor becomes dynamic too, and this forces us to use the more complex formula, but if we now the position of the anchor (more on this later) before, during and after scrolling we can have smooth scroll interactions also with dynamic content.

There’s a small but critical caveat here: the scroll position is not a managed state of the UI, it is independently managed by the browser and native scrolling functionality, and we have to keep it as such. This caveat means that we cannot treat the current content shift (read: scroll position) as a value we can easily replace by doing our calculations.

But this is where React Virtualized shines: we can virtualize this behaviour by applying different approaches based on the situation of the interaction and (I believe) never collide with native scroll features.

«At rest» approach

Moving the scroll position when the user is not scrolling at all has little to no performances issues.

When new heights are calculated for rows above the anchor we can simply update the scroll position too, keeping the anchor position in place.

«Scrolling» approach

This will be trickier but it’s something only React Virtualized can do. When the user is scrolling we can shift the content above the anchor position by a correction factor that adjusts for the difference between the actual scroll position and the calculated scroll position.

We can do this because we are not relying on the native vertical position algorithm for cells, placing them manually using absolutely positioned elements.

To not interfere with native scrolling behaviour we also need not to change the height of the content.

This therefore will move a part of the content outside the reachable area during normal scroll. Once the scrolling interaction has finished we can clean up by using the «At rest» approach.

Caveats

It can be tricky to correctly treat values passed to scrollToPosition when invoked during scrolling interaction. We could assume that the specified position is the correct, unadjusted one, but if it lies between the unreachable area it could lead to bad headaches since we can’t set the scrollTop value to negative values and then fix it up.

The same applies to scroll-to-top gestures that happen concurrently to row loading/height changes.

Anchor Position

Another challenge is how to calculate the position of the anchor. I propose a mock API here.

A new anchorBehaviour prop must be introduced, valid values are:

top, the first fully visible row is treated as the anchor and its position is the anchor position;
top-partial, the first even partially visible row is treated as the anchor;
middle, the row which is nearest the middle is treated as the anchor;
bottom, bottom-partial the last visible row (either fully or partially visible) is treated as the anchor;
proportional, when at the top of the scrolling content treat the prop as top, at the end as bottom, in the values between interpolate the values;
x%, treat the row which is under the x% of the visible area as the anchor point;
a function responsible for the calculation of the position (see below).

To the custom function the following values are passed: isScrolling, startIndex, stopIndex, rowCount, scrollTop, scrollHeight, list. The function will return the y position to be treated as anchor.

The list parameter is required in order for the function to call getOffsetForRow to convert indexes to positions.

For grids

More or less the same approach applies to grids too, but the API need to be specified for both axes as an array of values [ horizontal, vertical ] or as a function which returns an array of coordinates [ x, y ] (and to is passed gird instead of list).

For Collections and Tables

I currently have no idea how this applies to those components.

IchabodDee commented 7 years ago

@yuchi Thanks for the explanation + context. It is extremely helpful and thorough. I am using List with dynamic heights and have hit the same issue. Is there anything I can do to help with the Anchor Position API you have proposed?? Happy to try and help implement for List if you could use the help.

raffy2010 commented 7 years ago

@bvaughn any update about this issue? after dig into Grid and SizeAndPositionManager code. I have some ideas. whether we can add lastOffset in _cellSizeAndPositionData to store last offset of each cell. when we back up, because of cell rendered and grid offset need to be recomputed. in componentDidUpdate hook (precisely in _updateScrollLeftForScrollToColumn function) we can get the delta offset of specific cell and add the delta offset to container scrollTop to make the cell to stay same position of container visually.

bvaughn commented 7 years ago

@bvaughn any update about this issue?

Nope.

It's tagged as "help wanted" 😄

Shaked commented 7 years ago

Maybe this implementation can help? https://clauderic.github.io/react-tiny-virtual-list/#/controlled-props/scroll-to-index?_k=gwsgfc

bvaughn commented 7 years ago

That looks to be based on this library 😅 at least comparing clauderic/react-tiny-virtual-list SizeAndPositionManager to bvaughn/react-virtualized CellSizeAndPositionManager

clauderic commented 7 years ago

@bvaughn It totally is ❤️

chrisnojima commented 7 years ago

Hey,

So we're using react-virtualized for our chat component and we're running into this issue also. Basically we're always starting at 'the bottom' of the conversation and we want to be able to scroll up and load older messages. These messages are of varying heights. It seems like most of the machinery of List isn't built around this and assumes you want to always start at the top and append and grow downwards.

I've started to experiment with using css to invert the list and I think its actually working better. I'm still ironing out some issues but I wondered if you had any thoughts about this approach.

What I'm doing is basically using transform: scaleY(-1) on each item and on the innerScrollContainer and passing in my items in reverse order. This allows the list to think its doing things top town (and not thrash the styleCache as I'm scrolling)

Not far enough for a fork/PR yet but wanted to add to the conversation

yuchi commented 7 years ago

That's an old trick in the native mobile development toolbox that has been used for years. The problem is that it could mess with scroll bars and such.

I'm seriously starting to think scroll anchoring would be the solution to a whole class of problems.

bvaughn commented 7 years ago

@chrisnojima Sorry for responding so late. I've been crushed by open source stuff lately and I can't keep up. I'm very interested in seeing your proof of concept.

I've also got a couple of forks of RV that I've been trying to address the same problems with in a more generic way but so far, they aren't solid enough to consider releasing.

chrisnojima commented 7 years ago

Hey, I had to put it on the back burner a little bit. I'll get some more time over the next couple of weeks to play with it some more. The idea was to keep the overflow: auto area oriented the original way so the scrollbar is correct. The virtualized list is flipped using transform: scaleY(-1) so scrollTop:0 is on the bottom and each row is also transform: scaleY(-1). Then here (https://github.com/bvaughn/react-virtualized/blob/master/source/Grid/Grid.js#L367) you just subtract scrollHeight from scrollTopParam and it starts to render correctly.

jurgenzz commented 6 years ago

Hey, ran into similar issue at work, managed to resolve it, but as for the moment i am actually using react-tiny-virtual-list with react-scrollbars. The idea is that i am fully controlling scroll there, but i managed to create similar solution using your library.

At the moment there are quite few issues and i haven't done any performance tests using your library.

Idea behind all this, that i took full control over scroll and caching size of rows. But the magic hides in items, as after they are rendered and updated, i read their height within component itself and pass it up for caching, where afterwards we can differentiate it and if scroll direction was going up, we can adjust scroll from top.

  componentDidMount() {
    this.mounted = true;
    // simulate ajax for each item
    setTimeout(() => {
      if (!this.mounted) {
        return;
      }
      this.setState(
        {
          loaded: true
        },
        () => {
          //get new height and update it in parent
          let { height } = this.node.getBoundingClientRect();
          this.props.onLoad(height);
        }
      );
    }, 500);
  }

And this is called in parent after list item is rendered with updated height

handleMount(index, height, cb) {
    if (!this.customCache[index]) {
      this.customCache[index] = height;
      // default is 52
      // get diff

      let diff = height - 51;

      if (diff && this.scrollDirection === "SCROLL_UP") {
        let newScrollTop = this.scrollTop + diff;
        this.scrollTop = newScrollTop;
        this.setState(
          {
            scrollTop: newScrollTop
          },
          () => {
            this.List.recomputeRowHeights(index);
          }
        );
      } else {
        this.List.recomputeRowHeights(index);
      }
      cb();
    }
  }

Full code you can follow from here: https://github.com/jurgenzz/reverse-scroll-demo/blob/master/public/js/index.js#L51

Not a fan of this though - this.List.recomputeRowHeights(index);, feels like might run into performance issues because of this.

Pretty sure could combine this idea with CellMeasure or implement this in the library, catching scroll direction "up" (as scrolling down is working just fine), and adjust the scroll position, if row size is different than previous cache.

A demo can be seen here - http://jurg.is/virtual-test/ Repo for demo code - https://github.com/jurgenzz/reverse-scroll-demo

Note for the demo: it is a lit buggy and code is not minified there so it's easier to understand. It will scroll almost to the bottom and you can start scrolling up slowly, to see the difference.

Hopefully this will help someone :)

wuweiweiwu commented 6 years ago

Thanks @jurgenzz

andrewvmail commented 6 years ago

@chrisnojima genius on the flipping technique!

Bessonov commented 6 years ago

@chrisnojima @andrewvmail this doesn't work. You just get other problem. See https://github.com/facebook/react-native/issues/14520#issuecomment-366719024 :

The idea to reverse the list is quite cool, but well, that's not really fix the issue with adding elements to top without junks, because it leads to add-to-bottom issue.

chrisnojima commented 6 years ago

just a heads up, i've abandoned using react-virtualized for this and made my own solution short term. I do want to use the next version of this (react-window) where it'll hopefully work great. You can watch the issue in that repo: https://github.com/bvaughn/react-window/issues/6

andrewvmail commented 6 years ago

@Bessonov , @chrisnojima havent tried adding items yet but what kind of issue do you guys have? my use case is just need to add to bottom not top. i guess ill find out

will watch react window thanks for the heads up!

Alacho2 commented 4 years ago

In our case, the list items height isn't just dynamic, but the actual list is also dynamic, adding items to it by the user scrolling upwards (think chat). Our issue, however, is solving itself if we delay the focus on the row until we are sure that the recompute has happened. With this in mind, wouldn't it have made sense to offer a callback function out of when recomputeRowHeights() has run, and then allow the user to scroll to the index (or perform other actions if needed)?

eatfit-rohan commented 2 years ago

I'm facing the same issue on react virtualized 9.21.1 I'm using CellMeasurer with WindowScroller and scrollToRow

bvaughn / react-virtualized