-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add global styles related commands #51637
Conversation
Size Change: +19.6 kB (+1%) Total Size: 1.42 MB
ℹ️ View Unchanged
|
Flaky tests detected in 2de0919. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5333722020
|
Quick GIF: Shows the 3 commands you outlined. I think reset works, as does the revisions panel. But "Learn about global styles" doesn't seem that valuable to me, for me that welcome guide is a "only see the first time" thing. It's not a strong opinion and I'll defer to @richtabor if he has a strong opinion. What it does show, however, is that it would be nice with a "Go to Global Styles" command instead, which is the same as going to Styles and clicking Edit. |
I can add that one |
I've added the "open styles" command. For the moment, none of these new commands is "contextual" (shown automatically when you open the command center), let me know if you want me to change that. |
I noticed that when running the "Reset styles to defaults" and "Open styles" commands, the modal does not close. Is that intentional? CleanShot.2023-06-19.at.17.58.38.mp4 |
I think it's fine to leave. Doesn't add any real weight and can be helpful. |
|
We don't necessarily need it for this command, but I do think we'll want a class of commands that can be toggle-able commands (which this could be included with). i.e. Toggle fullscreen mode, Toggle code editor, Toggle settings sidebar, Toggle list view, etc. |
Let's make "Open styles" suggested. How does order work with contextual suggestions? Is there a manual order? I'd like Open styles above Reset template. |
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.
Works great; let's add the contextual suggestion either here or in a follow up.
@richtabor Right now the "open styles" command, also navigates to the "styles" page (regardless of whether you're in "edit" or "view" mode), in other words, it's not just about opening the "styles" sidebar. If you want it to be that way, I guess it should be a command that is only visible in "edit" mode because otherwise it seems like we're talking about a different command here. |
I think it's fine to have it open the styles sidebar regardless of site or edit context, and then close it (without going back to the site view). |
|
||
useCommand( { | ||
name: 'core/edit-site/open-global-styles-revisions', | ||
label: __( 'Open styles revisions' ), |
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.
Let's change this to "Open style revisions".
Related: #51667 (comment)
The problem is when you're editing a template and you run the command, you're going to leave the template because the "styles" sidebar forces the home page. |
Why does the "Open styles" command force you to the home page? It doesn't force you when you interact with it via the UI. |
When you open "styles" in the site editor sidebar, it does. |
Iirc we added this to ensure you'd always see something moderately useful when the Styles panel was open. Otherwise it would be possible to have something like an isolated template part or navigation there, which isn't very helpful in the global styles context. |
Ok, to avoid delaying this PR and all the other commands, I'm merging this one but we can continue and improve the behavior of the "open styles" command or others as follow-ups. |
useCommandLoader( { | ||
name: 'core/edit-site/reset-global-styles', | ||
hook: useGlobalStylesResetCommands, | ||
} ); |
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.
Why does Reset styles to default use a command loader? Couldn't useCommonCommands
call useGlobalStylesReset
directly and then call useCommand
? useGlobalStylesReset
doesn't appear to do any asynchronous data loading.
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.
It's because of the canReset
check. Sometimes the command need to be "hidden". I did notice this pattern already and it made me wonder whether we should replace it with an isDisabled
option in the useCommand
hook to unregister the command once disabled or something like that.
} ); | ||
openGeneralSidebar( 'edit-site/global-styles' ); | ||
set( 'core/edit-site', 'welcomeGuideStyles', true ); | ||
// sometimes there's a focus loss that happens after some time |
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.
@youknowriad 👋🏻 when does the focus loss occur? Is this always a probelm with the welcome guide?
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.
I was not able to track it properly but I think the focus loss is probably happening as a response to openGeneralSidebar( 'edit-site/global-styles' );
which means the set( 'core/edit-site', 'welcomeGuideStyles', true );
we call after it is reverted to "false" due to the modal closing because of the focus loss.
Co-authored-by: Dean Sas <[email protected]>
Related #51502
What?
This PR adds three "styles" related commands to the command center in the site editor based on the list from #51502
All of these commands are only available in the site editor because the actions to trigger them are not tied to the URL and are edit-site specific. At the moment we can't implement these commands in a global way yet.
Testing Instructions
1- Open the site editor
2- Try the three commands above.