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

theme.json: add appearanceTools flag to opt-in into appearance UI controls #36646

Merged
merged 10 commits into from Nov 23, 2021

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented Nov 19, 2021

Fixes #36187
Follow-up to #36246 and #36477

This PR adds a new setting property in theme.json called appearanceTools whose function is to opt-in into all appearance UI controls.

The result is that a theme.json like this one:

{
  'version': 2,
  'settings': {
    'appearanceTools': true
  }
}

is the same as this other:

{
  'version': 2,
  'settings': {
    'border': {
      'color': true,
      'radius': true,
      'style': true,
      'width': true
    },
   'color': {
     'link': true
   },
    'spacing': {
      'blockGap': true,
      'margin': true,
      'padding': true
    },
    'typography': {
      'lineHeight': true
    } 
  }
}

Themes that don't use this flag need to individually enable the opt-in features, which is the same behavior as of now in trunk.

Notes

Name

After some discussion in the first PR #36246 and consulting other people, we're going with appearanceTools for the name, which leaves appearance free for the future and clarifies the purpose of the flag.

Cascade behavior

Themes can still set anything to false and will be respected. So, this theme.json:

{
  'version': 2,
  'settings': {
    'appearanceTools': true,
    'typography': {
      'lineHeight': false
    }
  }
}

is the same as:

{
  'version': 2,
  'settings': {
    'border': {
      'color': true,
      'radius': true,
      'style': true,
      'width': true
    },
    'color': {
      'link': true
    },
    'spacing': {
      'blockGap': true,
      'margin': true,
      'padding': true
    },
    'typography': {
      'lineHeight': false
    } 
  }
}

@oandregal oandregal changed the title theme.json: add appearanceTools flag to opt-in into appareance settings theme.json: add appearanceTools flag to opt-in into appareance UI controls Nov 19, 2021
@oandregal oandregal self-assigned this Nov 19, 2021
@oandregal oandregal added [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Nov 19, 2021
@oandregal oandregal added this to 👀 Needs review in WordPress 5.9 Must-Haves via automation Nov 19, 2021
@oandregal oandregal added the Backport to WP Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Nov 19, 2021
@oandregal oandregal changed the title theme.json: add appearanceTools flag to opt-in into appareance UI controls theme.json: add appearanceTools flag to opt-in into appearance UI controls Nov 19, 2021
@overclokk
Copy link

What happen with a child theme?

@oandregal
Copy link
Member Author

What happen with a child theme?

Same as before: the child overrides the parent. Did you run into any issue?

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

This looks good to me personally.

Let's not forget to backport the theme.json unit tests updates that happened since WP 5.8 into WordPress trunk

@oandregal oandregal merged commit 1e78e92 into trunk Nov 23, 2021
WordPress 5.9 Must-Haves automation moved this from 👀 Needs review to ✅ Done Nov 23, 2021
@oandregal oandregal deleted the add/appearance-tools-flag branch November 23, 2021 10:01
@github-actions github-actions bot added this to the Gutenberg 12.1 milestone Nov 23, 2021
@oandregal
Copy link
Member Author

Let's not forget to backport the theme.json unit tests updates that happened since WP 5.8 into WordPress trunk

@youknowriad what do you mean by this?

@youknowriad
Copy link
Contributor

I mean that during the 5.9 lifecycle we did some changes to theme.json php unit tests like in this PR and we might not have back ported everything to Core.

Since at some point we will remove this code from Gutenberg (theme.json classes and tests), we should make sure to backport all the tests to avoid losing coverage.

@oandregal
Copy link
Member Author

FWIW, all of them should be already in core, that was part of the initial backporting (except for the PRs that still need to backported, such as this one).

@overclokk
Copy link

@oandregal not tryed yet on child theme.
I want to test in case a parent and child have different version of the schema.

@oandregal
Copy link
Member Author

Everything should work as expected, but wider and extensive testing is always appreciated :)

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. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Consider changing some theme.json settings to opt-out
4 participants