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

Update theme.json classes #1808

Closed
wants to merge 56 commits into from
Closed

Update theme.json classes #1808

wants to merge 56 commits into from

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented Nov 4, 2021

https://core.trac.wordpress.org/ticket/54336

This PR ports to core the changes to the classes that deal with theme.json code.

Status

Done:

  • Update WP_Theme_JSON and tests.
  • Update WP_Theme_JSON_Resolver and tests.
  • Add WP_Theme_JSON_Schema and tests.
  • Update theme-i18n.json schema.
  • Update kses for the new properties to work.
  • Update the uses of WP_Theme_JSON_Resolver class.
  • Fix tests.
  • Port render_duotone_filter_preset
  • Fix 5.6 test errors.

Follow-up PRs

How to test

  • Use TwentyTwentyOne.
  • In its functions.php file, comment the lines that add support for the following: custom-line-height, experimental-link-color, custom-spacing, custom-units.
  • Create a theme.json in the top-level directory of the theme and paste these contents:
{
	"version": 2,
	"settings": {
		"color": {
			"custom": false,
			"link": true
		},
		"spacing": {
			"padding": true
		},
		"typography": {
			"lineHeight": true,
			"fontSizes": []
		},
		"layout": {
			"contentSize": "840px",
			"wideSize": "1100px"
		}
	}
}

In the post editor

Verify that the settings are respected: users can't add custom colors, link color is present, the cover block has the spacing panel with the padding control, the typography panel only shows line-height but not font sizes.

Change the settings and verify that it still works as expected.

Substitute the content of the theme.json by these:

{
	"version": 1,
	"settings": {
		"color": {
			"custom": false,
			"link": true
		},
		"spacing": {
			"customPadding": true
		},
		"typography": {
			"customLineHeight": true,
			"fontSizes": []
		},
		"layout": {
			"contentSize": "840px",
			"wideSize": "1100px"
		}
	}
}

Verify that it still works as expected.

In the front

Verify that there's an embedded stylesheet whose name is global-styles-inline-css and that it contains:

  • the CSS Custom Properties for the presets defined by core and the theme
  • the layout/alignment styles
  • the classes for the presets defined by core and the theme
Expand content body { --wp--preset--color--black: #000000; --wp--preset--color--cyan-bluish-gray: #abb8c3; --wp--preset--color--white: #ffffff; --wp--preset--color--pale-pink: #f78da7; --wp--preset--color--vivid-red: #cf2e2e; --wp--preset--color--luminous-vivid-orange: #ff6900; --wp--preset--color--luminous-vivid-amber: #fcb900; --wp--preset--color--light-green-cyan: #7bdcb5; --wp--preset--color--vivid-green-cyan: #00d084; --wp--preset--color--pale-cyan-blue: #8ed1fc; --wp--preset--color--vivid-cyan-blue: #0693e3; --wp--preset--color--vivid-purple: #9b51e0; --wp--preset--color--dark-gray: #28303d; --wp--preset--color--gray: #39414d; --wp--preset--color--green: #d1e4dd; --wp--preset--color--blue: #d1dfe4; --wp--preset--color--purple: #d1d1e4; --wp--preset--color--red: #e4d1d1; --wp--preset--color--orange: #e4dad1; --wp--preset--color--yellow: #eeeadd; --wp--preset--gradient--vivid-cyan-blue-to-vivid-purple: linear-gradient( 135deg, rgba(6, 147, 227, 1) 0%, rgb(155, 81, 224) 100% ); --wp--preset--gradient--light-green-cyan-to-vivid-green-cyan: linear-gradient( 135deg, rgb(122, 220, 180) 0%, rgb(0, 208, 130) 100% ); --wp--preset--gradient--luminous-vivid-amber-to-luminous-vivid-orange: linear-gradient( 135deg, rgba(252, 185, 0, 1) 0%, rgba(255, 105, 0, 1) 100% ); --wp--preset--gradient--luminous-vivid-orange-to-vivid-red: linear-gradient( 135deg, rgba(255, 105, 0, 1) 0%, rgb(207, 46, 46) 100% ); --wp--preset--gradient--very-light-gray-to-cyan-bluish-gray: linear-gradient( 135deg, rgb(238, 238, 238) 0%, rgb(169, 184, 195) 100% ); --wp--preset--gradient--cool-to-warm-spectrum: linear-gradient( 135deg, rgb(74, 234, 220) 0%, rgb(151, 120, 209) 20%, rgb(207, 42, 186) 40%, rgb(238, 44, 130) 60%, rgb(251, 105, 98) 80%, rgb(254, 248, 76) 100% ); --wp--preset--gradient--blush-light-purple: linear-gradient( 135deg, rgb(255, 206, 236) 0%, rgb(152, 150, 240) 100% ); --wp--preset--gradient--blush-bordeaux: linear-gradient( 135deg, rgb(254, 205, 165) 0%, rgb(254, 45, 45) 50%, rgb(107, 0, 62) 100% ); --wp--preset--gradient--luminous-dusk: linear-gradient( 135deg, rgb(255, 203, 112) 0%, rgb(199, 81, 192) 50%, rgb(65, 88, 208) 100% ); --wp--preset--gradient--pale-ocean: linear-gradient( 135deg, rgb(255, 245, 203) 0%, rgb(182, 227, 212) 50%, rgb(51, 167, 181) 100% ); --wp--preset--gradient--electric-grass: linear-gradient( 135deg, rgb(202, 248, 128) 0%, rgb(113, 206, 126) 100% ); --wp--preset--gradient--midnight: linear-gradient( 135deg, rgb(2, 3, 129) 0%, rgb(40, 116, 252) 100% ); --wp--preset--gradient--purple-to-yellow: linear-gradient( 160deg, #d1d1e4 0%, #eeeadd 100% ); --wp--preset--gradient--yellow-to-purple: linear-gradient( 160deg, #eeeadd 0%, #d1d1e4 100% ); --wp--preset--gradient--green-to-yellow: linear-gradient( 160deg, #d1e4dd 0%, #eeeadd 100% ); --wp--preset--gradient--yellow-to-green: linear-gradient( 160deg, #eeeadd 0%, #d1e4dd 100% ); --wp--preset--gradient--red-to-yellow: linear-gradient( 160deg, #e4d1d1 0%, #eeeadd 100% ); --wp--preset--gradient--yellow-to-red: linear-gradient( 160deg, #eeeadd 0%, #e4d1d1 100% ); --wp--preset--gradient--purple-to-red: linear-gradient( 160deg, #d1d1e4 0%, #e4d1d1 100% ); --wp--preset--gradient--red-to-purple: linear-gradient( 160deg, #e4d1d1 0%, #d1d1e4 100% ); --wp--preset--font-size--small: 18px; --wp--preset--font-size--normal: 20px; --wp--preset--font-size--medium: 20px; --wp--preset--font-size--large: 24px; --wp--preset--font-size--huge: 96px; --wp--preset--font-size--extra-small: 16px; --wp--preset--font-size--extra-large: 40px; --wp--preset--font-size--gigantic: 144px; } body { margin: 0; } .wp-site-blocks > .alignleft { float: left; margin-right: 2em; } .wp-site-blocks > .alignright { float: right; margin-left: 2em; } .wp-site-blocks > .aligncenter { justify-content: center; margin-left: auto; margin-right: auto; } .has-black-color { color: var(--wp--preset--color--black) !important; } .has-cyan-bluish-gray-color { color: var(--wp--preset--color--cyan-bluish-gray) !important; } .has-white-color { color: var(--wp--preset--color--white) !important; } .has-pale-pink-color { color: var(--wp--preset--color--pale-pink) !important; } .has-vivid-red-color { color: var(--wp--preset--color--vivid-red) !important; } .has-luminous-vivid-orange-color { color: var(--wp--preset--color--luminous-vivid-orange) !important; } .has-luminous-vivid-amber-color { color: var(--wp--preset--color--luminous-vivid-amber) !important; } .has-light-green-cyan-color { color: var(--wp--preset--color--light-green-cyan) !important; } .has-vivid-green-cyan-color { color: var(--wp--preset--color--vivid-green-cyan) !important; } .has-pale-cyan-blue-color { color: var(--wp--preset--color--pale-cyan-blue) !important; } .has-vivid-cyan-blue-color { color: var(--wp--preset--color--vivid-cyan-blue) !important; } .has-vivid-purple-color { color: var(--wp--preset--color--vivid-purple) !important; } .has-dark-gray-color { color: var(--wp--preset--color--dark-gray) !important; } .has-gray-color { color: var(--wp--preset--color--gray) !important; } .has-green-color { color: var(--wp--preset--color--green) !important; } .has-blue-color { color: var(--wp--preset--color--blue) !important; } .has-purple-color { color: var(--wp--preset--color--purple) !important; } .has-red-color { color: var(--wp--preset--color--red) !important; } .has-orange-color { color: var(--wp--preset--color--orange) !important; } .has-yellow-color { color: var(--wp--preset--color--yellow) !important; } .has-black-background-color { background-color: var(--wp--preset--color--black) !important; } .has-cyan-bluish-gray-background-color { background-color: var(--wp--preset--color--cyan-bluish-gray) !important; } .has-white-background-color { background-color: var(--wp--preset--color--white) !important; } .has-pale-pink-background-color { background-color: var(--wp--preset--color--pale-pink) !important; } .has-vivid-red-background-color { background-color: var(--wp--preset--color--vivid-red) !important; } .has-luminous-vivid-orange-background-color { background-color: var(--wp--preset--color--luminous-vivid-orange) !important; } .has-luminous-vivid-amber-background-color { background-color: var(--wp--preset--color--luminous-vivid-amber) !important; } .has-light-green-cyan-background-color { background-color: var(--wp--preset--color--light-green-cyan) !important; } .has-vivid-green-cyan-background-color { background-color: var(--wp--preset--color--vivid-green-cyan) !important; } .has-pale-cyan-blue-background-color { background-color: var(--wp--preset--color--pale-cyan-blue) !important; } .has-vivid-cyan-blue-background-color { background-color: var(--wp--preset--color--vivid-cyan-blue) !important; } .has-vivid-purple-background-color { background-color: var(--wp--preset--color--vivid-purple) !important; } .has-dark-gray-background-color { background-color: var(--wp--preset--color--dark-gray) !important; } .has-gray-background-color { background-color: var(--wp--preset--color--gray) !important; } .has-green-background-color { background-color: var(--wp--preset--color--green) !important; } .has-blue-background-color { background-color: var(--wp--preset--color--blue) !important; } .has-purple-background-color { background-color: var(--wp--preset--color--purple) !important; } .has-red-background-color { background-color: var(--wp--preset--color--red) !important; } .has-orange-background-color { background-color: var(--wp--preset--color--orange) !important; } .has-yellow-background-color { background-color: var(--wp--preset--color--yellow) !important; } .has-black-border-color { border-color: var(--wp--preset--color--black) !important; } .has-cyan-bluish-gray-border-color { border-color: var(--wp--preset--color--cyan-bluish-gray) !important; } .has-white-border-color { border-color: var(--wp--preset--color--white) !important; } .has-pale-pink-border-color { border-color: var(--wp--preset--color--pale-pink) !important; } .has-vivid-red-border-color { border-color: var(--wp--preset--color--vivid-red) !important; } .has-luminous-vivid-orange-border-color { border-color: var(--wp--preset--color--luminous-vivid-orange) !important; } .has-luminous-vivid-amber-border-color { border-color: var(--wp--preset--color--luminous-vivid-amber) !important; } .has-light-green-cyan-border-color { border-color: var(--wp--preset--color--light-green-cyan) !important; } .has-vivid-green-cyan-border-color { border-color: var(--wp--preset--color--vivid-green-cyan) !important; } .has-pale-cyan-blue-border-color { border-color: var(--wp--preset--color--pale-cyan-blue) !important; } .has-vivid-cyan-blue-border-color { border-color: var(--wp--preset--color--vivid-cyan-blue) !important; } .has-vivid-purple-border-color { border-color: var(--wp--preset--color--vivid-purple) !important; } .has-dark-gray-border-color { border-color: var(--wp--preset--color--dark-gray) !important; } .has-gray-border-color { border-color: var(--wp--preset--color--gray) !important; } .has-green-border-color { border-color: var(--wp--preset--color--green) !important; } .has-blue-border-color { border-color: var(--wp--preset--color--blue) !important; } .has-purple-border-color { border-color: var(--wp--preset--color--purple) !important; } .has-red-border-color { border-color: var(--wp--preset--color--red) !important; } .has-orange-border-color { border-color: var(--wp--preset--color--orange) !important; } .has-yellow-border-color { border-color: var(--wp--preset--color--yellow) !important; } .has-vivid-cyan-blue-to-vivid-purple-gradient-background { background: var( --wp--preset--gradient--vivid-cyan-blue-to-vivid-purple ) !important; } .has-light-green-cyan-to-vivid-green-cyan-gradient-background { background: var( --wp--preset--gradient--light-green-cyan-to-vivid-green-cyan ) !important; } .has-luminous-vivid-amber-to-luminous-vivid-orange-gradient-background { background: var( --wp--preset--gradient--luminous-vivid-amber-to-luminous-vivid-orange ) !important; } .has-luminous-vivid-orange-to-vivid-red-gradient-background { background: var( --wp--preset--gradient--luminous-vivid-orange-to-vivid-red ) !important; } .has-very-light-gray-to-cyan-bluish-gray-gradient-background { background: var( --wp--preset--gradient--very-light-gray-to-cyan-bluish-gray ) !important; } .has-cool-to-warm-spectrum-gradient-background { background: var(--wp--preset--gradient--cool-to-warm-spectrum) !important; } .has-blush-light-purple-gradient-background { background: var(--wp--preset--gradient--blush-light-purple) !important; } .has-blush-bordeaux-gradient-background { background: var(--wp--preset--gradient--blush-bordeaux) !important; } .has-luminous-dusk-gradient-background { background: var(--wp--preset--gradient--luminous-dusk) !important; } .has-pale-ocean-gradient-background { background: var(--wp--preset--gradient--pale-ocean) !important; } .has-electric-grass-gradient-background { background: var(--wp--preset--gradient--electric-grass) !important; } .has-midnight-gradient-background { background: var(--wp--preset--gradient--midnight) !important; } .has-purple-to-yellow-gradient-background { background: var(--wp--preset--gradient--purple-to-yellow) !important; } .has-yellow-to-purple-gradient-background { background: var(--wp--preset--gradient--yellow-to-purple) !important; } .has-green-to-yellow-gradient-background { background: var(--wp--preset--gradient--green-to-yellow) !important; } .has-yellow-to-green-gradient-background { background: var(--wp--preset--gradient--yellow-to-green) !important; } .has-red-to-yellow-gradient-background { background: var(--wp--preset--gradient--red-to-yellow) !important; } .has-yellow-to-red-gradient-background { background: var(--wp--preset--gradient--yellow-to-red) !important; } .has-purple-to-red-gradient-background { background: var(--wp--preset--gradient--purple-to-red) !important; } .has-red-to-purple-gradient-background { background: var(--wp--preset--gradient--red-to-purple) !important; } .has-small-font-size { font-size: var(--wp--preset--font-size--small) !important; } .has-normal-font-size { font-size: var(--wp--preset--font-size--normal) !important; } .has-medium-font-size { font-size: var(--wp--preset--font-size--medium) !important; } .has-large-font-size { font-size: var(--wp--preset--font-size--large) !important; } .has-huge-font-size { font-size: var(--wp--preset--font-size--huge) !important; } .has-extra-small-font-size { font-size: var(--wp--preset--font-size--extra-small) !important; } .has-extra-large-font-size { font-size: var(--wp--preset--font-size--extra-large) !important; } .has-gigantic-font-size { font-size: var(--wp--preset--font-size--gigantic) !important; }

Add the following styles to the theme.json:

{
        "version": 2,
        "settings": { ... },
	"styles": {
		"color": {
			"background": "black",
			"text": "white"
		}
	}
}

and verify that the block styles are added, in addition to the above:

Expand ```css body { background-color: black; color: white; } ```

@oandregal
Copy link
Member Author

JFYI @jorgefilipecosta

@@ -2282,6 +2286,7 @@ function safecss_filter_attr( $css, $deprecated = '' ) {
'column-width',

'color',
'filter',
Copy link
Member

Choose a reason for hiding this comment

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

@noisysocks is already including most of these changes in #1804. Excluding filter, because filter was added in a different way than the others (in lib/global-styles.php) instead of lib/compat.php. Should we just update the plugin to add this in lib/compat.php and update Robert PR to contain all kses changes and remove kses from here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, we need the kses changes in this PR so the code works. Perhaps if this lands quicker than #1804, those changes can be removed from there.

Copy link
Member

Choose a reason for hiding this comment

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

#1804 will land last. If there is overlap then it will come up when I rebase my PR, so no worries 🙂

@oandregal
Copy link
Member Author

I've updated the base classes and all that's left now is to update their usage and fix some tests.

src/wp-includes/class-wp-theme-json-schema.php Outdated Show resolved Hide resolved
src/wp-includes/class-wp-theme-json.php Outdated Show resolved Hide resolved
src/wp-includes/class-wp-theme-json.php Outdated Show resolved Hide resolved
tests/phpunit/tests/theme/wpThemeJsonSchema.php Outdated Show resolved Hide resolved
@oandregal
Copy link
Member Author

I've done a second pass at this. What's left:

  • Port the render_duotone_filter_preset function, which is part of duotone block supports. This is used as a callback by WP_Theme_JSON. This can be a different PR to land before this one or just merge here.
  • I've seen some lint errors reported, will see how to fix them.

@oandregal oandregal marked this pull request as ready for review November 5, 2021 13:45
public static function get_merged_data( $settings = array() ) {
$theme_support_data = WP_Theme_JSON::get_from_editor_settings( $settings );

public static function get_merged_data( $origin = 'user' ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Changing the method signature could be a breaking change for backwards-compatibility. However, there is a proper value and data type check ===. Should be okay.

Copy link
Member Author

Choose a reason for hiding this comment

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

Does it make any difference that this class is marked as @private? Hence, it's expected that it's not used by anyone.

Copy link
Member

Choose a reason for hiding this comment

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

No not really. private is just a suggestion in WordPress, the whole code is public.

You do either this

public static function get_merged_data( $deprecated = array(), $origin = 'user' ) {
              if ( ! empty( $deprecated ) ) {
		 _deprecated_argument( __FUNCTION__, '5.9' );
	      }

or


public static function get_merged_data( $origin = 'user' ) {
              if ( is_array( $origin ) ) {
		 doing_it_wrong( 'Do not pass array here', '5.9' );
	      }

Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make any difference that this class is marked as @Private? Hence, it's expected that it's not used by anyone.

Yes, it should make a difference.

Here's my understanding of @access private:

Marking a global function or class as @access private means: it's only used in and by Core. It's internal functionality. There's no binding contract for or guarantee of backwards-compatibility externally.

Copy link
Contributor

@hellofromtonya hellofromtonya Nov 8, 2021

Choose a reason for hiding this comment

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

From the Core handbook:

@access: Only used for core-only functions or classes implementing “private” core APIs. If the element is private it will be output with a message stating its intention for internal use.

@oandregal
Copy link
Member Author

The remaining failing test is a bit obscure. It looks like it's an environment that can't be created. I can't repro locally.

@spacedmonkey
Copy link
Member

Unit tests seem to be failing.

* @param mixed $n Number of unknown type.
* @return float Value in the range [0,1].
*/
function wp_tinycolor_bound_alpha( $n ) {
Copy link
Member

Choose a reason for hiding this comment

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

This function should be marked as private prefixed with "_". This is internal only and we should look into renaming these color functions ASAP before any beta version of 5.9 comes out. The functions should be named in what they do and not where the inspiration comes from. Now we don't even use tinycolor on the client, we should not use a port of it on the server. We should look into what color functions we need on the server and name them accordingly.
This is an issue that comes from WP 5.8 so it is not a blocker provided we mark it as private with a to-do comment to remove or rename.

Copy link
Member Author

Choose a reason for hiding this comment

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

* The JS color picker considers the string "transparent" to be a hex value,
* so we need to handle it here as a special case.
*/
if ( 'transparent' === $color_str ) {
Copy link
Member

Choose a reason for hiding this comment

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

Not something related to this PR something we should look into the plugin. We accept many named colors not just transparent, one can select "red", "pink" etc. If we have a case to deal with a named color we should probably deal with the others.

@jorgefilipecosta
Copy link
Member

Thank you all for the feedback it was a huge PR that took some iterations but it is finally committed 60725c4.

That said don't hesitate in providing additional feedback if there are further improvements and we will iterate on the changes and propose another patch.

@oandregal
Copy link
Member Author

@oandregal oandregal closed this Nov 8, 2021
@oandregal oandregal deleted the update/theme-json-classes branch November 8, 2021 19:26
@ockham ockham mentioned this pull request Nov 8, 2021
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants