Edit Site: Use progress bar for loading screen #53032
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tyxla
added
[Type] Enhancement
tyxla
changed the title
|
Size Change: +240 B (0%) Total Size: 1.51 MB
ℹ️ View Unchanged
|
tyxla
force-pushed
the
try/site-editor-progress-bar
branch
from
9c91c36 to
d160f69
Compare
tyxla
force-pushed
the
try/site-editor-progress-bar
branch
from
f40abc7 to
177066e
Compare
tyxla
force-pushed
the
try/site-editor-progress-bar
branch
3 times, most recently
from
e2aa008 to
ba664c6
Compare
This was referenced Aug 4, 2023
tyxla
added
[Status] Blocked
tyxla
requested review from
mtias,
jameskoster,
jasmussen,
mirka,
jsnajdr,
flootr and
chad1008
tyxla
removed
Needs Design Feedback
|
@alexstine I think all your feedback has been addressed - let me know what you think when you get a chance! I'd love to be able to proceed with shipping this one, which seems to be pending only accessibility feedback at this time. |
|
@tyxla It is still not working for me and I'm not sure why. Exactly where in the editor has Thanks. |
Thanks for your review, @alexstine! That's correct - I've added the I've fully followed the instructions you suggested and inspecting the accessibility tree in Chrome's web tools proves that the region element properly captures the loading state coming from the progress bar element that describes it. So I'm not really sure what else can be done to help test this. In #53671 I've opened a PR that enforces a constant loading state, so you wouldn't have to slow your browser down; the loading state will just be infinitely displayed. Feel free to test with that PR if that helps. Let me know what else I can do to help you test this. |
|
@tyxla In your testing PR, I do not see those ARIA attributes. Can you verify they exist? I can verify the page does not load, but those attributes are not there. Thanks. |
Sorry for the trouble! I'm not sure what's the issue, but I can verify they exist, as demonstrated by the following explanation and screenshots. There's the
which has the following accessibility tree:
and there's the corresponding
which has the following accessibility tree:
On top of the fact that all the elements are there, the provided accessibility trees demonstrates that the canvas wrapper Let me know if that helps! |
tyxla
force-pushed
the
try/site-editor-progress-bar
branch
from
52e7767 to
ddc0096
Compare
|
@tyxla Seems to be working now, sorry about this. I was forgetting to do the famous |
|
@alexstine that's great news, thanks for confirming 🙌 Thanks, everyone for the thorough feedback 🚀 |
SiobhyB
added
the
[Feature] Site Editor
|
Work to change the progress bar to a determinate one happens in #53399. |
14 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This PR attempts to use the progress bar component from #53030 for the site editor loading screen.
This is still using the indeterminate progress bar, but ideally, we're aiming to implement a determinate one.
Ready for review
but currently blocked by:Themevia private APIs #53262tabIndexto make focusable #53383and needs to be rebased once it lands.cc @andrewhayward @alexstine for accessibility feedback, as discussed previously in #53030.
Why?
We believe that using a progress bar instead of a spinner will be a better loading experience for the user. Especially if we provide better feedback on the loading progress, which we're ultimately aiming to get to.
How?
We're extracting the highlighted colors from global styles previews to the
useStylesPreviewColors()hook so we'd be able to reuse them for the loading experience.We're replacing the
Spinnerwith aProgressBarfor the site editor loading experience.For theming/colors, we're using the
@wordpress/componentsTheme system, using the first highlighted color as an accent color (falling back to the text color if none), and the background color as a background. Then we rely on the theming system to generate the gray for us, and we use the accent color as-is. I could use some feedback for that particular part, and whether anyone has a good idea for what colors to use so they'd work best in all scenarios.Furthermore, this PR still includes a couple of improvements to the
ProgressBarcomponent that we're aiming to discuss and land separately in #53347 and #53349.The next step after this PR will be to experiment with making the progress determinate.
Testing Instructions
Testing Instructions for Keyboard
Similar to the above.
Screenshots or screencast
Site editor loading experience with the progress bar:
Screen.Recording.2023-08-04.at.15.57.02.mov
Demo with all theme style variations of the
twentytwentythreetheme:Screen.Recording.2023-08-04.at.15.54.21.mov