Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] scrollSeekConfiguration with reversed list: flickering #410

Open
eliasws opened this issue Jul 20, 2021 · 11 comments
Open

[BUG] scrollSeekConfiguration with reversed list: flickering #410

eliasws opened this issue Jul 20, 2021 · 11 comments
Labels
bug Something isn't working

Comments

@eliasws
Copy link

eliasws commented Jul 20, 2021

Describe the bug
When using scrollSeekConfiguration with a reversed list the scrollSeekConfiguration.onEnter callback is called directly on the first render with a high velocity (probably height of the list) which causes the ScrollSeekPlaceholder to be shown. It seems that this was introduced in version 0.21.0-alpha.1.

Reproduction
https://codesandbox.io/s/react-virtuoso-scrolling-issue-forked-xis50?file=/src/App.js

To Reproduce

  1. Reload the page
  2. Check console, scrollSeekConfiguration.onEnter gets called with a velocity of 1446
  3. Items render -> then the ScrollSeekPlaceholder gets rendered for split second -> Items render

Expected behaviour
scrollSeekConfiguration.onEnter should not be called on first render without actual scrolling

Screenshots

Desktop (please complete the following information):

  • MacOS, Android
  • chrome, chrome mobile

Additional context
isScrolling gets also called on first render.

@eliasws eliasws added the bug Something isn't working label Jul 20, 2021
@petyosi
Copy link
Owner

petyosi commented Jul 25, 2021

Confirming that this is a bug, The scrollSeek system should wait for the didMount observable.

@eliasws
Copy link
Author

eliasws commented Aug 5, 2021

I tried to implement it and create a PR. At least in this case didMount seems to be always true (so would not have any effect):

pipe(
    scrollVelocity,
    withLatestFrom(scrollSeekConfiguration, isSeeking, rangeChanged, didMount),
    filter(([_, config]) => !!config),
    map(([speed, config, isSeeking, range, didMount]) => {
      const { exit, enter } = config as ScrollSeekConfiguration
      console.log("DID MOUNT", didMount);
      if (isSeeking) {
        if (exit(speed, range)) {
          return false
        }
 // ........

@petyosi
Copy link
Owner

petyosi commented Aug 6, 2021

@eliasws
Copy link
Author

eliasws commented Aug 6, 2021

scrolledToInitialItem has the same problem :) (always true).

Seems like

u.publish(scrolledToInitialItem, true)

is firing to fast.

For now i just disabled the scrollSeek.

@petyosi
Copy link
Owner

petyosi commented Aug 6, 2021

This flag will turn true after the initial destination has been reached. My understanding is that you are trying to avoid that call.

At any case, I will look at that at some point.

@eliasws
Copy link
Author

eliasws commented Aug 9, 2021

@petyosi FYI, just checked: Same happens with endless scroll ("prepending-items"); the velocity "jumps" for a moment when startReached is called and the new items are prepended.

@petyosi
Copy link
Owner

petyosi commented Aug 9, 2021

That's understandable, based on how velocity and prepending work. Scrolling is readjusted to accommodate the prepended items.

@phenry20
Copy link

phenry20 commented Sep 1, 2021

Any updates?

@tomholford
Copy link

Tried to use scrollSeek + reverse endless and got the same results. Ended up disabling it.

In the meantime, is there a callback that can be hooked into to get the current scroll velocity? The goal is to adjust the fetch page size - if the user is scrolling quickly, we will fetch more data to pre-pend to the list

@petyosi
Copy link
Owner

petyosi commented Nov 18, 2022

@tomholford you can hook up to the scrollerRef, listen to scrollTop changes and calculate the velocity.

@tomholford
Copy link

@tomholford you can hook up to the scrollerRef, listen to scrollTop changes and calculate the velocity.

Great suggestion, will give this a shot. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants