Site Editor: Add new 'Push changes to Global Styles' button #46446
Conversation
|
Size Change: +3.53 kB (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
|
Here's what this branch looks like. Thanks @ramonjd for hacking on it with me 😀 Kapture.2022-12-12.at.11.33.12.mp4To be honest I'm not so keen on this flow. The Push changes to Global Styles button is somewhat removed from where the changes are made and so there's a moment of what just happened? after you click the button. It isn't clear, I think, which styles will be pushed. I added a snackbar notification with information about which styles were pushed to try and alleviate this, but that doesn't make the button itself any less mysterious. I think it might be clearer what will happen if the Push changes button exists on each panel and only pertains to the styles within that panel? Perhaps using blue dots as in #44361 (comment)? or something else. A more technical limitation I faced is that there currently no way to have @jasmussen @jameskoster: Have a play with this and let me know what you think, if we should steer the ship in a different direction, or if my hesitations are misplaced and I should just continue with this approach! |
|
Nice, this seems remarkably close! As I understand, the button in advanced does this:
Is that correct? (If so, awesome) I got a small error when trying to push a change, though:
I got no snackbar, so I presume the change wasn't pushed. Was just trying to set all-caps on a navigation menu inside a template part: Two more questions:
|
Yeah that's right. To clarify on the first point: it resets any style changes made to the block (same as the Reset button in ToolsPanel does) and then makes those same changes to the block type (e.g. Heading) in Global Styles.
Will look into it. This was a pretty rough attempt so not surprising there are bugs 😀
Sure, will look into it. |
👌 |
I looked into this. Functionally, it works fine using the post editor's existing multi-entity save flow. One caveat is that it's a little strange that after pushing you won't be able to see those pushed changes in Global Styles because the post editor doesn't have the Global Styles panel. Maybe not a big deal—we could look at linking to the site editor. Technically speaking it's not a small task as as we have to move a few bits of
All achievable but something that I'd prefer to do in a follow-up PR as it requires careful consideration. |
noisysocks
changed the title
noisysocks
added
[Type] Feature
|
OK think this is ready for a proper review now. I updated the PR description with more details and added a basic E2E test since these sorts of features tend to attract regressions as they're not super visible. |
|
I don't think pushing styles in the post editor needs to block this PR, but it would be nice to have it in both, simply so you don't have to wonder why it's missing in one place. It also seems fine that there isn't necerssarily a preview; a) this is an advanced feature, and b) I think we can have previews in the multi entity saving flow in the future, which would likely benefit theme switching as well. As far as sharing code between the editors, it really would be nice if that were simpler. Conceptually they are mostly the same. |
|
I promise we'll circle back to the post editor @jasmussen! 😀 |
…set is selected. Getting a few Uncaught SyntaxError: "undefined" is not valid JSON for this function.
There was a problem hiding this comment.
Great work! 🙇
Pushing block styles to global is working pretty well for me. So too is the "undo" button in the notification.
One thing I noticed is that resetting styles doesn't work reliably.
2022-12-16.12.18.08.mp4
I rebased to include the change in #46486, but still couldn't crack it.
I think it might be related to #46505. If so, then it's in trunk, and can be addressed separately.
It only stands out in this particular feature because I wanted to reset the block styles that I'd pushed (and saved). 🤷
packages/edit-site/src/hooks/push-changes-to-global-styles/index.js
Outdated
Show resolved
Hide resolved
| disabled={ changes.length === 0 } | ||
| onClick={ pushChanges } | ||
| > | ||
| { __( 'Push changes to Global Styles' ) } |
There was a problem hiding this comment.
Not a showstopper, just asking:
Is "Global Styles" a term that we use in the editor? Would something like Apply styles globally communicate what we want to say?
There was a problem hiding this comment.
You're right, we don't call it Global Styles anywhere in the UI. How about "Apply styles to all Heading blocks"? (or "Move"?)
There was a problem hiding this comment.
Sounds grand.
I tried it out and we might need to adjust white space for some block names (taking into account i18n as well)
The following has, for example:
white-space: normal;
padding: $grid-unit-10;
height: auto;
There was a problem hiding this comment.
Hmmm it's so long, looks a bit ridiculous 😅
Other options:
Push changes to Styles (technically this is what we call Global Styles)
Push styles
Apply styles globally
There was a problem hiding this comment.
How about we make @jasmussen and @jameskoster decide? 😛
There was a problem hiding this comment.
How about we make @jasmussen and @jameskoster decide? 😛
Delegate. I like it.
Agreed, looks like the same issue. |
|
LGTM Let's iron out copy and reset stuff in a follow up |
Is there a follow-up issue for this already? I've been testing this on 14.9.0-rc1 and was pretty confused by this. I pressed 'Push changes to Global Styles' and a toaster said it had been pushed but when I looked at the site nothing had changed. Going back to the editor the button was still active, as if it hadn't worked. It took me five minutes to realise that 'Push changes to Global Styles' was only updating the editor state, and not actually saving anything. The button looks a bit like the save button and pushing sounds a bit like saving (to me who git pushes stuff all of the time at least). |
|
Thanks for the feedback @dsas! Pinging @jasmussen in case it sparks any ideas for improving the UI. There is no follow up issue for this but it is something I intend on iterating on now that am back from holidays. |
|
@noisysocks apologies I missed the discussion here, but there was another issue opened up over in #46928 about the naming for the button, and I've got a tiny PR up in #46965 to take a look at a small bit of renaming. |
| function cloneDeep( object ) { | ||
| return ! object ? {} : JSON.parse( JSON.stringify( object ) ); | ||
| } |
There was a problem hiding this comment.
I came across this code while testing a different PR.
@tyxla, if I recall correctly, we already have a similar utility method. Maybe we should use it here as well.
There was a problem hiding this comment.
Thanks for the reminder @Mamaduka!
This is going to be addressed soon as part of the Lodash removal. Instead of the set() usages I'm planning to use setImmutably() which already clones the object anyway, so there may be no need to clone it additionally.
There was a problem hiding this comment.
I had a feeling you already had a plan for this :)
What?
Alternative to #45961. Closes #44361.
Adds a new 'Push changes to Global Styles' button to the Advanced panel for all blocks in the Site Editor. Pressing it applies the block's typography, spacing, dimensions, and color styling to all blocks of that type in Global Styles.
Why?
See #44361.
How?
A new
editor.BlockEditfilter is added to@wordpress/edit-site. This filter renders the button in anAdvancedControlsfill.When the button is pressed, we iterate through the styles that are supported by the selected block type in Global Styles, and, for each one: 1) use
setAttribute()to unset the block style; 2) useGlobalStylesContextto move the style to Global Styles for that block type.It's difficult to do this in one undo level using the current
@wordpress/core-dataAPI so, for now, we create no undo levels and implement a custom Undo button in a snackbar notification.Testing Instructions
A basic E2E test is included. To test manually:
Follow up work
@wordpress/core-dataundo/redo functionality instead of rolling our ownNote: @noisysocks wrote this description, so don't judge @ramonjd if there are typos 😀