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

List virtualization and lazy thumbnails #2161

Merged
merged 16 commits into from
May 5, 2020

Conversation

goto-bus-stop
Copy link
Contributor

@goto-bus-stop goto-bus-stop commented Mar 30, 2020

This patch changes the Dashboard to only render those files that are actually in view, and to only generate thumbnails for files that are in view.

When a FileItem mounts, a thumbnail is requested. There is a case where you might scroll to one place A, and then scroll to another place B before all the thumbnails were ready. Then, you might have to wait until Uppy has generated thumbnails for the files visible at A before any of the files at B start getting thumbnails. So, if a FileItem unmounts before a thumbnail is generated, we cancel the thumbnail request, which reduces that delay.

Issues:

  • Focus can jump back when a row of thumbnails is scrolled out of view.
  • Focus issue 2: Tabbing to the end of a row may not render the next row yet. Then focus is hopeless, so we need to render beyond the viewport a little more aggressively.
  • Hardcoding the number of files per row here seems not ideal … it's derived from the CSS, so it's probably not wrong, but it would be nice to be able to do it automatically or something. If nothing else, both the CSS and JS need a code comment to remind us to update the other if we ever change one.

@goto-bus-stop goto-bus-stop changed the title Virtualization and lazy thumbnails List virtualization and lazy thumbnails Mar 30, 2020
@goto-bus-stop goto-bus-stop marked this pull request as ready for review April 6, 2020 13:24
@goto-bus-stop goto-bus-stop requested a review from arturi April 6, 2020 13:24
@arturi
Copy link
Contributor

arturi commented Apr 11, 2020

Great-great work, Renée! 👍 🥇

Tested in Chrome, Firefox and Safari on Mac, Chrome is the slowest for some reason. Thumbnail generation, even delayed, seems to be the bottleneck — text files render much snappier.

@arturi arturi requested a review from lakesare April 13, 2020 08:46
@arturi
Copy link
Contributor

arturi commented Apr 13, 2020

@lakesare you reviewed this last time and found important bugs, could you take a look again, please? :)

@lakesare
Copy link
Contributor

@arturi, sure, I can review it on Tuesday if it can wait 👌

@lakesare

This comment has been minimized.

@lakesare
Copy link
Contributor

lakesare commented Apr 14, 2020

The interesting artefact is that when we delete the file, focus jumps to the first item in the last virtualized row. Not a huge issue, but doesn't look 100% good on desktops when we click with a mouse.

image

@goto-bus-stop
Copy link
Contributor Author

thanks for the feedback @lakesare! gonna look into the mobile view issue now, that's not very good 😬

@goto-bus-stop

This comment has been minimized.

@goto-bus-stop goto-bus-stop force-pushed the virtualization-and-lazy-thumbnails branch from 2b7bd91 to d3c1179 Compare May 4, 2020 13:55
@goto-bus-stop goto-bus-stop merged commit 0cc2c36 into master May 5, 2020
@goto-bus-stop goto-bus-stop deleted the virtualization-and-lazy-thumbnails branch May 5, 2020 10:22
@kvz
Copy link
Member

kvz commented May 5, 2020

🎉🚀🥇💯🎉🚀🥇💯🎉🚀🥇💯

@arturi
Copy link
Contributor

arturi commented May 5, 2020

🎉 🎉 🎉 🚀 🚀 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants