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

fix(shares): use nextcloud/upload to track upload progress #11198

Merged
merged 6 commits into from
Feb 12, 2024

Conversation

Antreesy
Copy link
Contributor

@Antreesy Antreesy commented Dec 11, 2023

☑️ Resolves

  • Fix webdav doesn't support upload progress since v5 #11153
    • add nextcloud/upload library to perform file uploads to server
    • attach uploadManager data to track common upload progress
  • Split large uploadFiles action into 4 smaller manageable actions
  • update NcProgressBar styles (so it won't affects scroller height and chat jumping)

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

image

loading.mp4

🚧 TODO

  • track upload progress for each single file
  • test coverage

🏁 Checklist

  • 🌏 Tested with Chrome, Firefox and Safari or should not be risky to browser differences
  • 🖌️ Design was reviewed, approved or inspired by the design team
  • ⛑️ Tests are included or not possible

@Antreesy Antreesy added 2. developing bug feature: frontend 🖌️ "Web UI" client feature: upload & shares & voice 📤🎙️ Sharing files into a chat and audio recordings dependencies Pull requests that update a dependency file labels Dec 11, 2023
@Antreesy Antreesy requested a review from DorraJaouad December 11, 2023 17:35
@Antreesy Antreesy self-assigned this Dec 11, 2023
@Antreesy Antreesy force-pushed the fix/11153/add-readable-stream branch from 2eaf461 to 5830ddb Compare December 11, 2023 20:14
@Antreesy Antreesy force-pushed the fix/11153/add-readable-stream branch 3 times, most recently from 1e80b79 to 0cd7591 Compare January 22, 2024 11:19
@Antreesy Antreesy marked this pull request as ready for review January 22, 2024 11:22
@Antreesy

This comment was marked as resolved.

@Antreesy Antreesy force-pushed the fix/11153/add-readable-stream branch from 0cd7591 to 7d82327 Compare January 31, 2024 20:01
@Antreesy
Copy link
Contributor Author

Rebased, updated style of NcProgressBar to circular

@Antreesy

This comment was marked as resolved.

Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

@Antreesy would it be possible to put it to the left of the title?

In the first option there's not enough contrast with the background and the shapes below make it a bit confusing, while in the second option I fear that the indicator might be too far from the title and icon.

@Antreesy
Copy link
Contributor Author

Antreesy commented Feb 8, 2024

put it to the left of the title?

That wouldn't work for the files with a preview and without a title, like images / text
I'd suggest to place it on the right side of a preview/icon, or design two cases for files with/without a title

@Antreesy
Copy link
Contributor Author

Antreesy commented Feb 8, 2024

@marcoambrosini If you're not ready to come up quickly with a solution, we could split out the design changes into follow-up and merge only library + code for now

@marcoambrosini
Copy link
Member

I'd suggest to place it on the right side of a preview/icon

@Antreesy this is good too!

@Antreesy Antreesy force-pushed the fix/11153/add-readable-stream branch from 0adf0ae to cb6d3df Compare February 9, 2024 09:43
@Antreesy Antreesy requested a review from DorraJaouad February 9, 2024 09:43
Copy link
Contributor

@DorraJaouad DorraJaouad left a comment

Choose a reason for hiding this comment

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

Tested 🦅

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy Antreesy force-pushed the fix/11153/add-readable-stream branch from cb6d3df to 467dc40 Compare February 12, 2024 11:51
@Antreesy Antreesy enabled auto-merge February 12, 2024 11:53
@Antreesy Antreesy merged commit eeb71fd into main Feb 12, 2024
37 checks passed
@Antreesy Antreesy deleted the fix/11153/add-readable-stream branch February 12, 2024 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review bug dependencies Pull requests that update a dependency file feature: frontend 🖌️ "Web UI" client feature: upload & shares & voice 📤🎙️ Sharing files into a chat and audio recordings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

webdav doesn't support upload progress since v5
4 participants