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.

Sign up for GitHub

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

added outline support for blocks via theme.json #43526

Merged
merged 2 commits into from
Sep 7, 2022
Merged

added outline support for blocks via theme.json #43526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ea5aca3
added outline to theme.json
MaggieCabrera Aug 23, 2022
6d94d1c
linting
MaggieCabrera Aug 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions docs/reference-guides/theme-json-reference/theme-json-living.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,19 @@ Color styles.

---

### outline

Outline styles.

| Property | Type | Props |
| --- | --- |--- |
| color | string | |
| offset | string | |
| style | string | |
| width | string | |

---

### spacing

Spacing styles.
Expand Down
10 changes: 10 additions & 0 deletions lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ class WP_Theme_JSON_6_1 extends WP_Theme_JSON_6_0 {
'margin-right' => array( 'spacing', 'margin', 'right' ),
'margin-bottom' => array( 'spacing', 'margin', 'bottom' ),
'margin-left' => array( 'spacing', 'margin', 'left' ),
'outline-color' => array( 'outline', 'color' ),
'outline-offset' => array( 'outline', 'offset' ),
'outline-style' => array( 'outline', 'style' ),
'outline-width' => array( 'outline', 'width' ),
'padding' => array( 'spacing', 'padding' ),
'padding-top' => array( 'spacing', 'padding', 'top' ),
'padding-right' => array( 'spacing', 'padding', 'right' ),
Expand Down Expand Up @@ -322,6 +326,12 @@ public static function remove_insecure_properties( $theme_json ) {
'filter' => array(
'duotone' => null,
),
'outline' => array(
'color' => null,
'offset' => null,
'style' => null,
'width' => null,
),
'spacing' => array(
'margin' => null,
'padding' => null,
Expand Down
2 changes: 2 additions & 0 deletions packages/style-engine/src/styles/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
import border from './border';
import color from './color';
import shadow from './shadow';
import outline from './outline';
import spacing from './spacing';
import typography from './typography';

export const styleDefinitions = [
...border,
...color,
...outline,
Copy link
Member

@ramonjd ramonjd Sep 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

These changes need an Unreleased change log entry, e.g.,

## New features 

- Adding outline support in the Style Engine JS package [#43526](https://github.com/WordPress/gutenberg/pull/43526)

...spacing,
...typography,
...shadow,
Expand Down
55 changes: 55 additions & 0 deletions packages/style-engine/src/styles/outline/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/**
* Internal dependencies
*/
import type { GeneratedCSSRule, Style, StyleOptions } from '../../types';
import { generateRule } from '../utils';

const color = {
name: 'color',
generate: (
style: Style,
options: StyleOptions,
path: string[] = [ 'outline', 'color' ],
ruleKey: string = 'outlineColor'
Comment on lines +12 to +13
Copy link
Contributor

@ajlende ajlende Aug 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I think these two arguments are for box model generation (top, right, bottom, left) which outline doesn't have. They should probably be removed here and passed directly to the generateRule function.

@ramonjd am I correct in thinking that? Or does this not matter?

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the ping!

I think these two arguments are for box model generation (top, right, bottom, left) which outline doesn't have.

While they share similar arguments, box model generation should use generateBoxRules rather than generateRule.

Example is margin. Note the margin%s pattern for individual sides:

const margin = {
	name: 'margin',
	generate: ( style: Style, options: StyleOptions ) => {
		return generateBoxRules( style, options, [ 'spacing', 'margin' ], {
			default: 'margin',
			individual: 'margin%s',
		} );
	},
};

So I think this looks okay.

They should probably be removed here and passed directly to the generateRule function.

I don't think it matters in practice, but in theory it might be better to be explicit about the arguments passed to generateRule, rather than set them as defaults.

There is little risk of it I'd say, but it would prevent unexpected path and ruleKey values being pass to the generate functions.

See how typography is set up, e.g., :

const fontSize = {
	name: 'fontSize',
	generate: ( style: Style, options: StyleOptions ) => {
		return generateRule(
			style,
			options,
			[ 'typography', 'fontSize' ],
			'fontSize'
		);
	},
};

Copy link
Contributor

@ajlende ajlende Aug 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@ramonjd Thanks for sharing!

I was able to do some refactoring to remove the dependence on those two arguments to enforce consistency between all of the styles. Details are in #43594.

If we make the changes to be more like typography there wouldn't be any conflicts with it in this PR.

): GeneratedCSSRule[] => {
return generateRule( style, options, path, ruleKey );
},
};

const offset = {
name: 'offset',
generate: (
style: Style,
options: StyleOptions,
path: string[] = [ 'outline', 'offset' ],
ruleKey: string = 'outlineColor'
): GeneratedCSSRule[] => {
return generateRule( style, options, path, ruleKey );
},
};

const outlineStyle = {
name: 'style',
generate: (
style: Style,
options: StyleOptions,
path: string[] = [ 'outline', 'style' ],
ruleKey: string = 'outlineStyle'
): GeneratedCSSRule[] => {
return generateRule( style, options, path, ruleKey );
},
};

const width = {
name: 'width',
generate: (
style: Style,
options: StyleOptions,
path: string[] = [ 'outline', 'width' ],
ruleKey: string = 'outlineWidth'
): GeneratedCSSRule[] => {
return generateRule( style, options, path, ruleKey );
},
};

export default [ color, outlineStyle, offset, width ];
23 changes: 23 additions & 0 deletions schemas/json/theme.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -981,6 +981,29 @@
},
"additionalProperties": false
},
"outline": {
"description": "Outline styles.",
"type": "object",
"properties": {
"color": {
"description": "Sets the `outline-color` CSS property.",
"type": "string"
},
"offset": {
"description": "Sets the `outline-offset` CSS property.",
"type": "string"
},
"style": {
"description": "Sets the `outline-style` CSS property.",
"type": "string"
},
"width": {
"description": "Sets the `outline-width` CSS property.",
"type": "string"
}
},
"additionalProperties": false
},
"spacing": {
"description": "Spacing styles.",
"type": "object",
Expand Down