Upgrade from 1.1.2 to 1.3.0 causes janky scrolling.

[dilizarov]

I added a demo of the behavior I see.

There was some weird behavior in 1.1.2 where very rarely, I'd see my items flicker when I scrolled... upgraded to 1.3.0 thinking maybe that could help and it didn't. You'll notice this issue in my video as well.

Here is the video of 1.3.0:

https://streamable.com/brjpsj

You'll see the janky scrolling is obvious and erratic. This was not happening in 1.1.2 - my scrolling was smooth.

As for the flickers, I recommend looking closely at the bottom right message at around 29 seconds. Another example is the message on the bottom left between seconds 34-35.

There are more instances of the flickers, but I pick out two cases.

<Virtuoso
      data={messages}
      overscan={6}
      followOutput="smooth"
      initialTopMostItemIndex={messages.length - 1}
      itemContent={(index, message) => { ... }} />

This is my code. I omit the item piece because I think it is irrelevant. Also, I switched margins on my item to padding like you suggest, so that isn't causing the janky scrolling.

[dilizarov]

On another note, I don't think overscan actually works. When I'm inspecting my list in google chrome dev tools, you don't actually see the overscan items mounted at all.

[petyosi]

I see what you mean, but I will need to reproduce this on my side to help further. Overscan works, until proven otherwise :). Please create code sandboxes for both cases. Notice that the item contents are important, large complex items can reveal such behavior.

[petyosi]

Could be related to https://github.com/petyosi/react-virtuoso/issues/254 - need a repro to verify.

[dilizarov]

Apologies, I just realized I treated overscan as “number of additional items” and not “number of additional pixels”

I’m positive it’ll work.

I will either get a repro or confirm your suspicion.

On Sat, Jan 23, 2021 at 5:55 AM Petyo Ivanov notifications@github.com wrote:

Could be related to #254 https://github.com/petyosi/react-virtuoso/issues/254 - need a repro to verify.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/petyosi/react-virtuoso/issues/269#issuecomment-766083375, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5BT2UXFDMPUAPHXXPHJM3S3LINHANCNFSM4WPRRAKA .

-- Sent from my iPhone

[dilizarov]

Can confirm that overscan works when assuming the number is pixels.

[dilizarov]

@petyosi I can confirm that this release https://github.com/petyosi/react-virtuoso/releases/tag/v1.2.5 causes the issue.

Will look into it a bit more.

[petyosi]

This makes sense. The fix there introduces an additional frame draw. If you can do the reproduction in a codesandbox, this will help me ensure that what you have will be addressed properly.

[dilizarov]

Have you read this? https://stackoverflow.com/questions/49384120/resizeobserver-loop-limit-exceeded

The answer given by specification authors seems to suggest that we shouldn't worry about this and the error is benign and I imagine that is definitely the case with this library.

https://github.com/WICG/resize-observer/issues/38 has a good discussion.

Best action is probably to revert the change made in 1.2.5. For a library as intensive as this (where performance and feel matters), every little thing makes a difference and the RAF probably should not be included for the sole purpose of quieting this benign error.

Just my two cents.

If anything, I imagine reading the above will further your conviction that what you're doing is correct and getting the error makes sense in your use-case (while acknowledging that the error brings no harm).

[petyosi]

I have read that. Unfortunately, react-beautiful-dnd does not take the nature of the error into account. The lib has some error handling mechanism that is triggered by that exact error :(. I was hoping that the RAF will resolve the integration with it.

I will have to revert the change and suggest a workaround that's using stopImmediatePropagation.

[dilizarov]

Oh I see. If I understand correctly, this library uses react-beautiful-dnd under the hood and error handling logic ends up getting called because of this error.

That is rough.

This comment may help:

We had this issue in coming from @microsoft/applicationinsights-web which does our client error logging. So we just ignore this error by setting up an error event handler prior to applicationInsights and call stopImmediatePropagation and preventDefault

[dilizarov]

Also, I hope I don't jinx myself, but I must say... If we ignore this bug, then this is the first time I've incorporated virtualization in a project and have said to myself "wow that was so simple and the results are amazing" instead of cursing and trying to get everything right. So kudos to you.

I've said it before, but this really is a magical library. I hope my results remain the same when I cross-browser test and test on mobile too 😂.

[petyosi]

Heads up: reverse chat scroll compensation (the behavior we are discussing right now) in iOS has its separate handling path, which is not so smooth as the one in Chrome (before the RAF caused glitch).

Apple tries their best to not let developers mess up the native kinetics of the browser, so some scrollBy calls (which compensate internally for variably sized items) are dropped. To work around that, the glitch compensation is applied when the scrolling stops (code is here). That's the best I could come up with.

Since I don't have an android, I am not sure about the native behavior there.

[dilizarov]

I will test both accordingly.

[github-actions[bot]]

:tada: This issue has been resolved in version 1.3.1 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: