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

Infinite-scrolling example not working using the scrollbar #3894

Closed
rkulinski opened this issue Aug 1, 2022 · 11 comments
Closed

Infinite-scrolling example not working using the scrollbar #3894

rkulinski opened this issue Aug 1, 2022 · 11 comments
Labels
bug Something isn't working examples

Comments

@rkulinski
Copy link
Contributor

rkulinski commented Aug 1, 2022

What version of Remix are you using?

1.6.4

Steps to Reproduce

Open examples/infinte-scrolling and select page/simple.

Expected Behavior

When I scroll by grabbing scroll bar to bottom I should see all the elements loaded.

Actual Behavior

Elements displays as Nothing to see here...
Screen Shot 2022-08-01 at 10 13 51

I think you might want to change:

  const rowVirtualizer = useVirtual({
    size: data.totalItems,
    parentRef,
    estimateSize: useCallback(() => 35, []),
    initialRect: { width: 0, height: 800 },
  });

to

  const rowVirtualizer = useVirtual({
    size: items.length,
    parentRef,
    estimateSize: useCallback(() => 35, []),
    initialRect: { width: 0, height: 800 },
  });

Otherwise this is generating many problems like calling issues when scrolling fast or throwing warnings:

Warning: Maximum update depth exceeded. This can happen when a component calls setState inside useEffect, but useEffect either doesn't have a dependency array, or one of the dependencies changes on every render.
@machour
Copy link
Collaborator

machour commented Aug 1, 2022

I just tried the example using Remix 1.6.7, and everything is working as expected:

Screen Shot 2022-08-01 at 9 43 54 AM

In the network tab, I can see the XHRs being fired, and the response does contain totalItems entry, which is being correctly used here.

Can you share more details about your setup ? (node version, browser, etc)

Thanks !

@machour machour added examples needs-response We need a response from the original author about this issue/PR labels Aug 1, 2022
@rkulinski
Copy link
Contributor Author

rkulinski commented Aug 1, 2022

Thanks for such a short reply. I cannot run this example on code sandbox - not sure what is wrong, but I'm getting blank screen. I downloaded repo and run it.

Node v16.14.0
Firefox

Note that you can't use you're mouse wheel or touchpad. To repro this you need to drag scroll bar with mouse.

@machour
Copy link
Collaborator

machour commented Aug 1, 2022

I also encountered a problem with codesandbox, and ran the project locally using yarn create remix --template https://github.com/remix-run/remix/tree/main/examples/infinite-scrolling

I was using the touchpad, but I do see the problem now while using the scrollbar !

@machour machour added bug Something isn't working and removed needs-response We need a response from the original author about this issue/PR bug:unverified labels Aug 1, 2022
@machour machour changed the title Infinite-scrolling example not working Infinite-scrolling example not working using the scrollbar Aug 1, 2022
@machour
Copy link
Collaborator

machour commented Aug 1, 2022

I tested your change, and it seems to do the trick, although the scrollbar gets a little jumpy.
Would you mind opening a PR with your proposed fix so we can get to the root of the problem with the rest of the team?

@rkulinski
Copy link
Contributor Author

rkulinski commented Aug 1, 2022

Yeah it does. Although in my scenario I had to some more tweaks to make it work.

I agree that scrollbar is jumpy, but if there would be a wish to have scrollbar mimic fully loaded data set it would require refactoring of how items are loaded into array (currently the problem is that due to so many state changes some of them seems to be skipped, hence indexes mismatch).

Maybe, before making code changes it would be a good idea to discuss and have a clear goal for this example.

I can help out with this naturally :)

In case we are okay with "classic" infinite scroll (where list extends height as more items are loaded) I can provide some PR to fix this example.

@machour
Copy link
Collaborator

machour commented Aug 1, 2022

Summoning @kentcdodds 🪄

@kentcdodds
Copy link
Member

Can we compare what you're experiencing with other infinite scroll examples like Twitter? Do you experience the jumpiness there? It's it just an issue with infinite scroll in general?

@rkulinski
Copy link
Contributor Author

I don't have a Twitter, but e.g. 9gag is using the same pattern:
https://9gag.com/

Although it seems like a cool thing to display scrollbar as if all items are already loaded it is causing many technical obstacles. And when I think about it more UX is not ideal as well. Let's say list is 30k elements. With such number of items scroll is not precise anyway, so I think it's not that useful.

@kentcdodds
Copy link
Member

I'd like to see what code changes actually look like in a PR. I think I'd probably merge it 👍

@rkulinski
Copy link
Contributor Author

@kentcdodds
Really simple change:
#4003

@kentcdodds
Copy link
Member

Merged. 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 examples
Projects
None yet
Development

No branches or pull requests

3 participants