The Wayback Machine - https://webcf.waybackmachine.org/web/20211216012707/https://github.com/WordPress/gutenberg/pull/36646
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 on Nov 23, 2021

Conversation

@oandregal
Copy link
Member

@oandregal oandregal commented on 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 on Nov 19, 2021
@oandregal oandregal self-assigned this on Nov 19, 2021
@oandregal oandregal added this to 👀 Needs review in WordPress 5.9 Must-Haves via automation on Nov 19, 2021
@oandregal oandregal requested a review from ajlende as a code owner 3 years ago
@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 on Nov 19, 2021
@overclokk
Copy link

@overclokk overclokk commented on Nov 19, 2021

What happen with a child theme?

Loading

@oandregal
Copy link
Member Author

@oandregal oandregal commented on Nov 23, 2021

What happen with a child theme?

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

Loading

Copy link
Contributor

@youknowriad youknowriad left a comment

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

Loading

@oandregal oandregal merged commit 1e78e92 into trunk on Nov 23, 2021
20 checks passed
Loading
WordPress 5.9 Must-Haves automation moved this from 👀 Needs review to ✅ Done on Nov 23, 2021
@oandregal oandregal deleted the add/appearance-tools-flag branch 3 years ago
@github-actions github-actions bot added this to the Gutenberg 12.1 milestone on Nov 23, 2021
@oandregal
Copy link
Member Author

@oandregal oandregal commented on Nov 23, 2021

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?

Loading

@youknowriad
Copy link
Contributor

@youknowriad youknowriad commented on Nov 23, 2021

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.

Loading

@oandregal
Copy link
Member Author

@oandregal oandregal commented on Nov 23, 2021

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).

Loading

@overclokk
Copy link

@overclokk overclokk commented on Nov 24, 2021

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

Loading

@oandregal
Copy link
Member Author

@oandregal oandregal commented on Nov 24, 2021

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

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment