-
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
Specify what settings can be part of settings.layout
#33303
Conversation
@@ -92,7 +92,10 @@ class WP_Theme_JSON_Gutenberg { | |||
'palette' => null, | |||
), | |||
'custom' => null, | |||
'layout' => null, | |||
'layout' => array( |
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 think later we'll have "type" and a different shape of layout depending of the "type". Maybe it's fine for now to have this but I thought I'd mention in case it's any impactful.
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.
When/If we have those new fields we can add them all and/or have some specific logic for layout
to make sure only the ones that make sense depending on type are allowed.
By porting this to WordPress core we won't allow anything else than what it's shipped in v1 (the two fields in use atm) without the plugin. With the plugin, consumers will have those new things available. Makes sense?
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.
WFM 👍
5a7da4e
to
3493a0a
Compare
Stems from #33280
This PR makes it so that only the specific allowed settings can be part of
settings.layout
.