petyosi / react-virtuoso

The most powerful virtual list component for React
https://virtuoso.dev
MIT License
5.25k stars 301 forks source link

react-beautiful-dnd and Items with images (async sizing) #283

Closed Brian-McBride closed 3 years ago

Brian-McBride commented 3 years ago

First, thanks for the lib. I'm looking to move from react-virtualized. I'm liking what you have going on here!

I have a list that contains images. Currently, I do not cache the image height in the data payload, just the image URL. When I was following the example at https://virtuoso.dev/react-beautiful-dnd/ it has the whole HeightPreservingItem component with the data-known-size prop. In this example, the component gets measured before the image is loaded, but then after the image loads, there isn't a resize for that item.

Is there a way you recommend triggering a resize for each item when there is an image load?

petyosi commented 3 years ago

Starting with the good news, triggering a resize is not the way to go (it is going to get very messy). The HeightPreservingItem function is to prevent the full collapsing of the container when the item gets dragged around, leading to a zero-height element (not supported by Virtuoso).

I am sure that there are many ways to avoid that, you can try with minHeight:

style={{ minHeight: props["data-known-size"] || undefined }}

minHeight: 1 works for sure:

style={{ minHeight: 1 }}

Keep me posted on what you came up with.

Brian-McBride commented 3 years ago

The minHeight worked great. I would actually suggest that you change your documentation to have that as the default instead of height. I should have thought of trying it. I'll close the issue.

I saw you split out a state engine. I love observables in general as well and generally dislike redux. In React, I have come to default to hookstate.js Just sharing it in case you haven't run across it. It is fast, I really like the simplicity of it. The way it hooks in is so much better than context as well. There might be some things in there that might inspire your own state lib (or not). Not to start a discussion on this thread, just wanted to share while I was here.

Thanks again for the fast response and simple solution.

russelldc commented 3 years ago

@Brian-McBride Would you be able to share more about how this worked out for you?

I've tried extending @petyosi's original react-beautiful-dnd codesandbox, such that every even indexed item has 2 lines of text, while every odd indexed item has 1 line of text. As far as I can tell (based on fiddling with the demo and inspecting the html elements, not exploring virtuoso's code), it seems like every item's data-known-size value is based on the height of the first item. You can see that represented as a gap below each of those 1 liner items:

https://user-images.githubusercontent.com/5100126/107308622-cecca700-6a3d-11eb-975d-4a3c92ee32e3.mp4

If the even/odd condition is flipped around, then the list looks fine at first, until you try to move something:

https://user-images.githubusercontent.com/5100126/107308777-17846000-6a3e-11eb-87f8-80c6312cf353.mp4

Switching from height to minHeight doesn't seem to make a difference in my case: https://codesandbox.io/s/react-virutoso-with-react-beautiful-dnd-forked-37ds9?file=/src/index.js

petyosi commented 3 years ago

@russelldc you can use other means to prevent the wrapper from collapsing - for example, by putting a 1px tall div, like this.

Another thing that I had to add to your example was a stopImmediatePropagation error handler. This is a harmless error that is being caused by the internal Virtuoso resize observer, which causes DnD to stop scrolling.

russelldc commented 3 years ago

@petyosi Even on the sandbox you linked, I'm still seeing very weird shifting behavior when I pick up/drag an item:

https://user-images.githubusercontent.com/5100126/107420475-c623b180-6acd-11eb-9a57-8515567d17c4.mp4

petyosi commented 3 years ago

Reopening; needs a more reliable solution.

russelldc commented 3 years ago

This might be why it seemed fine at first glance... if you pick up an item and auto-scroll the list a bit by dragging it towards the bottom, eventually the surrounding items will look like they're shifting correctly around the dragged item. On the other hand, if you pick one up and don't scroll around, that's when the weird shifting is obvious:

https://user-images.githubusercontent.com/5100126/107421675-19e2ca80-6acf-11eb-92de-fa8982c69b8e.mp4

petyosi commented 3 years ago

@russelldc thanks. This might be related to the way mode="virtual" is implemented (as it has react-window as an integration target). I will look into a reliable solution for the content measurement.

petyosi commented 3 years ago

@russelldc @Brian-McBride @ozsirmajotform take a look at this variation - let me know if this makes sense.

ozsirma commented 3 years ago

I did not encounter a problem. Nice to see the new approach works. 👍

russelldc commented 3 years ago

@petyosi Your codesandbox version is working perfectly for me (besides some leftover padding in the list while an item is being dragged, but I think that can probably be easily dealt with, and depends on the general styling choice for the items).

I haven't had the chance yet to try it out in my target project, but will let you know ASAP whether I run into any issues.

Thanks so much for addressing this so quickly!

petyosi commented 3 years ago

Thanks everyone for the feedback, I am closing the issue.

uffou commented 3 years ago

When dragging items from this example there is some jumping, not sure how to deal with it. @russelldc Have you managed to find a solution?

https://user-images.githubusercontent.com/2581788/130256493-c104bfe0-616d-481a-a2ff-cf80c41b7681.mp4