Copy changes for theme export back to core#2534
Copy changes for theme export back to core#2534scruffian wants to merge 23 commits intoWordPress:trunkfrom
Conversation
|
With the introduction of |
| $has_block_templates_dir = $zip->locateName( 'theme/templates/' ) !== false; | ||
| $has_block_template_parts_dir = $zip->locateName( 'theme/parts/' ) !== false; | ||
| $this->assertTrue( $has_theme_dir, 'theme directory exists' ); | ||
| $has_theme_json = $zip->locateName( 'theme.json' ) !== false; |
There was a problem hiding this comment.
This is a change that's not in Gutenberg
ajlende
left a comment
ajlende
left a comment
There was a problem hiding this comment.
I didn't have push access, so here's a patch for the defaultDutone notes.
|
Thanks, I have pushed these to the PR |
| * @return Bool Whether this file is in an ignored directory. | ||
| */ | ||
| function wp_is_theme_directory_ignored( $path ) { | ||
| $directories_to_ignore = array( '.git', 'node_modules', 'vendor' ); |
There was a problem hiding this comment.
Do we want to look for other VCS'? We check for array( '.svn', '.git', '.hg', '.bzr' ) in WP_Automatic_Updater.
There was a problem hiding this comment.
If we're now including the whole theme in the export, excluding vendor may be incorrect. It isn't always/only for development dependencies. So excluding it here would preclude using runtime composer dependencies.
This is something the WP Theme Review Team is doing, though maybe this would never happen for FSE themes.
There was a problem hiding this comment.
Do we want to look for other VCS'? We check for array( '.svn', '.git', '.hg', '.bzr' ) in WP_Automatic_Updater.
Done
There was a problem hiding this comment.
maybe this would never happen for FSE themes.
This is my hunch but I'm happy to remove it. The main motivation here is that TT2 has a vendor folder which is very large (30MB) so if people are using that as a base theme (which seems fairly likely), they will end up copying that every time. Happy to remove it though if you prefer.
There was a problem hiding this comment.
Yeah I definitely think we don't want to be zipping up 30MB here. I think your intuition is probably the best way to go. Maybe make the ignore directories list filterable?
| * @return WP_Theme_JSON Entity that holds theme data. | ||
| */ | ||
| public static function get_theme_data( $deprecated = array() ) { | ||
| public static function get_theme_data( $deprecated = array(), $settings = array( 'with_supports' => true ) ) { |
There was a problem hiding this comment.
We don't typically follow this pattern in Core. Traditionally this would be an empty array and use wp_parse_args to set defaults.
|
Left what ended up as general feedback, REST API changes look fine to me. |
Co-authored-by: Timothy Jacobs <timothy@ironbounddesigns.com>
Co-authored-by: Timothy Jacobs <timothy@ironbounddesigns.com>
…rdpress-develop into update/theme-export-for-6.0
ajlende
left a comment
ajlende
left a comment
There was a problem hiding this comment.
Changes for defaultDuotone look good to me. Thanks for taking care of that @scruffian!
oandregal
left a comment
oandregal
left a comment
There was a problem hiding this comment.
Export works as expected (appearanceTools and schema included) and code looks good.
|
I only wanted to mention that this patch includes also changes from WordPress/gutenberg#38132 and WordPress/gutenberg#40149 that were listed as requiring to backport to WP core. |
|
Committed with https://core.trac.wordpress.org/changeset/53129. Thank you everyone for the collaboration on this one ❤️ |
5 participants
Part of WordPress/gutenberg#39889
This bring across changes to theme export functionality, and related code, and tests.
Includes also changes from WordPress/gutenberg#38132.
Trac ticket: https://core.trac.wordpress.org/ticket/55505