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

Centralize style & support mappings for blocks #25185

Merged
merged 13 commits into from
Sep 10, 2020
Merged

Centralize style & support mappings for blocks #25185

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
d51a5de
Create constants in blocks API
oandregal Sep 9, 2020
e78fd5a
Migrate block-editor to use constants from blocks API
oandregal Sep 9, 2020
b4b50c8
Migrate edit-site to use constants from blocks API
oandregal Sep 9, 2020
7c9560a
Make linter happy
oandregal Sep 9, 2020
cc79a34
Move support keys back to block-editor
oandregal Sep 9, 2020
6821e59
Reformat back to be equal to master
oandregal Sep 9, 2020
4b882e4
Get back master format
oandregal Sep 9, 2020
68f8e6c
Get back format as in master
oandregal Sep 9, 2020
9d1b159
Add experimental prefix
oandregal Sep 10, 2020
391336a
Do not export link color
oandregal Sep 10, 2020
6b635ad
Getter and setter only take the property name
oandregal Sep 10, 2020
7a44eaa
Rename getter/setter for better semantics
oandregal Sep 10, 2020
b6229f7
Typography panel: use property name in getter/setter
oandregal Sep 10, 2020
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
54 changes: 35 additions & 19 deletions lib/global-styles.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,26 +262,50 @@ function gutenberg_experimental_global_styles_get_theme() {
}

/**
* Returns the style features a particular block supports.
* Return how the style property is structured.
*
* @param array $supports The block supports array.
* @return array Style property structure.
*/
function gutenberg_experimental_global_styles_get_style_property() {
return array(
'line-height' => array( 'typography', 'lineHeight' ),
'font-size' => array( 'typography', 'fontSize' ),
'background' => array( 'color', 'gradient' ),
'background-color' => array( 'color', 'background' ),
'color' => array( 'color', 'text' ),
'--wp--style--color--link' => array( 'color', 'link' ),
);
}

/**
* Return how the support keys are structured.
*
* @return array Style features supported by the block.
* @return array Support keys structure.
*/
function gutenberg_experimental_global_styles_get_supported_styles( $supports ) {
$style_features = array(
function gutenberg_experimental_global_styles_get_support_keys() {
return array(
'--wp--style--color--link' => array( '__experimentalColor', 'linkColor' ),
'background-color' => array( '__experimentalColor' ),
Copy link
Member Author

Choose a reason for hiding this comment

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

These keys were renamed to use the same conventions as client-side (camelCase over kebab-case), making everything more coherent and less computation-demanding.

'backgroundColor' => array( '__experimentalColor' ),
'background' => array( '__experimentalColor', 'gradients' ),
'color' => array( '__experimentalColor' ),
'font-size' => array( '__experimentalFontSize' ),
'line-height' => array( '__experimentalLineHeight' ),
'fontSize' => array( '__experimentalFontSize' ),
'lineHeight' => array( '__experimentalLineHeight' ),
);
}

/**
* Returns the style features a particular block supports.
*
* @param array $supports The block supports array.
*
* @return array Style features supported by the block.
*/
function gutenberg_experimental_global_styles_get_supported_styles( $supports ) {
$support_keys = gutenberg_experimental_global_styles_get_support_keys();
$supported_features = array();
foreach ( $style_features as $style_feature => $path ) {
foreach ( $support_keys as $key => $path ) {
if ( gutenberg_experimental_get( $supports, $path ) ) {
$supported_features[] = $style_feature;
$supported_features[] = $key;
}
}

Expand Down Expand Up @@ -385,17 +409,9 @@ function gutenberg_experimental_global_styles_get_block_data() {
* @return array Containing a set of css rules.
*/
function gutenberg_experimental_global_styles_flatten_styles_tree( $styles ) {
$mappings = array(
Copy link
Member Author

Choose a reason for hiding this comment

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

No changes here, only extracted to a separate function.

'line-height' => array( 'typography', 'lineHeight' ),
'font-size' => array( 'typography', 'fontSize' ),
'background' => array( 'color', 'gradient' ),
'background-color' => array( 'color', 'background' ),
'color' => array( 'color', 'text' ),
'--wp--style--color--link' => array( 'color', 'link' ),
);
$mappings = gutenberg_experimental_global_styles_get_style_property();

$result = array();

foreach ( $mappings as $key => $path ) {
$value = gutenberg_experimental_get( $styles, $path, null );
if ( null !== $value ) {
Expand Down
5 changes: 2 additions & 3 deletions packages/block-editor/src/hooks/style.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { has, get, startsWith } from 'lodash';
* WordPress dependencies
*/
import { addFilter } from '@wordpress/hooks';
import { hasBlockSupport } from '@wordpress/blocks';
import { hasBlockSupport, STYLE_PROPERTY } from '@wordpress/blocks';
import { createHigherOrderComponent } from '@wordpress/compose';

/**
Expand All @@ -17,7 +17,6 @@ import { COLOR_SUPPORT_KEY, ColorEdit } from './color';
import { TypographyPanel, TYPOGRAPHY_SUPPORT_KEYS } from './typography';
import { PADDING_SUPPORT_KEY, PaddingEdit } from './padding';
import SpacingPanelControl from '../components/spacing-panel-control';
import { STYLE_MAPPINGS } from './utils';

const styleSupportKeys = [
...TYPOGRAPHY_SUPPORT_KEYS,
Expand Down Expand Up @@ -50,7 +49,7 @@ function compileStyleValue( uncompiledValue ) {
*/
export function getInlineStyles( styles = {} ) {
const output = {};
Object.entries( STYLE_MAPPINGS ).forEach(
Object.entries( STYLE_PROPERTY ).forEach(
( [ styleKey, ...otherObjectKeys ] ) => {
const [ objectKeys ] = otherObjectKeys;

Expand Down
13 changes: 0 additions & 13 deletions packages/block-editor/src/hooks/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,3 @@ export const cleanEmptyObject = ( object ) => {
? undefined
: cleanedNestedObjects;
};

export const STYLE_MAPPINGS = {
lineHeight: [ 'typography', 'lineHeight' ],
fontSize: [ 'typography', 'fontSize' ],
background: [ 'color', 'gradient' ],
backgroundColor: [ 'color', 'background' ],
color: [ 'color', 'text' ],
'--wp--style--color--link': [ 'color', 'link' ],
paddingTop: [ 'spacing', 'padding', 'top' ],
paddingRight: [ 'spacing', 'padding', 'right' ],
paddingBottom: [ 'spacing', 'padding', 'bottom' ],
paddingLeft: [ 'spacing', 'padding', 'left' ],
};
1 change: 0 additions & 1 deletion packages/block-editor/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,3 @@ export * from './components';
export * from './utils';
export { storeConfig } from './store';
export { SETTINGS_DEFAULTS } from './store/defaults';
export { STYLE_MAPPINGS as __EXPERIMENTAL_STYLE_MAPPINGS } from './hooks/utils';
8 changes: 8 additions & 0 deletions packages/blocks/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,10 @@ _Returns_

- `boolean`: True if the parameter is a valid icon and false otherwise.

<a name="LINK_COLOR" href="#LINK_COLOR">#</a> **LINK_COLOR**

Undocumented declaration.

<a name="normalizeIconObject" href="#normalizeIconObject">#</a> **normalizeIconObject**

Function that receives an icon as set by the blocks during the registration
Expand Down Expand Up @@ -724,6 +728,10 @@ _Parameters_

- _blockName_ `string`: Block name.

<a name="STYLE_PROPERTY" href="#STYLE_PROPERTY">#</a> **STYLE_PROPERTY**

Undocumented declaration.

<a name="switchToBlockType" href="#switchToBlockType">#</a> **switchToBlockType**

Switch one or more blocks into one or more blocks of the new block type.
Expand Down
15 changes: 15 additions & 0 deletions packages/blocks/src/api/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,18 @@ export const DEPRECATED_ENTRY_KEYS = [
'migrate',
'isEligible',
];

export const LINK_COLOR = '--wp--style--color--link';

export const STYLE_PROPERTY = {
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is a good place for this kind of info, as it speaks about how the block attributes are structured. However, I don't feel super strongly about this. If this is a blocker I'm fine moving it to any other place.

Copy link
Member

Choose a reason for hiding this comment

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

I guess we should prefix the constant with experimental.

[ LINK_COLOR ]: [ 'color', 'link' ],
background: [ 'color', 'gradient' ],
backgroundColor: [ 'color', 'background' ],
color: [ 'color', 'text' ],
fontSize: [ 'typography', 'fontSize' ],
lineHeight: [ 'typography', 'lineHeight' ],
paddingBottom: [ 'spacing', 'padding', 'bottom' ],
paddingLeft: [ 'spacing', 'padding', 'left' ],
paddingRight: [ 'spacing', 'padding', 'right' ],
paddingTop: [ 'spacing', 'padding', 'top' ],
};
1 change: 1 addition & 0 deletions packages/blocks/src/api/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,4 @@ export {
} from './templates';
export { default as children } from './children';
export { default as node } from './node';
export { LINK_COLOR, STYLE_PROPERTY } from './constants';
23 changes: 14 additions & 9 deletions packages/edit-site/src/components/editor/global-styles-renderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
/**
* External dependencies
*/
import { get } from 'lodash';
import { get, kebabCase } from 'lodash';

/**
* WordPress dependencies
*/
import { STYLE_PROPERTY } from '@wordpress/blocks';

/**
* Internal dependencies
*/
import {
STYLE_PROPS,
PRESET_CATEGORIES,
LINK_COLOR_DECLARATION,
} from './utils';
import { PRESET_CATEGORIES, LINK_COLOR_DECLARATION } from './utils';

const mergeTrees = ( baseData, userData ) => {
// Deep clone from base data.
Expand Down Expand Up @@ -56,13 +57,17 @@ export default ( blockData, baseTree, userTree ) => {
*/
const getBlockStylesDeclarations = ( blockSupports, blockStyles ) => {
const declarations = [];
Object.keys( STYLE_PROPS ).forEach( ( key ) => {
Object.keys( STYLE_PROPERTY ).forEach( ( key ) => {
const cssProperty = key.startsWith( '--' ) ? key : kebabCase( key );
if (
blockSupports.includes( key ) &&
get( blockStyles, STYLE_PROPS[ key ], false )
get( blockStyles, STYLE_PROPERTY[ key ], false )
) {
declarations.push(
`${ key }: ${ get( blockStyles, STYLE_PROPS[ key ] ) }`
`${ cssProperty }: ${ get(
blockStyles,
STYLE_PROPERTY[ key ]
) }`
);
}
} );
Expand Down
19 changes: 4 additions & 15 deletions packages/edit-site/src/components/editor/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,11 @@
/* CSS properties */
export const FONT_SIZE = 'font-size';
export const LINK_COLOR = '--wp--style--color--link';
export const BACKGROUND_COLOR = 'background-color';
export const GRADIENT_COLOR = 'background';
export const TEXT_COLOR = 'color';
export const LINE_HEIGHT = 'line-height';
/**
* WordPress dependencies
*/
import { LINK_COLOR } from '@wordpress/blocks';

/* Supporting data */
export const GLOBAL_CONTEXT = 'global';
export const PRESET_CATEGORIES = [ 'color', 'font-size', 'gradient' ];
export const STYLE_PROPS = {
[ FONT_SIZE ]: 'typography.fontSize',
[ LINE_HEIGHT ]: 'typography.lineHeight',
[ TEXT_COLOR ]: 'color.text',
[ BACKGROUND_COLOR ]: 'color.background',
[ GRADIENT_COLOR ]: 'color.gradient',
[ LINK_COLOR ]: 'color.link',
};
export const LINK_COLOR_DECLARATION = `a { color: var(${ LINK_COLOR }, #00e); }`;

/* Helpers for unit processing */
Expand Down
43 changes: 17 additions & 26 deletions packages/edit-site/src/components/sidebar/color-panel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,63 +3,54 @@
*/
import { __experimentalPanelColorGradientSettings as PanelColorGradientSettings } from '@wordpress/block-editor';
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import {
BACKGROUND_COLOR,
GRADIENT_COLOR,
LINK_COLOR,
TEXT_COLOR,
} from '../editor/utils';
import { STYLE_PROPERTY, LINK_COLOR } from '@wordpress/blocks';

export default ( {
context: { supports, name },
getProperty,
setProperty,
} ) => {
if (
! supports.includes( TEXT_COLOR ) &&
! supports.includes( BACKGROUND_COLOR ) &&
! supports.includes( GRADIENT_COLOR ) &&
! supports.includes( 'color' ) &&
Copy link
Member Author

Choose a reason for hiding this comment

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

These keys are the ones that were updated in the server (gutenberg_experimental_global_styles_get_support_keys changes). This way we use the common convention for the styles properties everywhere.

! supports.includes( 'backgrounColor' ) &&
! supports.includes( 'background' ) &&
! supports.includes( LINK_COLOR )
) {
return null;
}

const settings = [];

if ( supports.includes( TEXT_COLOR ) ) {
if ( supports.includes( 'color' ) ) {
settings.push( {
colorValue: getProperty( name, [ 'color', 'text' ] ),
colorValue: getProperty( name, STYLE_PROPERTY.color ),
Copy link
Member Author

Choose a reason for hiding this comment

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

Personally, this is the major advantage of centralization, and what pushed me to create this PR. Controls don't need to know what's the path of the property they want to access.

Copy link
Member

Choose a reason for hiding this comment

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

If the idea is to let control not need to know the shape of the property they want to access, I guess getProperty could receive the property instead of the path, and be responsible for resolving the property.

Of course, we may need to change something that does not contain a property e.g: a preset. So I guess in the future we may have a get and setter that receives the property directly and one that receives the paths.

But for now, what we have is ok.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes! You're so right. At 6b635ad and 7a44eaa I've renamed the getter & setter to getStyleProperty and setStyleProperty and they take the property name now. This clean-up makes everything much better :)

onColorChange: ( value ) =>
setProperty( name, [ 'color', 'text' ], value ),
setProperty( name, STYLE_PROPERTY.color, value ),
label: __( 'Text color' ),
} );
}

let backgroundSettings = {};
if ( supports.includes( BACKGROUND_COLOR ) ) {
if ( supports.includes( 'backgroundColor' ) ) {
backgroundSettings = {
colorValue: getProperty( name, [ 'color', 'background' ] ),
colorValue: getProperty( name, STYLE_PROPERTY.backgroundColor ),
onColorChange: ( value ) =>
setProperty( name, [ 'color', 'background' ], value ),
setProperty( name, STYLE_PROPERTY.backgroundColor, value ),
};
}

let gradientSettings = {};
if ( supports.includes( GRADIENT_COLOR ) ) {
if ( supports.includes( 'background' ) ) {
gradientSettings = {
gradientValue: getProperty( name, [ 'color', 'gradient' ] ),
gradientValue: getProperty( name, STYLE_PROPERTY.background ),
onGradientChange: ( value ) =>
setProperty( name, [ 'color', 'gradient' ], value ),
setProperty( name, STYLE_PROPERTY.background, value ),
};
}

if (
supports.includes( GRADIENT_COLOR ) ||
supports.includes( BACKGROUND_COLOR )
supports.includes( 'background' ) ||
supports.includes( 'backgroundColor' )
) {
settings.push( {
...backgroundSettings,
Expand All @@ -70,9 +61,9 @@ export default ( {

if ( supports.includes( LINK_COLOR ) ) {
settings.push( {
colorValue: getProperty( name, [ 'color', 'link' ] ),
colorValue: getProperty( name, STYLE_PROPERTY[ LINK_COLOR ] ),
onColorChange: ( value ) =>
setProperty( name, [ 'color', 'link' ], value ),
setProperty( name, STYLE_PROPERTY[ LINK_COLOR ], value ),
label: __( 'Link color' ),
} );
}
Expand Down
26 changes: 10 additions & 16 deletions packages/edit-site/src/components/sidebar/typography-panel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,52 +7,46 @@ import {
} from '@wordpress/block-editor';
import { PanelBody } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { STYLE_PROPERTY } from '@wordpress/blocks';

/**
* Internal dependencies
*/
import { FONT_SIZE, LINE_HEIGHT, fromPx, toPx } from '../editor/utils';
import { fromPx, toPx } from '../editor/utils';

export default ( {
context: { supports, name },
getProperty,
setProperty,
} ) => {
if (
! supports.includes( FONT_SIZE ) &&
! supports.includes( LINE_HEIGHT )
! supports.includes( 'fontSize' ) &&
! supports.includes( 'lineHeight' )
) {
return null;
}

return (
<PanelBody title={ __( 'Typography' ) } initialOpen={ true }>
{ supports.includes( FONT_SIZE ) && (
{ supports.includes( 'fontSize' ) && (
<FontSizePicker
value={ fromPx(
getProperty( name, [ 'typography', 'fontSize' ] )
getProperty( name, STYLE_PROPERTY.fontSize )
) }
onChange={ ( value ) =>
setProperty(
name,
[ 'typography', 'fontSize' ],
STYLE_PROPERTY.fontSize,
toPx( value )
)
}
/>
) }
{ supports.includes( LINE_HEIGHT ) && (
{ supports.includes( 'lineHeight' ) && (
<LineHeightControl
value={ getProperty( name, [
'typography',
'lineHeight',
] ) }
value={ getProperty( name, STYLE_PROPERTY.lineHeight ) }
onChange={ ( value ) =>
setProperty(
name,
[ 'typography', 'lineHeight' ],
value
)
setProperty( name, STYLE_PROPERTY.lineHeight, value )
}
/>
) }
Expand Down