Editor: Add global styles to settings using existing context code #61556
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
|
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. |
scruffian
reviewed
May 10, 2024
|
Size Change: +307 B (+0.02%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
|
Tested in the post editor and site editor using Poetry (WIP community theme) and this works as intended, looking at the code now |
ajlende
approved these changes
May 10, 2024
|
Thanks for the reviews, everyone 🙇 I think we can move forward with this approach. It's all behind private APIs / keys so we should be free to tweak further if required or further feedback comes in via the original PR #59929. |
scruffian
approved these changes
May 10, 2024
13 tasks
|
Nice, will this let us make progress on #37752? |
|
Will this have a negative impact on themes that currently rely silently for that "extra" space to be there if they don't update their |
@jasmussen, yes, this will give the post editor access to the merged global styles values. With that, the inspector controls could be updated to reflect those values as "defaults".
I'm not sure I follow sorry @remkus. Was this question related to a different PR? |
|
Nice, thanks. @youknowriad What do you think of progressing 37752? |
|
Yes, I think now, there's no technical blocker to advance with 37752. Just need to find a dev with availability to work on it. I believe both @michalczaplinski and @jorgefilipecosta have attempted this previously. So I wonder who's willing to give it a try. |
|
I'd be happy to help with or work on this one, after 6.6 once the section styling feature ships. |
I don't think so, @aaronrobertshaw. It was based on @richtabor linking to this GitHub issue in a reply on X about the discrepency between settings showing that the should be no margin/padding applied when they are, in fact, applied. And if this PR fixes that problem, I was wondering what the implications of this change would be recursively on existing FSE built sites. Probably good to read the whole thread I linked to. |
|
Thanks for the clarification and the link @remkus 👍
As I understand your comment, you are referring to the current UX issue where the styles from theme.json and global styles are not reflected in the block inspector controls. It is the value, or lack thereof in the controls, that is incorrect in this case rather than styles being applied that shouldn't be.
This PR does not address the UX issue in question directly. What it does do, is provide access to the style data from theme.json and global styles so it can be used regardless of the editor you are using (site or post editor). Now, with access to all the style data we are in a position to proceed with displaying the applied style values and improve this UX.
When the next step is taken and the block inspector controls display the style values being applied, nothing will change with regard to the actual display of sites. The styles already being applied won't change in any way, they would simply show up in their respective UI controls. For more context and history, I'd recommend taking a look at #37752. |
|
Excellent, thanks for the clarification and extra info. |
It's looks like that going to be me :D |
|
@Mamaduka just a quick heads up there's a small performance follow-up needed for the global styles data here so that it follows the same approach as I'm still catching up after being away but plan to address it early next week. |
|
@aaronrobertshaw Can you be more precise? make the setting a function? Why? |
@ellatrix suggested that we might wish to defer the collection and merging of global styles data on performance grounds. I haven't had the chance to measure what impact the suggested optimisation might have. I'm happy to leave this as is for now though if you think that best. The data is behind a private key so we should have some freedom to iterate on it later. |
|
I think global styles is different than patterns or things like that and are things that should probably be there from the initial rendering. So I think if there's no major performance downsides, I'd leave this as is. |
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.
What?
This PR is an alternative approach to #59929 for adding merged global styles data to the block editor settings.
Why?
The global styles data is required in the block editor settings so it is accessible within both editors for feature such as block style variations (section styling) and style inheritance updates in the UI.
Reasons for this alternate approach over #59929
How?
Testing Instructions
As this PR adds the data to a private key within the block editor settings, it can't easily be inspected via Redux dev tools.
The following snippet will log out the global styles data via a
console.debug()call when there is a block with an element style applied. From there you can easily confirm the correct data. Bonus points if you make tweaks in theme.json and global styles, then reconfirm that all gets merged properly.Test snippet