-
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
Accept transforms in gutenberg_get_global_styles context params #50484
Accept transforms in gutenberg_get_global_styles context params #50484
Conversation
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @samnajian! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
5977de8
to
1b1cb69
Compare
f5a641c
to
6b90136
Compare
9853971
to
5b201fc
Compare
Hey there! I just did a quick test of these changes to see what the response would be like and this would benefit the mobile editor a lot since we won't need to go through the data and manually parse it (which on mid-end devices could take a toll on performance). I can't add too much value to the code review though since my PHP knowledge is limited 😅 but I like that it still uses To test it I updated the mobile endpoint to pass the transforms: $context = array(
'transforms' => array(
'resolve-variables',
),
);
$settings['__experimentalStyles'] = gutenberg_get_global_styles(array(), $context); Thank you for working on this @samnajian 👏 |
3e03386
to
14e536c
Compare
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.
This is brilliant work, Sam! Thanks for all your effort here. There's only one use case we need to fix, but otherwise it's working as expected.
2f7dbb0
to
ae2ca1f
Compare
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.
This is ready to be merged. Thanks for pushing until it was feature complete, Sam! It was harder than anticipated, and you did great.
I see some lint issues that need fixing before merging, though.
The function is defined to return styles with values instead of variables
Once transforms => array( 'resolve_variables' ) is defined the css variables are resolved to their values
…olve_style_values This will allow a more composable interface to this class
ae2ca1f
to
bcb6c73
Compare
Thanks everyone for working on this. I'll add it to the backport list for WP 6.3 🙇 |
Backporting at WordPress/wordpress-develop#4656 |
What?
This PR addresses the changes explained in #49712 by introducing the
transforms
to the$context
param as form of an array, so thatreturns:
and not the following:
for a
theme.json
with the following content:Why?
There are some usages of the
gutenberg_get_global_styles
where the user is interested in the values of the css rules and not the variables.How?
Currently the acceptable value is only
resolve-variables
as inand in future PRs other transforms can be added such as
Testing Instructions
Paste the following in
functions.php
of the theme: