-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Edit Site: Update progress bar to determinate #53399
Conversation
Size Change: +80 B (0%) Total Size: 1.52 MB
ℹ️ View Unchanged
|
This is looking promising! |
b5b67cc
to
d08732e
Compare
52e7767
to
ddc0096
Compare
70decb8
to
afba806
Compare
Flaky tests detected in e42847e. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5950525136
|
Feels great! Some questions about how it's measuring progress, if you look closely at this GIF of the progressbar, you'll see that it starts not from zero but from a ways in, and it sits there for just about a second before the page itself fades in: Give or take a little wiggleroom, it would seem the most user intuitive if the progressbar started from zero, and only milliseconds after completing the track faded the page in. But I guess as far as the loading progress it also counts loading itself? And what might explain the second at the end where it suggests loading to be complete but still sits there a bit? Detail territory, to be clear! But I'm curious. Nice work. |
Thanks for taking a look so quickly @jasmussen! Ideally, the progress bar would progress steadily from 0% to 100% smoothly, at the same pace from the beginning to the end. Unfortunately, we don't live in an ideal world, and the network requests don't take the same amount of time, and they can vary in terms of when they start, how long they take, and when they finish. However, the good part about all that is that the progress bar displays the actual, real loading progress, defined by the pending and finished requests. It happens to have a head start sometimes because some of the requests finish really quickly, and others, like the ones that load towards the end, are taking a while longer. And we treat requests as equal, regardless of how quick or complex they are. The fact that you're experiencing what you described only demonstrates that the site editor progress bar is actually working great and reflects the actual loading that happens behind the scenes! Let me know if that makes sense. |
It does make sense. I wonder if this hack would work, simply add High level though, this seems good to ship, and these things can all be explored separately. Including, potentially, the ability to for the determinate slider to temporarily change to being indeterminate, and back, if one of the request takes a bit longer than expected. |
A transition or |
@jasmussen @mtias Adding a transition is a great idea! Since it's a separate improvement to the |
afba806
to
70e8d10
Compare
In the meantime, I've rebased the PR and it's still waiting for a code review before it gets merged. Noting that it also depends on #53767 which introduces a new |
Nice! I noticed today when re-testing, that when loading the site editor, this progressbar fades in at the start. I wonder, if it is determinate ideally it'd start at 0 percent, so essentially the progressbar would come into being from nothing. Can we/should we remove that initial fade-in? Or just make it really fast? Small detail. |
70e8d10
to
e42847e
Compare
Happy to try this! Let's get a code review and land it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e42847e
to
538b212
Compare
What?
This PR builds on top of the work done in #53032 and makes the progress bar determinate.
Blocked by #53767 and needs to be rebased once it lands.
Why?
To provide the end users with more accurate feedback on their site editor loading experience.
How?
We believe that using a determinate progress bar instead will be a better loading experience for the user, because it provides better feedback on the loading progress.
This PR also contains a new redux metadata selector that we're separately introducing in #53767.
Testing Instructions
Testing Instructions for Keyboard
None
Screenshots or screencast
Screen.Recording.2023-08-23.at.14.15.42.mov