Skip to content
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

Enable appearance tools via theme_support #43337

Merged
merged 6 commits into from
Aug 22, 2022
Merged

Conversation

pbking
Copy link
Contributor

@pbking pbking commented Aug 17, 2022

What?

Allow a theme to enable appearanceTools to any theme including classic themes.

Why?

Appearance tools are options in the block editor and don't require the use of the FSE to take advantage of. However there's no way to enable those tools for themes that don't use theme.json.

How?

This checks for the 'appearance-tools' support and if present flags settings.appearanceTools as true in the theme portion of Global Styles.

Testing Instructions

Using a theme that does NOT use a theme.json file enable the 'appearanceTools` theme support.

add_theme_support( 'appearance-tools' );

Add a group block to a new post. Note the block settings panel includes Border settings.

Copy link
Member

@mikachan mikachan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ace, thank you for working on it! It tests well for me ✨

There is a list of theme features in this theme.ts file, I'm not sure if we need to add appearance tools there as well?

@pbking
Copy link
Contributor Author

pbking commented Aug 18, 2022

There is a list of theme features in this theme.ts file, I'm not sure if we need to add appearance tools there as well?

Good call, yea.

@pbking pbking requested a review from nerrad as a code owner August 18, 2022 12:59
Fix spelling and punctuation.

Co-authored-by: Jeff Ong <jonger4@gmail.com>
@pbking pbking merged commit f695d8c into trunk Aug 22, 2022
@pbking pbking deleted the enhance/appearanceTools branch August 22, 2022 20:19
@github-actions github-actions bot added this to the Gutenberg 14.0 milestone Aug 22, 2022
@carolinan
Copy link
Contributor

@pbking Did you already open a trac issue for adding the theme support to the bundled themes?

@jffng
Copy link
Contributor

jffng commented Sep 1, 2022

https://core.trac.wordpress.org/ticket/56487#ticket

@pbking
Copy link
Contributor Author

pbking commented Sep 21, 2022

Once this change landed in the plugin we implemented it in all of the Varia themes. In part to provide more options for users of those themes and in part to evaluate the feature.

We found that, mostly due to the fact that this enables block gap in layout blocks with a default value that the spacing and padding of all of the themes changed due to enabling this feature. As far as I know/understand there's no way to limit what features are enabled and since that feature effects a site without user action makes it feel like a dangerous option to use.

I'm hesitant recommending that we enable this feature on core bundled themes; at least not without careful evaluation of each when it's enabled. A better scenario would be more fine-grained control of the features... but that seems more like adding a theme.json file would be the tool for that job (which also causes layout changes to a theme which is why the theme_support option was explored).

@carolinan
Copy link
Contributor

carolinan commented Sep 21, 2022

Allowing themes to enable the theme support is separate from adding it to the default themes. The theme support would still have been a big help for new classic themes where the spacing would not be an unexpected change.

@colorful-tones
Copy link
Member

@pbking Will this be a part of the backport for 6.1? I'm currently testing this in WordPress 6.1 beta1, but it does not work unless I enable the Gutenberg plugin.

@carolinan
Copy link
Contributor

I do not know why it was not included but we are past the feature freeze for 6.1

@annezazu annezazu added Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. labels Sep 26, 2022
@annezazu
Copy link
Contributor

@ockham can we consider this for backport please? :)

@ockham
Copy link
Contributor

ockham commented Sep 27, 2022

@ockham can we consider this for backport please? :)

I've filed one now: WordPress/wordpress-develop#3337

Note that since this affects PHP code in lib/, it requires a manual backport -- that's also why it didn't end up in Core automatically (e.g. via a @wordpress/ package sync).

This might not be the easiest to get into 6.1, since Core is only accepting bugfixes at this point. If you'd like to see this accepted into Core, y'all might wanna campaign for it over at WordPress/wordpress-develop#3337 😅

@ockham
Copy link
Contributor

ockham commented Sep 27, 2022

Removing the "Backport to WP Beta/RC" label; nothing left to do with this PR, the backport is entirely covered by WordPress/wordpress-develop#3337.

@ockham ockham removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Sep 27, 2022
@bph bph added Needs Dev Note Requires a developer note for a major WordPress release cycle and removed Needs Dev Note Requires a developer note for a major WordPress release cycle labels Oct 8, 2022
@bph
Copy link
Contributor

bph commented Oct 8, 2022

Punted to 6.2
see comment on the trac issue https://core.trac.wordpress.org/ticket/56487#comment:16

@bph
Copy link
Contributor

bph commented Oct 8, 2022

needs dev note label for 6.2

@bph bph added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Oct 8, 2022
oandregal added a commit that referenced this pull request Dec 22, 2022
Port changes to the base class that were meant to be part of 6.1.
See #43337
oandregal added a commit that referenced this pull request Dec 22, 2022
… inheriting per WordPress version (#46750)

* Backport WP_Theme_JSON_Resolver from core as it is

* Substitute WP_Theme_JSON by WP_Theme_JSON_Gutenberg

* Substitute WP_Theme_JSON_Data by WP_Theme_JSON_Data_Gutenberg

* Rename WP_Theme_JSON_Resolver to WP_Theme_JSON_Resolver_Base

The goal is to use it as base to inherit from, until we are able
to remove all the children.

* Make WP_Theme_JSON_Resolver_6_1 inherit from WP_Theme_JSON_Resolver_Base

* 6.1: remove field already defined in base class

* 6.1: remove translate, it is equal to base method

* 6.1: remove get_core_data, it does not have base changes

* 6.1: remove get_user_data

Missed core changes and does not do anything differently from core.

* 6.1: remove get_user_data_from_wp_global_styles

It misses core changes and does not do anything differently.

* 6.2: inherit from WP_Theme_JSON_Resolver_Base instead of 6.1

This makes the WP_Theme_JSON_Resover_6_1 class unused.

* 6.1: remove class no longer in use

* 6.2: deprecate theme_has_support

#45380

* 6.2: substitute WP_Theme_JSON_Resolver::theme_has_support

by wp_theme_has_theme_json()

#45168

* 6.2: port changes to get_user_data_from_wp_global_styles

#46043

* 6.2: update get_merged_data

#45969

* 6.2: remove get_user_data

Same code as core. There's a check for detecting whether the class
is an instance of WP_Theme_JSON_Gutenberg that was not ported.
This check was introduced to make sure the cache of the core class
didn't interfere with the cache of the Gutenberg class,
so it's no longer necessary.

See #42756

* experimental: make it inherit from WP_Theme_JSON_Resolver_Base

* 6.2: remove class no longer in use

* experimental: delete remove_json_comments

It's already part of the base class.

* experimental: remove get_block_data

It's already part of the base class and it didn't have
the changes from core.

* experimental: appearanceTools

Port changes to the base class that were meant to be part of 6.1.
See #43337

* experimental: port webfonts (experimental API)

see #37140

* experimental: use gutenberg_get_legacy_theme_supports_for_theme_json

#46112

* experimental: get_theme_data, all code is in the base class

* experimental: remove empty class

* Rename WP_Theme_JSON_Resolver_Base to WP_Theme_JSON_Resolver_Gutenberg

* Fix lint issue: rename class-wp-theme-json-resolver.php to class-wp-theme-json-resolver-gutenberg.php

* Move theme.json to unversioned lib/

* Move theme-i18n.json to unversioned lib/
noahtallen pushed a commit that referenced this pull request Dec 28, 2022
… inheriting per WordPress version (#46750)

* Backport WP_Theme_JSON_Resolver from core as it is

* Substitute WP_Theme_JSON by WP_Theme_JSON_Gutenberg

* Substitute WP_Theme_JSON_Data by WP_Theme_JSON_Data_Gutenberg

* Rename WP_Theme_JSON_Resolver to WP_Theme_JSON_Resolver_Base

The goal is to use it as base to inherit from, until we are able
to remove all the children.

* Make WP_Theme_JSON_Resolver_6_1 inherit from WP_Theme_JSON_Resolver_Base

* 6.1: remove field already defined in base class

* 6.1: remove translate, it is equal to base method

* 6.1: remove get_core_data, it does not have base changes

* 6.1: remove get_user_data

Missed core changes and does not do anything differently from core.

* 6.1: remove get_user_data_from_wp_global_styles

It misses core changes and does not do anything differently.

* 6.2: inherit from WP_Theme_JSON_Resolver_Base instead of 6.1

This makes the WP_Theme_JSON_Resover_6_1 class unused.

* 6.1: remove class no longer in use

* 6.2: deprecate theme_has_support

#45380

* 6.2: substitute WP_Theme_JSON_Resolver::theme_has_support

by wp_theme_has_theme_json()

#45168

* 6.2: port changes to get_user_data_from_wp_global_styles

#46043

* 6.2: update get_merged_data

#45969

* 6.2: remove get_user_data

Same code as core. There's a check for detecting whether the class
is an instance of WP_Theme_JSON_Gutenberg that was not ported.
This check was introduced to make sure the cache of the core class
didn't interfere with the cache of the Gutenberg class,
so it's no longer necessary.

See #42756

* experimental: make it inherit from WP_Theme_JSON_Resolver_Base

* 6.2: remove class no longer in use

* experimental: delete remove_json_comments

It's already part of the base class.

* experimental: remove get_block_data

It's already part of the base class and it didn't have
the changes from core.

* experimental: appearanceTools

Port changes to the base class that were meant to be part of 6.1.
See #43337

* experimental: port webfonts (experimental API)

see #37140

* experimental: use gutenberg_get_legacy_theme_supports_for_theme_json

#46112

* experimental: get_theme_data, all code is in the base class

* experimental: remove empty class

* Rename WP_Theme_JSON_Resolver_Base to WP_Theme_JSON_Resolver_Gutenberg

* Fix lint issue: rename class-wp-theme-json-resolver.php to class-wp-theme-json-resolver-gutenberg.php

* Move theme.json to unversioned lib/

* Move theme-i18n.json to unversioned lib/
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Jan 14, 2023
This changeset merges the following changes from Gutenberg repository: 

- Allow themes without `theme.json` to opt-in to appearance tools via `add_theme_support( 'appearance-tools' );`
- Update `wpThemeJsonResolver` unit tests accordingly

See the following pull request for more info: WordPress/gutenberg#43337

Props ironprogrammer, audrasjb.
Fixes #57460.

Built from https://develop.svn.wordpress.org/trunk@55067


git-svn-id: http://core.svn.wordpress.org/trunk@54600 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot pushed a commit to gilzow/wordpress-performance that referenced this pull request Jan 14, 2023
This changeset merges the following changes from Gutenberg repository: 

- Allow themes without `theme.json` to opt-in to appearance tools via `add_theme_support( 'appearance-tools' );`
- Update `wpThemeJsonResolver` unit tests accordingly

See the following pull request for more info: WordPress/gutenberg#43337

Props ironprogrammer, audrasjb.
Fixes #57460.

Built from https://develop.svn.wordpress.org/trunk@55067


git-svn-id: https://core.svn.wordpress.org/trunk@54600 1a063a9b-81f0-0310-95a4-ce76da25c4cd
@bph bph mentioned this pull request Feb 6, 2023
47 tasks
@carolinan carolinan removed the Needs Dev Note Requires a developer note for a major WordPress release cycle label Feb 22, 2023
@carolinan
Copy link
Contributor

I have removed the needs dev note label because this feature is not in 6.2:
https://core.trac.wordpress.org/ticket/57649

@oandregal
Copy link
Member

I've noticed this feature was backported to 6.2 as of https://core.trac.wordpress.org/ticket/56487 and had to be reverted as of https://core.trac.wordpress.org/ticket/57649

In #48622 I'm marking this code not to backport until the issues are resolved.

VenusPR added a commit to VenusPR/Wordpress_Richard that referenced this pull request Mar 9, 2023
This changeset merges the following changes from Gutenberg repository: 

- Allow themes without `theme.json` to opt-in to appearance tools via `add_theme_support( 'appearance-tools' );`
- Update `wpThemeJsonResolver` unit tests accordingly

See the following pull request for more info: WordPress/gutenberg#43337

Props ironprogrammer, audrasjb.
Fixes #57460.

Built from https://develop.svn.wordpress.org/trunk@55067


git-svn-id: http://core.svn.wordpress.org/trunk@54600 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants