plone / volto

React-based frontend for the Plone Content Management System
https://demo.plone.org/
MIT License
427 stars 575 forks source link

Replace spinner with Progress bar and fix it's position . (#5620) #5929

Open victorchrollo14 opened 1 month ago

victorchrollo14 commented 1 month ago

Fixes #5620.

To Implement the progress bar.

Screencast from 26-03-24 06:57:09 PM IST.webm

netlify[bot] commented 1 month ago

Deploy Preview for volto canceled.

Name Link
Latest commit 7805969453d70b50aeef17a21f9b6a59eda291e0
Latest deploy log https://app.netlify.com/sites/volto/deploys/663116fea42ef700081eae60
netlify[bot] commented 1 month ago

Deploy Preview for plone-components canceled.

Name Link
Latest commit 7805969453d70b50aeef17a21f9b6a59eda291e0
Latest deploy log https://app.netlify.com/sites/plone-components/deploys/663116fe8e6cc1000810e423
stevepiercy commented 1 month ago

I restarted the failed CI check, and we'll see if it passes this time.

stevepiercy commented 1 month ago

CI is all green. Let's get a review from @plone/volto-team.

victorchrollo14 commented 1 month ago

That's great that you took the extra effort to provide translations to all those languages. I'll request review from the suggested folks, but anyone who has an interest in getting this merged is welcome to add a review. Thank you!

Hey @stevepiercy. I didn't add the translations, the earlier message of 'uploading files' got automatically removed after I made the changes, I thought this is normal and committed those changes.

I'll add the translations and push it up then.

ichim-david commented 1 month ago

@victorchrollo14 the feature looks like it works based on your recording. I wouldn't be surprised if you won't receive feedback to tweak the styles or for us todo so after merging. I would still like to test it and see how it performs and it might take a while until this is merged but I have a good feeling about this one, congratulations.

victorchrollo14 commented 1 month ago

@victorchrollo14 the feature looks like it works based on your recording. I wouldn't be surprised if you won't receive feedback to tweak the styles or for us todo so after merging. I would still like to test it and see how it performs and it might take a while until this is merged but I have a good feeling about this one, congratulations.

I just came up with a random implementation and was expecting someone would tell me to implement it in a better way or tweak it a bit.

I too am surprised that I was not asked to make changes in code. I'll fix it up if there are any issues.

victorchrollo14 commented 3 weeks ago

hey @sneridagh. does this look alright?

sneridagh commented 3 weeks ago

@victorchrollo14 if it works well, please then simplify it and we are good to go.

victorchrollo14 commented 3 weeks ago

@victorchrollo14 if it works well, please then simplify it and we are good to go.

alright. I'll revert to the previous commit and push this change.

sneridagh commented 1 day ago

@plone/volto-team I need a final ack on this one, @nileshgulia1 are all your questions answered?

stevepiercy commented 1 day ago

I dislike the spacing around the counter. I think it is OK to have the text jump as this happens in every progress bar I have ever watched, but if not, then I would left-align the text and replace it with two lines:

Files uploaded: <count>
Total files to upload: <total>

I agree that the contrast is too low between the counter text and its grey overlay background. However I think that a background color on the entire progress indicator container would be the better option. The rounded corners of both the counter text container and the progress bar stacked on top of each other is inconsistent with Pastanaga's principles of no rounded corners.

Finally, I think the progress bar itself should be rectangular without rounded corners.

ichim-david commented 1 day ago

I dislike the spacing around the counter. I think it is OK to have the text jump as this happens in every progress bar I have ever watched, but if not, then I would left-align the text and replace it with two lines:

Files uploaded: <count>
Total files to upload: <total>

I agree that the contrast is too low between the counter text and its grey overlay background. However I think that a background color on the entire progress indicator container would be the better option. The rounded corners of both the counter text container and the progress bar stacked on top of each other is inconsistent with Pastanaga's principles of no rounded corners.

Finally, I think the progress bar itself should be rectangular without rounded corners.

I like the 2 line solution and it would get rid of the spacing and counter padding needed and since it's the last text the previous text would probably remain in place.

I am fine with having a bg color for that section and getting rid of the rounded option if this is desired. @stevepiercy you might need to make a screenshot with your proposal so that we know exactly what you want for bg color

stevepiercy commented 1 day ago

@stevepiercy you might need to make a screenshot with your proposal so that we know exactly what you want for bg color

As long as there is sufficient contrast, I'm OK with whatever is chosen. White appears to be the default background color, so I would go with that. However there might be an existing style or class that can be reused instead of creating a new one. blocks-chooser may be a good class to reuse.

djay commented 23 hours ago

I'm not saying there is a need to change anything UI wise but quanta does have a design for how this should look.

If there are changes to the UI then it would be nice to make them in the direction of the quanta design (layout, not styles)

image

Some differences are

Quanta doesn't say much about the progress bar looks but we did an implementation of upload screen that worked with TUS that which means that it gives progress on individual files (in case they are large). Not sure that can be done here as you might not get single file progress using the normal upload api?

The details for how we proposed the final upload UI would look are all in

5423

In any case, just a heads up this UI may change again in the future as the proposal was to switch to TUS for all uploads

stevepiercy commented 22 hours ago

There are two other Quanta related PLIPs:

They are targeted for Plone 7 and depend on other parts that are beyond the scope of fixing this bug. I'd prefer to fix the bug now, reusing styles that already exist.

ichim-david commented 22 hours ago

@stevepiercy @djay we have to take into consideration also the scope of this ticket which is to simply provide a progress bar with some info on how many files.

I took Steve's suggestion and tweaked the rounded corners of the progress and added the info on 2 lines. I think this is a good solution but let's hear your opinion.

https://github.com/plone/volto/assets/152852/3df17c41-53a1-4ec0-879f-5786ca7d1c29

stevepiercy commented 22 hours ago

@ichim-david the contrast is sufficient, and there's no more jumpy text. I think it is good. Would you please push a commit with your work?

I think we're waiting for input from @nileshgulia1.

djay commented 21 hours ago

To be clear I wasn't referring to quanta styles but the layout and functionality.

victorchrollo14 commented 18 hours ago

@stevepiercy @djay we have to take into consideration also the scope of this ticket which is to simply provide a progress bar with some info on how many files.

I took Steve's suggestion and tweaked the rounded corners of the progress and added the info on 2 lines. I think this is a good solution but let's hear your opinion.

flex-two-lines.mp4

@ichim-david @stevepiercy @djay @sneridagh @nileshgulia1. Can we try something like this. if this doesn't feel fine, then I'll make the changes @ichim-david suggested.

Screencast from 01-05-24 03:10:15 PM IST.webm

stevepiercy commented 18 hours ago

@victorchrollo14 absolutely, positively, 100% YES! That is exactly what I imagined, but could not put into words. It looks consistent, accessible, and well-integrated.

victorchrollo14 commented 1 hour ago

@victorchrollo14 absolutely, positively, 100% YES! That is exactly what I imagined, but could not put into words. It looks consistent, accessible, and well-integrated.

I'll push these changes then.