-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 API to access global settings, styles, and stylesheet #34843
Conversation
cc @jorgefilipecosta @youknowriad what do you think of this? Context (last slack thread). |
get_settings_from_theme
function to get data from theme.json
get_settings_from_theme
function to get data from the theme's theme.json
That is very interesting and I agree that we need something like this. The second argument make me think a bit. Right now only potentially "contexts" are blocks but do we expect other kind of contexts later? Should we keep it, should we make it an object instead, should we remove it at first? |
The other things that comes to my mind is that in |
Yeah. Like what if we end up adding elements to the settings? In that case, the theme.json could look like: {
"settings": {
"color": {},
"blocks": {
"core/paragraph": {
"color": {},
"elements": {
"link": {}
}
}
},
"elements": {
"link": {
"color": {}
}
}
}
} But there's the question of other templates, template parts, etc. If the second argument is an object it can more easily grow/be modified. Alternatively, if we remove it we could grow by adding more functions when/if we need them: get_settings_from_theme( $path )
get_settings_from_theme_and_block( $path, $block )
get_settings_from_theme_and_element( $path, $element )
... It seems that the default experience should be the global settings and the other contexts should be secondary. |
We could update |
The discussion in Slack which suggested this was based on a use case where we need to pull out (for example) |
I wonder if we should drop |
939a6e6
to
3c27278
Compare
Size Change: +6 B (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
packages/editor/src/components/provider/use-block-editor-settings.js
Outdated
Show resolved
Hide resolved
3c27278
to
44e359a
Compare
get_settings_from_theme
function to get data from the theme's theme.json
cc @youknowriad @jorgefilipecosta this is ready for a review. |
It might be good to do some kind of schema about the dependencies between all these settings and APIs, I think it will clarify things for everyone. Happy to pair on that. |
44e359a
to
594abb1
Compare
594abb1
to
66dbe4c
Compare
I pushed some changes and this is ready to review @youknowriad @jorgefilipecosta Riad and I paired on this and the ability to pass theme supports settings to the functions is now removed. The settings were only needed when these functions were used in the Gutenberg plugin: the plugin hooks a callback to the |
Just want to say thanks for this; I was kinda stuck how to detect whether wide and full aligned blocks were enabled in a theme; and this let me do it by accessing I don't know enough to make a very informed judgement but I'd just point out that will further solidify the structure of theme.json. For example, if you (the lead developers) want to change the nesting levels of some existing settings in the future in theme.json, such changes could also break any scripts that would use this new API. |
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 really like these APIs, they hide all the complexity behind a comprehensible API which potentially could allow us to simplify/clarify the internals further in the future (aka WP_Theme_JSON_Resolver_Gutenberg)
Some unit tests for these APIs might be good to have later.
* | ||
* @return array The settings to retrieve. | ||
*/ | ||
function gutenberg_get_global_settings( $path = array(), $block_name = '', $origin = 'all' ) { |
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.
While right now, the $block_name is the only possible "context", I wonder if we'd have new kind of context in the future so wondering if this should be a decision factor for the API shape or if it's fine for now. (One possibility could be to use an object for the last two arguments in order to support more options in the future, but I'm hesitant).
In the frontend, I think the API is still internal so that's why it's not a problem there yet.
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.
* | ||
* @return array The styles to retrieve. | ||
*/ | ||
function gutenberg_get_global_styles( $path = array(), $block_name = '', $origin = 'all' ) { |
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.
The other question is whether we should support string dotted paths.
* trunk: (494 commits) remove consecutive rc warning (#35855) Update Changelog for 11.8.0-rc.2 Bump plugin version to 11.8.0-rc.2 [RNMobile] Disable React Native E2E Tests (iOS) (#35844) Add section about using the schema during development (#35835) Add a method to disable auto-accepting dialogs (#35828) Wrap NavigationContainer with SafeAreaView. (#35570) Update Appium to 1.22.0 (#35829) Post Comment: Handle the case where a comment does not exist (#35810) Clear selected block when clicking on the gray background (#35816) Post excerpt: Don't print the wrapper when there is no excerpt (#35749) [Block] Navigation: Fix padding for social links on mobile (#35824) Fix issue with responsive navigation causing wrapping. (#35820) [Block Editor]: Fix displaying only `none` alignment option (#35822) Add API to access global settings, styles, and stylesheet (#34843) Mobile Release v1.64.1 (#35804) Add resizer to template part focus mode (#35728) Update Changelog for 11.7.1 Gallery block: Only show the gallery upload error message if mixed multiple files uploaded (#35790) Update Changelog for 11.8.0-rc.1 ...
This is a proposal to add a public API for plugins/themes to extract data from the settings & styles instead of using the
WP_Theme_JSON_Resolver
which is marked as private. It's also arguably easier to reason about for consumers.The signatures
gutenberg_get_global_settings( $path = array() , $block_name = '', $origin = 'all' )
gutenberg_get_global_styles( $path = array() , $block_name = '', $origin = 'all' )
gutenberg_get_global_stylesheet( $type = '' )
Common use cases
Get all settings:
gutenberg_get_global_settings()
.Get layout settings for the root context:
gutenberg_get_global_settings( array( 'layout' ) )
.Get color settings for the paragraph block:
gutenberg_get_global_settings( array( 'color' ), 'core/paragraph' )
.Get all styles:
gutenberg_get_global_styles()
.Get the stylesheet:
gutenberg_get_global_stylesheet()
.Get the stylesheet corresponding to block styles:
gutenberg_get_global_stylesheet( 'block_styles' ) )
.Get the stylesheet corresponding to CSS vars:
gutenberg_get_global_stylesheet( 'css_variables' ) )
.Notes
This doesn't cover all the use cases we have in our code base. People also use the resolver to retrieve the template parts (see), which I intentionally left out can be added later if we want.
These functions can't be used in a callback to the
block_editor_settings
filter, like the one used in the Gutenberg plugin. The reason is that these new functions don't take the filtered theme supports.