FSE: Allow child theme.json to be merged with parent theme.json #35459
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
scruffian
requested review from
Mamaduka,
youknowriad,
oandregal,
aristath,
kjellr,
MaggieCabrera,
jffng and
jorgefilipecosta
This was referenced Oct 8, 2021
mtias
added
the
[Feature] Themes
aristath
reviewed
Oct 8, 2021
mtias
reviewed
Oct 8, 2021
Co-authored-by: Ari Stathopoulos <aristath@gmail.com>
Mamaduka
reviewed
Oct 8, 2021
Mamaduka
reviewed
Oct 8, 2021
Co-authored-by: George Mamadashvili <georgemamadashvili@gmail.com>
Co-authored-by: George Mamadashvili <georgemamadashvili@gmail.com>
|
👋 We need to cover and test a few scenarios here:
I was testing the second scenario and it reports an error because the parent's |
|
The current At #34843 I'm working on some code simplifications but we essentially have two code paths:
So I was thinking about how the "theme supports" of the parent theme fit in this new scenario. As far as I've tested, it seems theme supports are inherited by the child. I've tested this by creating a child of TwentyTwentyOne and verifying that the child still has support for line height and links as declared by the parent, as well as it takes palettes from the parent. I've also looked into Just wanted to bring it up to make sure others check this as well, in case I've missed anything. |
|
Follow-up on my comment above. I've tried to remove theme support in the child theme by doing: function twenty_twenty_one_child_setup() {
remove_theme_support( 'experimental-link-color' );
remove_theme_support( 'custom-line-height' );
}
add_action( 'after_setup_theme', 'twenty_twenty_one_child_setup' );but it didn't work. I still saw the link panel and the line-height control in the block sidebar. Not sure if I'm doing something wrong. |
Only use get_theme_file_path in theme_has_support check
|
👋 I just pushed some updates. It should fix the issue @oandregal reported when a parent has no theme.json, but the child does. Method changes
In ProgressCurrently working on integration tests for this scenario - "Parent & child have theme.json => merge child with a parent." I think other cases outlined by Andre are straightforward, and we can unit test those with the Re:
|
|
I added integration tests for parent/child theme data merging. Let me know if we want to cover more cases. The |
|
Size Change: 0 B Total Size: 1.07 MB ℹ️ View Unchanged
|
aristath
requested changes
Oct 18, 2021
aristath
approved these changes
Oct 18, 2021
@Mamaduka @aristath I've tried with this and it still doesn't work. This is my setup:
I've also tried adding a |
oandregal
reviewed
Oct 19, 2021
| @@ -255,4 +255,63 @@ function test_add_theme_supports_are_loaded_for_themes_without_theme_json() { | |||
| $this->assertSame( $color_palette, $settings['color']['palette']['theme'] ); | |||
| } | |||
|
|
|||
| function test_merges_child_theme_json_into_parent_theme_json() { | |||
There was a problem hiding this comment.
In #35759 I tweak a bit the test so that we test that single parts can be updated by the child while the untouched parent sections remain the same (the current tests checks that all the child sections override the parent).
|
@oandregal That's interesting. Let's create a separate issue for tracking. I'll try to have a look tomorrow morning. |
oandregal
added
the
Global Styles
|
Created #35762 |
|
@Mamaduka In the previous version, the |
|
Hi, @briceduclos. Thanks for contributing! What's the use case for filtering the The reason I removed |
|
@Mamaduka Thanks for the reply. I've built a plugin that includes a setting that allows the user to select a different theme.json file to change the site configuration. |
|
@briceduclos, thanks for the additional details. @oandregal, do we have a different filter that plugins can use to override theme.json settings? |
|
Plugins can use the existing filters we use in the Gutenberg plugin, see #27504 (comment) There's been a few places in which specific filters for global styles have been brought up, but there's been no progress on that front yet. The first step is landing #34843 that adds an API to get data out from theme.json. |
|
Thanks, @oandregal! |
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.
Description
If a theme has a parent theme, then this applies the child theme.json settings over the top of those for the parent theme.
Co-authored with @MaggieCabrera.
How has this been tested?
Checkout this PR: Automattic/themes#4807
The child theme behaves the same as before!
Types of changes
New feature (non-breaking change which adds functionality)
Checklist:
*.native.jsfiles for terms that need renaming or removal).