-
Notifications
You must be signed in to change notification settings - Fork 2k
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
UI doesn't work satisfactory when adding 10k files #4389
Labels
Comments
I just tried 11k files from remote provider (google drive) and now it actually works (after my big PR #4384 was merged). It takes about 7 seconds from the files are finished loading until they show up in the list. so UI is blocked for 7 seconds, and after that it's fully responsive (on Chrome). but I have a MBA M2, so maybe it's cheating. will test some more and try to get the block time down |
mifi
added a commit
that referenced
this issue
Apr 5, 2023
don't do it for every file added, as it's very slow instead do the check at the end when all files are added. this allows us to easily work with 10k+ files fixes #4389
mifi
added a commit
that referenced
this issue
Apr 15, 2023
* show how many files are added when loading remake of #4388 * add french (cherry pick) * implement concurrent file listing * refactor / fix lint * refactor/reduce duplication * pull out totals validation don't do it for every file added, as it's very slow instead do the check at the end when all files are added. this allows us to easily work with 10k+ files fixes #4389 * Update packages/@uppy/core/src/Uppy.js Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> * make restricter.validate validate everything instead make more specific methods for sub-validation also rename validateTotals to validateAggregateRestrictions * improve errors and user feedback - handle errors centrally so that we can limit the amount of toasts (informers) sent to the users (prevent flooding hundreds/thousands of them) - introduce FileRestrictionError which is a restriction error for a specific file - introduce isUserFacing field for RestrictionError * fix performance issue reintroduced * improvements - show "%{count} additional restrictions were not fulfilled" for any restriction errors more than 4 - refactor/rename methods - improve ghost logic/comments * improve performance when uploading - introduce new event "upload-start" that can contain multiple files - make a new patchFilesState method to allow updating more files - unify "upload-start" logic in all plugins (send it before files start uploading) - defer slicing buffer until we need the data - refactor to reuse code * fix e2e build issue * try to upgrade cypress maybe it fixes the error * Revert "fix e2e build issue" This reverts commit ff3e580. * upgrade parcel * move mutation logic to end * remove FileRestrictionError merge it with RestrictionError * fix silly bug looks like the e2e tests are doing its job 👏 --------- Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 task
This was referenced May 3, 2023
This was referenced May 8, 2023
This was referenced May 20, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Steps to reproduce
Expected behavior
It should show some kind of progress to the user and then allow them use the UI and to upload the files within a reasonable amount of time
Actual behavior
It takes about 5 minutes to load all files (due to pagination on the api towards Google) this is expected and unavoidable - with #4388 this is OK because the user will be able to see what's happening
Then it takes about 2 minutes for all the
addFile
calls to finishaddFiles
instead (ok done!)Then there's two errors that get spit out:
Then the whole DOM/UI freezes for about 5 minutes and you can't click anything or scroll the list. Only the first few files are shown, without any thumbnail.
After 5 minutes you can scroll again, and you may now start the upload.
Maybe we should have an e2e test for this.
Note: Similar problem also for Drag/drop local files.
The text was updated successfully, but these errors were encountered: