Global Styles: Allow opt out of certain sets of styles in the editor #61035
+211
−187
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
aaronrobertshaw
added
[Type] Enhancement
|
Size Change: +84 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
|
Just noting here that the documentation around how these global styles are generated needs some updating as flagged in #61026 (review). A task to do this has been added to the tracking issue for section styles. |
|
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. |
talldan
reviewed
Apr 29, 2024
| marginReset: true, | ||
| presets: true, | ||
| rootPadding: true, | ||
| ...styleOptions, |
There was a problem hiding this comment.
I think this will throw an error by default because styleOptions is undefined. Using a default value of {} or EMPTY_ARRAY instead of undefined might resolve it.
There was a problem hiding this comment.
Primitives can be spread into objects as well. All falsy values have no enumerable properties so nothing is spread.
Further proof there is that if you run the unit tests that don't supply the style options, there are no errors.
I'm not against updating the default value here just clarifying it doesn't throw an error as suggested.
I know the difference is completely negligible but is a default value of undefined ever so slightly less resource intensive than creating an empty object?
There was a problem hiding this comment.
You're right, my mistake, it's arrays where undefined can't be spread.
There was a problem hiding this comment.
Always good to challenge anything that looks odd. I appreciate the attention to detail 🙇
talldan
approved these changes
Apr 30, 2024
Does that mean we don't support "settings" in style variations or it's something we want to start with in a conservative manner, or what's the idea here? |
At this stage block style variations do not support their own "settings". The current enhancements to block style variations being worked on add the ability to style inner blocks/elements as well as nest the application of block style variations. To achieve this (primarily the nesting) #57908 generates styles per-application of a block style variation on a block and enqueues them. The root, layout, preset styles etc have already been generated and loaded by the primary theme.json/global styles stylesheet so they are superfluous in the context of the variation styles generated. It is on the radar that eventually block style variations would support their own settings definitions but for the time being it's out of scope for the current work. |
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.
Related:
Note: The changes in this PR are being split out from #57908 to make that more manageable
What?
Adds a new option param to the editor's
toStylesfunction that provides the ability to opt out of certain sets of styles.Why?
The extended block style variations in #57908 need to generate a stylesheet per application. The catch is they do not need all the root styles, layout, presets etc that would be generated without the ability to opt out of those.
Testing Instructions
npm run test:unit packages/block-editor/src/components/global-styles/test/use-global-styles-output.jsThe unit tests should pass.
Note: While reviewing this PR it might be easier to toggle off the whitespace changes.