Styles Navigation Screen: Add Style Book #50566
Conversation
andrewserong
added
[Type] Enhancement
| openGeneralSidebar( 'edit-site/global-styles' ); | ||
| } } | ||
| /> | ||
| <div> |
There was a problem hiding this comment.
I've just used a simple div here so that there isn't any space between the buttons. Happy to change this if we need the buttons to have some breathing room.
There was a problem hiding this comment.
Sounds good 👍
I'm not an expert here, but would something like <div role="group" aria-label="Styles commands"> be appropriate too?
Asking for a friend 
There was a problem hiding this comment.
Good question — I only added the wrapping div for styling purposes rather than to flag anything semantically. Since there's only two controls at the moment, I wasn't sure how valuable having a group role would be for those two buttons?
Happy to add it in, though, if you think it's better to treat them as a group!
There was a problem hiding this comment.
Honestly I don't know. I think it's fine to leave as is. If we find out a group role is helpful it's easy to add in 👍
There was a problem hiding this comment.
Cheers, sounds good 👍
|
Size Change: +571 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
packages/edit-site/src/components/sidebar-navigation-screen-global-styles/index.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/sidebar-navigation-screen-global-styles/index.js
Outdated
Show resolved
Hide resolved
|
Oh, thanks for the good suggestion @ramonjd, I'll have a play! 🙇 |
|
Nicely done @ramonjd, your suggestion of using async functions and Thanks again, I'd entirely forgotten that dispatches return promises! 🎉 |
|
Flaky tests detected in 2e9a4f5. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4977045249
|
|
My thinking was that we would render the style book in the frame but not navigate to the editor. So that you can check different styles quickly against the style book. If you click on the frame, then you'd go into the editor. Can we try that? |
That's an interesting idea. Would you still be able to navigate the tabs, or as soon as you interact it opens the Edit view? |
|
No, the tabs wouldn't work in their current design. But I think we need to iterate there anyways and take them out of the frame, fwiw. |
andrewserong
force-pushed
the
add/style-book-button-in-styles-navigation-screen-in-browse-mode
branch
from
52cb96d
to
2e9a4f5
Compare
|
Thanks for the feedback and ideas, folks! I've had a play with seeing if we can render the Style Book in the canvas within browse mode. It seems to work kind of okay, with a few caveats. Here's how it's currently looking: 2023-05-15.15.53.54.mp4
Let me know what everyone thinks — if this seems like a good direction, I'm happy to keep tweaking / refining. At this stage, I'd probably lean toward seeing how we can use the Style Book pretty much as-is within this PR. If we're looking at redesigning the Style Book / removing the tabs, then we might want to split out that work into a separate PR. |
|
Catching up here, looks good. To Matías note, we need to explore a different interface than the tabs. That can happen separately. But the tabs being functional in the frame, also the close button, that isn't a great experience. Can we make it so a click anywhere on the frame, when the style book is open, goes to the fullscreen editor with the global styles panel, and the style book open? And can we hide the tabs in the mean time? |
|
Nice one!
Just to note that the close button can be hidden if that's desired
My assumption was that the foundational purpose of this screen was to allow the user to select a style variation on the left hand side. I can see the worth of being able to preview different blocks in each style variation, and therefore tab interactions, but clicking on a block and being taken to the site editor might detract from the primary purpose of the page: to select a style variation. |
Oh, thanks!
If we hide the tabs, would we expect to list all blocks out, or just those in the first tab, with the tabs hidden? I'll have a play around and see what's possible. Thanks again for the feedback and ideas, folks! |
|
Update: I think I've managed to get it working pretty well while hiding the tabs in the frame view: 2023-05-16.15.14.40.mp4In the latest update:
I'm sure there'll be other refinements and tweaks to do, too, but let me know how this feels for balance at the moment. |
|
Took this for a spin, nice work:
It helps a lot that the tabs are omitted, and it gives us the benefit that you can see the various style variations applied across the style book, before saving. However I just realized, that we might be able to skip this interim step. That is, go directly from this screen: 
to this screen: 
I should've noted this sooner, sorry, but there are a few benefits to this:
This also ties into some of the work @beafialho has been doing on the style book, where here's a screen that retires the tabs: 
I recognize the downside that you can't preview the style variations on the style book, and we can potentially revisit this. But it might be a simpler starting point? CC: @jameskoster |
|
I originally thought that's how this would work (deep link). It looks like that was the initial implementation, but it was later changed on request. Being able to preview the style variations and view all blocks at the same time seems quite handy to me, as it's not something currently possible in the Styles panel in Edit view. So I don't mind the toggle. Especially as we'll likely have a similar toggle on Template / Part list panels for moving between browse/manage views. Until we've done a design exploration that considers Styles to be a more discrete experience I don't have an especially strong opinion on this one. |
|
Ah, then let's go with this as is! No reason to hold this up. |
|
This looks really nice! It'd be interesting to have some styles for the |
andrewserong
force-pushed
the
add/style-book-button-in-styles-navigation-screen-in-browse-mode
branch
from
05c20d7
to
5e7783f
Compare
|
Thanks for the feedback, folks! I've given this PR a rebase and I believe it should be ready for a final review now.
Yes, it'd be good to take a look at the 
Very happy to help efforts to improve the style book — once the designs are ready, shall we open up a separate issue to track that?
Thanks — I can see good arguments between keeping the Style Book within the frame in browse mode (current state of this PR) and opening up edit mode (the original implementation in this PR). It won't be difficult to switch out the approach after merging if we decide we'd like to switch back to opening the full edit mode. Please let me know if everyone's happy with the current state of this PR, or if you'd like further iterations before landing 🙂 |
packages/edit-site/src/components/sidebar-navigation-screen-global-styles/index.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/sidebar-navigation-screen-global-styles/index.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I just left a couple of optional code changes. This is working well for me. 👍
2023-05-17.12.22.31.mp4
I can't find any regressions to the style book in canvas === 'edit' mode.
I agree it'd be handy for my 🧠 to have an is-pressed state on the style book button in the sidebar to indicate that it's open.
Co-authored-by: ramon <ramonjd@gmail.com>
|
Thanks for the re-review and suggestions @ramonjd! I'll leave this PR open overnight to see if there are any other requests from designers, and if not will merge this in tomorrow and/or follow-up with the |
|
Thanks for all the work. Agreed we should look at the toggled state separately and not let it block this one. We need that designed in a more careful way, as it's something we'll want across other toggle states in the dark material. I feel like @jameskoster may have some thoughts there as well, so I'll defer to him, but just as a baseline, something like this would feel more at home: 
I recognize that doesn't check all the boxes, all the more reason to explore it as a followup. |
There was a problem hiding this comment.
Great work @andrewserong. This is testing well for me, looks good to merge.
Only thing I noticed was the flickering from white of each style preview box in the sidebar when the styles sidebar first loads. It's a bit jankier than it could be. It'd be good to try and improve that in a follow up.
|
Thanks all! I'll merge this in now. Happy to continue tweaking in follow-ups, and please let me know if there were any other ideas you'd like me to look into 🙂 |
andrewserong
deleted the
add/style-book-button-in-styles-navigation-screen-in-browse-mode
branch
What?
Fixes: #50393
Add a button to the Styles left-hand navigation screen in the site editor Browse mode. Clicking the button opens the Style Book within the frame, displayed without tabs. Clicking anywhere within the Style Book frame opens up the edit mode with the Style Book open.
Why?
As raised in #50393, the goal is to make it easier for folks to open up the Style Book and see how styles look across blocks at a glance.
How?
Testing Instructions
Testing Instructions for Keyboard
Similar to above, but tab over to the eye icon from the Styles screen. Also, try tabbing all the way to the Style Book frame. You should be able to press Enter / Space to open up the edit mode.
Screenshots or screencast
2023-05-16.15.14.40.mp4