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

V15: Show upload progress for dropped files in the Media Library #18148

Merged
merged 25 commits into from
Jan 29, 2025

Conversation

iOvergaard
Copy link
Contributor

@iOvergaard iOvergaard commented Jan 28, 2025

Description

With the change from Fetch to XHR in #18113 we now can follow along on the upload progress on dropped files on the dropzone in the Media Library. This PR aims to wire up the code from the upload function all the way back to the temporary file badge.

Changes

  • Wires up onProgress -> progress() through the progressItems and into the media collection grid view to show on the temporary file badge
  • Changes the design of the temporary file badge to fill out the placeholder container, so that it is more visible
  • Replaces the arrow-up icon in the temporary file badge with the actual progress in percent when uploading
  • Introduces a new warning message if a dropped file has no media types (it was just ignored before)
  • Fixes a lot of mock endpoints and data that were missing in MSW (note: XHR upload progress does not work through the service worker, so the progress goes from 0 to 100 without any indication, so test on the live server)
  • Changes the signature of the newly introduced xhrRequest so it no longer accepts a controller host - it will now use the OpenApi object to infer the token directly instead allowing it to better mimic the generated ts client AND work with the mock server (this hasn't been released yet, so should be safe to break)
Screenshare.-.2025-01-28.5_03_11.PM.mp4

Note: The error shown when a file is too large is going to be addressed in another PR.

How to test

  • Drag & drop media on the media library
  • Try and test both files that are not allowed and allowed at the same time (you need to go to the media types and type in allowed file extensions)
  • Test larger files that take a while to upload to see that the progress is reflected

@iOvergaard iOvergaard enabled auto-merge (squash) January 28, 2025 17:14
@bjarnef
Copy link
Contributor

bjarnef commented Jan 28, 2025

Perhaps it is just me or would a simple progress bar look more clean/slick .. imagine uploading 20 files :)

I like something like these examples:
https://www.uinkits.com/components/file-upload-ui-element
https://vaadin.com/docs/latest/components/upload

Perhaps just show an overlay with progress and refresh view + tree when finished?

@iOvergaard
Copy link
Contributor Author

@bjarnef Thanks for your input. I like the Vaadin component. We went another way at some point and tried to hide the dropzone UI a bit unless you use it.

I agree that some overlay could persist as the files are uploading or shown above the collection view. Code-wise, it would also be more slick, as we wouldn't have to force these placeholder items into the collection view.

I think it is out-of-scope to change the whole UI in this PR, but I would certainly invite to a discussion in the area. Would you mind if I convert your comment into a discussion?

@bjarnef
Copy link
Contributor

bjarnef commented Jan 29, 2025

@iOvergaard created a discussion here: #18155

Copy link
Member

@leekelleher leekelleher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested out, it works really well! 🚀

As for any cosmetic tweaks, we can see how the discussion goes and iterate on that.

@iOvergaard iOvergaard merged commit ab98ea5 into v15/dev Jan 29, 2025
29 checks passed
@iOvergaard iOvergaard deleted the v15/feature/media-library-drop-progress branch January 29, 2025 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants