Site editor: integrate global styles controls and style book preview into the styles panel #65619
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +1.03 kB (+0.06%) Total Size: 1.82 MB
ℹ️ View Unchanged
|
|
Great start. I feel this gives already a good sense of pros and cons. Here's some remarks about the experience.
But globally, I think this feels way better than what we have on trunk. We should try to gather more feedback here from everyone @WordPress/gutenberg-design |
youknowriad
added
[Type] Enhancement
|
Nice work. There are some strong aspects to this, mainly in that it places global changes in context of your site overall, rather than in context of editing an individual page or template as is the case now. This, in turn, can also benefit the template editing experience where sometimes you'll confuse the block inspector with the global styles inspector. The white second panel takes up a lot of space, I like to think there are some longer term things we can do here, possibly even collapse the dark material on the left, or show some or all of the style pieces in the dark material (potentially pending #56669). But that's a future thing, and the white second column at the moment makes it possible to make good progress on this with speed. Revisions, custom CSS, revert, these are pieces we need to port over in the short term, but also elevate better in the longer term. Notably revisions is something that can potentially benefit from being thought of as part of the save dialog, or just the save button itself. CC: @auareyou as I know you've looked at the save dialog. One thing we need to do is decide what clicking on items in the frame does. Does it open the fullscreen editor? Or does it select individual block types? If it's the latter, then it should navigate the the appropriate block in the 2nd column, just like it does in trunk. We need the inverse too, perhaps not in this branch but eventually: when you click "Blocks → Image", we should load only the image block preview in the frame on the right. This is going to especially benefit interfaces as those proposed here, where the full range of block states can be shown: hover, active, focus, etc. This way of making the preview frame contextual to what is selected in the second column can also potentially be leveraged to benefit what's being outlined in #53431:
But a first step might be to simply load a particular block in the 2nd column, when it is selected in the interface on the right. Also, should we make this an experiment so it can be tested easily? |
I think we should make this an experiment if we judge that the proposal is a temporarily worse experience than trunk. To be honest, I'm not entirely sure :P I feel like this is already better than trunk. But I also understand if you want to keep it "more hidden" until we iron out more things. |
|
For me, we could make the following update as well:
|
Oh right yes, the plugin is also a way to test. I'm super into not hiding this behind an experiment, though if that's the case, we do need to nail down the interactivity, or lack of, in the frame itself. That plus the contextuality of the frame showing categories contextually to what you select on the left + porting over the revisions, resets, etc. Another thing that's important to consider is: right now there's a use case, potentially common, of seeing your site or another template, as you edit your global styles. Should there be a toggle, like an inverted version of this button, to let you see your main site template instead?
|
|
So, we need to decide if this should be an experiment or something we can merge and then improve upon. What are the minimum viable changes that are required to merge an initial PR? Would making the styles sidebar show all the other menu items with styles selected plus a button to show the home template be enough? For an initial version? It would then allow us to work in parallel with multiple follow-ups. |
jorgefilipecosta
force-pushed
the
update/make-styles-open-the-style-book-with-a-left-styles-sidebar
branch
from
176a36d to
9309e34
Compare
|
This is looking very cool! Just pinging @ramonjd and @aaronrobertshaw for visibility as they're also looking at some Style Book work (though more focused on the body of the Style Book, whereas this is exposing the Styles section from global styles, next to the Style Book). |
|
I am just testing at end of my day. I think it's a great start to make global styles more accessible. Thank you!
I agree this would be a helpful feature to make editing block global styles easier.
As part of the work going on in #64707, color (and possibly) typography preset previews will also be rendered to the style book. Layout would be a nice additions. What do folks think about the position of the popovers? It doesn't bother me personally, however for some reason my brain expects them to hover over the canvas. Seems more feng shui for the popovers to point in the direction of the canvas. It's a small thing however right now. 
As others have mentioned, I reckon:
Later on:
|
I agree it seems more natural on the right. I think it can be a follow-up though, I can see a "position" prop exposed on the GlobalStylesUI component. |
This message should be removed (or alternatively, we should try not navigating the sidebar entirely) |
|
Thank you all for the reviews.
The popovers should go to the right but I think that can be a simple follow-up. There are many follow-ups and things we can do but I think it would be nice to merge this one as soon as possible as it open the door to many tasks we can work on in parallel/independent PR's. |
|
Hi @youknowriad the feedback was applied. |
|
This looks like a promising start to me. A couple of high-level pieces of feedback:
|
|
Hi @jameskoster, I removed the frame interaction and resizing and made changes to the styles menu to prevent it from drilling down. Now it just opens a panel.
I'm a bit confused about what needs to be fixed. I currently don't see any horizontal scrolling on the styles panel. Could you possibly send a screenshot of the issue that needs to be addressed? Thank you a lot! |
| preview: <Editor />, | ||
| mobile: hasEditCanvasMode && <Editor />, | ||
| content: <GlobalStylesUIWrapper />, | ||
| preview: <Editor isPreviewOnly />, |
There was a problem hiding this comment.
For some reason, when I navigate to the "styles" menu, the frame doesn't animate anymore (you don't see it moving to the right), I'm pretty sure it used to animate earlier in the PR
There was a problem hiding this comment.
Hi @youknowriad, that was intentional following the feedback from @jameskoster "The "Styles" menu item should not be a drill-down, it should just open the styles panel on click.".
I think removing the drill-down removes the animation of the panel opening, I think we can not easily avoid having the drill down and also have the animation when the panel opens.
There was a problem hiding this comment.
I think removing the drill-down removes the animation of the panel opening, I think we can not easily avoid having the drill down and also have the animation when the panel opens.
Why, how is the drill down related to the animation?
There was a problem hiding this comment.
I can easily re-add the animation by changing key from default to styles:
if (path && path.startsWith('/wp_global_styles')) {
return {
key: 'styles',
The problem is that doing this doesn't just animate the style panel, but also animates the main "Design" area, which feels strange because there is no change in that part, only the styles item gets the blue background.
It seems that we can either animate the "Design" section and the style area, or we don't animate any of the areas. I don't see an option to not animate the style area but animate the styles panel.
There was a problem hiding this comment.
Maybe it's better to avoid animating based on the key, but let's not get blocked with that here, we can look at the animation separately.
jorgefilipecosta
force-pushed
the
update/make-styles-open-the-style-book-with-a-left-styles-sidebar
branch
from
fb01dc7 to
3525c68
Compare
Disable animation for style view
I've done this for now. Here's the updated state: 2024-11-02.14.50.58.mp4Proposed next steps:
One thing to note is that trunk still has stylebook and edit icons: 
The other option for this PR is to keep the site/stylebook toggle, and allow canvas clicking to navigate to the editor for both. |
I read the feedback as switching what we default to, to being the editor / template preview, rather than removing the Style Book toggle altogether? That said, I agree if it helps land this PR sooner, we can add the toggle back in, in a follow-up 👍
That fix is feeling nice to me! |
Oh yeah, I misread. Thanks for pulling me up on that @andrewserong I can revert. I also perceived things were slowing down around how a user should switch between the two views (site and style book), as well as the copy, so that was also my motivation to move the conversation elsewhere. Whatever folks think is better. |
Reverted in 8902fa3 |
packages/edit-site/src/components/sidebar-global-styles-wrapper/index.js
Show resolved
Hide resolved
Looking good to me! 2024-11-04.16.16.42.mp4 |
There was a problem hiding this comment.
Great effort across the board on this one, everyone. Thanks for continuing to drive it forward @ramonjd.
It is testing well for me and appears to address nearly all the feedback received so far. I've left a couple of minor nits as inline comments below.
I read the feedback as switching what we default to, to being the editor / template preview, rather than removing the Style Book toggle altogether?
This was my read as well. Thanks for reverting the other change there.
Defaulting to the site preview does appear to make it harder to fall into the trap of not easily getting back to the site preview and entering the editor. Good enough to land this PR with that approach, I think.
In case it helps, here's a small diff with suggested tweaks from the comments below.
Quick diff, feel free to take it or leave it 🤷
diff --git a/packages/edit-site/src/components/sidebar-global-styles-wrapper/index.js b/packages/edit-site/src/components/sidebar-global-styles-wrapper/index.js
index 356385a10b..afa9f489dd 100644
--- a/packages/edit-site/src/components/sidebar-global-styles-wrapper/index.js
+++ b/packages/edit-site/src/components/sidebar-global-styles-wrapper/index.js
@@ -30,7 +30,7 @@ const GlobalStylesPageActions = ( {
return (
<Menu
trigger={
- <Button __next40pxDefaultSize variant="secondary">
+ <Button __next40pxDefaultSize variant="tertiary" size="compact">
{ __( 'Preview' ) }
</Button>
}
@@ -38,7 +38,7 @@ const GlobalStylesPageActions = ( {
<Menu.RadioItem
value
checked={ isStyleBookOpened }
- name="radio-controlled"
+ name="styles-preview-actions"
onChange={ () => setIsStyleBookOpened( true ) }
defaultChecked
>
@@ -50,7 +50,7 @@ const GlobalStylesPageActions = ( {
<Menu.RadioItem
value={ false }
checked={ ! isStyleBookOpened }
- name="radio-controlled"
+ name="styles-preview-actions"
onChange={ () => setIsStyleBookOpened( false ) }
>
<Menu.ItemLabel>{ __( 'Site' ) }</Menu.ItemLabel>
diff --git a/packages/edit-site/src/components/sidebar-navigation-item/style.scss b/packages/edit-site/src/components/sidebar-navigation-item/style.scss
index f9f3a01868..202de53000 100644
--- a/packages/edit-site/src/components/sidebar-navigation-item/style.scss
+++ b/packages/edit-site/src/components/sidebar-navigation-item/style.scss
@@ -28,8 +28,6 @@
&.with-suffix {
padding-right: $grid-unit-20;
}
-
-
}
.edit-site-sidebar-navigation-screen__content .block-editor-list-view-block-select-button {
packages/edit-site/src/components/sidebar-global-styles-wrapper/index.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/sidebar-global-styles-wrapper/index.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/sidebar-navigation-item/style.scss
Outdated
Show resolved
Hide resolved
|
Thanks again for testing again folks. @aaronrobertshaw great suggestions. Done all round. |
|
I'd make the preview button a tertiary button, compact size. Otherwise, congrats for bringing this forward! |
This was addressed in 4d5a7b5 after my earlier review. |
|
Fantastic, thank you! |
ramonjd
changed the title
ramonjd
deleted the
update/make-styles-open-the-style-book-with-a-left-styles-sidebar
branch
…into the styles panel (WordPress#65619) This commit integrates global styles controls and a style book preview into the styles panel. This affects the site editor in view mode. A toggle allows users to switch between previewing the site and the style book while editing global styles. --------- Co-authored-by: jorgefilipecosta <[email protected]> Co-authored-by: ramonjd <[email protected]> Co-authored-by: youknowriad <[email protected]> Co-authored-by: tellthemachines <[email protected]> Co-authored-by: ntsekouras <[email protected]> Co-authored-by: aaronrobertshaw <[email protected]> Co-authored-by: jasmussen <[email protected]> Co-authored-by: andrewserong <[email protected]> Co-authored-by: jameskoster <[email protected]> Co-authored-by: annezazu <[email protected]> Co-authored-by: richtabor <[email protected]> Co-authored-by: mtias <[email protected]> Co-authored-by: afercia <[email protected]>
|
Thank you all for bringing this to the finish line 🥳 |
Part of: #53483
Closes #66376
This PR is a first step towards this vision proposed by @jameskoster and @jasmussen:
styles.mp4
Global styles controls are now shown in the style panel. The default preview is "Site", but users can activate the style book as well.
Screenshots
This PR:
2024-11-04.16.06.21.mp4
Next steps