Skip to content

[ui-storagebrowser] add progressbar in file upload #4059

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Mar 19, 2025
Merged

[ui-storagebrowser] add progressbar in file upload #4059

merged 9 commits into from
Mar 19, 2025

Conversation

ramprasadagarwal
Copy link
Collaborator

@ramprasadagarwal ramprasadagarwal commented Mar 16, 2025 •
edited
Loading

What changes were proposed in this pull request?

  • Added a progress bar showing the progress of the file upload for each file
  • Refactored useChunkUpload to improve performance.

How was this patch tested?

  • Manually tested

Please review Hue Contributing Guide before opening a pull request.

image

@ramprasadagarwal ramprasadagarwal self-assigned this Mar 16, 2025
@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 16, 2025 •
edited
Loading

Backend Codecov

Backend Code Coverage Report •
FileStmtsMissCoverMissing
TOTAL533132612451% 
report-only-changed-files is enabled. No files were changed during this commit :)

Pytest Report

Tests Skipped Failures Errors Time
1090 107 💤 0 ❌ 0 🔥 5m 55s ⏱️

@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

@ramprasadagarwal ramprasadagarwal marked this pull request as ready for review March 18, 2025 10:01
bjornalm
bjornalm reviewed Mar 18, 2025
View reviewed changes
Copy link
Collaborator

@bjornalm bjornalm left a comment

Choose a reason for hiding this comment

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

Nice work, see the comments and take an additional look at the BEM naming.

bjornalm
bjornalm requested changes Mar 18, 2025
View reviewed changes
Copy link
Collaborator

@bjornalm bjornalm left a comment

Choose a reason for hiding this comment

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

Nice work to get this all working, but I find the code a bit hard to read.

E.g. "uploadChunkItem" is a function and "UploadChunkItem" is a type that has a uuid that can be used to identify multiple "InProgressChunks" called chunks but then the "UploadChunkItem" also has a "chunkIndex" and those together identify a unique "InProgressChunks" called "chunk".

Are these concepts clear and well defined, if so can we come up with better naming and spend a little time thinking about about to make it all more readable?

@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

bjornalm
bjornalm approved these changes Mar 19, 2025
View reviewed changes
JohanAhlen
JohanAhlen approved these changes Mar 19, 2025
View reviewed changes
@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

@github-actions GitHub Actions
Copy link

⚠️ No unit test files modified. Please ensure that changes are properly tested. ⚠️

@ramprasadagarwal ramprasadagarwal enabled auto-merge (squash) March 19, 2025 16:53
@ramprasadagarwal ramprasadagarwal merged commit 382177d into master Mar 19, 2025
7 of 13 checks passed
@ramprasadagarwal ramprasadagarwal deleted the feat/storage-browser-35 branch March 19, 2025 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JohanAhlen JohanAhlen JohanAhlen approved these changes

@bjornalm bjornalm bjornalm approved these changes

@tabraiz12 tabraiz12 Awaiting requested review from tabraiz12

@nidhibhatg nidhibhatg Awaiting requested review from nidhibhatg

Assignees

@ramprasadagarwal ramprasadagarwal

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ramprasadagarwal @JohanAhlen @bjornalm