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

Add: API to allow blocks to access global styles. #34178

Closed
2 changes: 1 addition & 1 deletion lib/class-wp-rest-block-editor-settings-controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public function get_item_schema() {
'__experimentalStyles' => array(
'description' => __( 'Styles consolidated from core, theme, and user origins.', 'gutenberg' ),
'type' => 'object',
'context' => array( 'mobile' ),
'context' => array( 'post-editor', 'widgets-editor', 'mobile' ),
),

'alignWide' => array(
Expand Down
2 changes: 1 addition & 1 deletion lib/global-styles.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ function_exists( 'gutenberg_is_edit_site_page' ) &&
}
$consolidated = WP_Theme_JSON_Resolver_Gutenberg::get_merged_data( $settings, $origin );

if ( 'mobile' === $context ) {
if ( 'site-editor' !== $context ) {
$settings['__experimentalStyles'] = $consolidated->get_raw_data()['styles'];
}

Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,3 +147,4 @@ export { default as __experimentalUseNoRecursiveRenders } from './use-no-recursi

export { default as BlockEditorProvider } from './provider';
export { default as useSetting } from './use-setting';
export { default as __experimentalUseStyle } from './use-style';
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ import {
isLineHeightDefined,
} from './utils';

export default function LineHeightControl( { value: lineHeight, onChange } ) {
export default function LineHeightControl( {
value: lineHeight,
onChange,
placeholder = BASE_DEFAULT_VALUE,
} ) {
const isDefined = isLineHeightDefined( lineHeight );

const handleOnKeyDown = ( event ) => {
Expand Down Expand Up @@ -70,7 +74,7 @@ export default function LineHeightControl( { value: lineHeight, onChange } ) {
onKeyDown={ handleOnKeyDown }
onChange={ handleOnChange }
label={ __( 'Line height' ) }
placeholder={ BASE_DEFAULT_VALUE }
placeholder={ placeholder }
Copy link
Member

Choose a reason for hiding this comment

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

I like this way of doing things. ❤️ Having the default value as the placeholder works I think as I can still set numerical values to 0 for example, and the native appearance (greyed out) indicates that it's a default value.

Just noting a couple of things down, all of which are probably best for follow ups if this gets off the ground.

  • We'll have to parse unit values (px, vh etc) from the default values and reset them in the controls where units change
  • I tested a few controls, and I think there will be some work to ensure we can pass default values as placeholders to the rest of our controls hooks. For example, box-control will require a tweak to handle placeholders for top/right/bottom/left inputs. This is for spacing controls such as margin and padding. But anything importing <UnitControl /> directly such as border-width should just work 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point regarding the units @ramonjd. I guess we can deal with units a follow-up to avoid growing this PR.

step={ STEP }
type="number"
value={ value }
Expand Down
51 changes: 51 additions & 0 deletions packages/block-editor/src/components/use-style/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/**
* External dependencies
*/
import { get } from 'lodash';

/**
* WordPress dependencies
*/
import { useSelect } from '@wordpress/data';
import { useMemo } from '@wordpress/element';

/**
* Internal dependencies
*/
import { useBlockEditContext } from '../block-edit';
import { store as blockEditorStore } from '../../store';
import { getValueFromVariable } from '../../utils/style-variable-resolution';

/**
* Hook that retrieves the global styles of a block.
* It works with nested objects using by finding the value at path.
*
* @param {string|Array} path The path to the setting.
*
* @return {any} Returns the style value defined for the path.
*
* @example
* ```js
* const backgroundColor = useStyle( 'color.background' );
* ```
*/
export default function useStyle( path ) {
const { name: blockName } = useBlockEditContext();

const settings = useSelect( ( select ) => {
return select( blockEditorStore ).getSettings();
}, [] );
const stylesForBlock = get( settings, [
'__experimentalStyles',
'blocks',
blockName,
] );
const value = get( stylesForBlock, path );
return useMemo( () => {
return getValueFromVariable(
settings.__experimentalFeatures,
blockName,
value
);
}, [ settings.__experimentalFeatures, blockName, value ] );
}
3 changes: 3 additions & 0 deletions packages/block-editor/src/hooks/line-height.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { hasBlockSupport } from '@wordpress/blocks';
import LineHeightControl from '../components/line-height-control';
import { cleanEmptyObject } from './utils';
import useSetting from '../components/use-setting';
import useStyle from '../components/use-style';

export const LINE_HEIGHT_SUPPORT_KEY = 'typography.lineHeight';

Expand All @@ -24,6 +25,7 @@ export function LineHeightEdit( props ) {
attributes: { style },
} = props;
const isDisabled = useIsLineHeightDisabled( props );
const defaultLineHeight = useStyle( [ 'typography', 'lineHeight' ] );

if ( isDisabled ) {
return null;
Expand All @@ -45,6 +47,7 @@ export function LineHeightEdit( props ) {
<LineHeightControl
value={ style?.typography?.lineHeight }
onChange={ onChange }
placeholder={ defaultLineHeight }
/>
);
}
Expand Down
4 changes: 4 additions & 0 deletions packages/block-editor/src/utils/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
export { default as transformStyles } from './transform-styles';
export * from './theme';
export * from './block-variation-transforms';
export {
getValueFromVariable as __experimentalGetValueFromVariable,
getPresetVariableFromValue as __experimentalGetPresetVariableFromValue,
} from './style-variable-resolution';
188 changes: 188 additions & 0 deletions packages/block-editor/src/utils/style-variable-resolution.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
/**
* External dependencies
*/
import { get, find, isString, kebabCase } from 'lodash';

/**
* WordPress dependencies
*/
import { __EXPERIMENTAL_PRESET_METADATA as PRESET_METADATA } from '@wordpress/blocks';

const STYLE_PROPERTIES_TO_CSS_VAR_INFIX = {
linkColor: 'color',
backgroundColor: 'color',
background: 'gradient',
};

function findInPresetsBy(
features,
blockName,
presetPath,
presetProperty,
Copy link
Member

Choose a reason for hiding this comment

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

How about naming these two presetPropertyName, presetPropertyValue to communicate what they are?

Copy link
Member

Choose a reason for hiding this comment

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

It seems the role of findInPresetsBy is to find the preset object whose presetPropertyName matches the presetPropertyValue among all the available.

The complexity here is that it needs to take into account that 1) there may be a block and a global preset and 2) any of those may have core, theme, and user presets. This is the full list of candidate presets available, sorted by priority:

  1. block preset from user
  2. block preset from theme
  3. block preset from core
  4. global preset from user
  5. global preset from theme
  6. global preset from core

My understanding is that we should return the first candidate whose presetPropertyName matches presetPropertyValue. Wouldn't that be enough? I've looked at the PR that introduced this function #33149 and I don't understand why we need the recursion when the property to look up is not the slug.

Copy link
Member

Choose a reason for hiding this comment

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

I've updated in trunk the implementation of this function to this algorithm:

  1. Create a list of the available presets, sorted by priority.
  2. Return the first preset.

And it works as described at #33149

Code, for reference:

function findInPresetsBy(
	styles,
	blockName,
	presetPath,
	presetPropertyName,
	presetPropertyValue
) {
	const candidates = [];
	const origins = [ 'user', 'theme', 'user' ];

	const blockPresets = get( styles, [
		'settings',
		'blocks',
		blockName,
		...presetPath,
	] );
	origins.forEach( ( origin ) => {
		if ( blockPresets?.[ origin ] ) {
			blockPresets[ origin ].forEach( ( preset ) => {
				if ( preset[ presetPropertyName ] === presetPropertyValue ) {
					candidates.push( preset );
				}
			} );
		}
	} );

	const globalPresets = get( styles, [ 'settings', ...presetPath ] );
	origins.forEach( ( origin ) => {
		if ( globalPresets?.[ origin ] ) {
			globalPresets[ origin ].forEach( ( preset ) => {
				if ( preset[ presetPropertyName ] === presetPropertyValue ) {
					candidates.push( preset );
				}
			} );
		}
	} );

	return candidates.length >= 1 ? candidates[ 0 ] : undefined;
}

Copy link
Member Author

@jorgefilipecosta jorgefilipecosta Aug 31, 2021

Choose a reason for hiding this comment

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

Hi @oandregal,

My understanding is that we should return the first candidate whose presetPropertyName matches presetPropertyValue. Wouldn't that be enough?

I don't think it is enough.
Imagining user theme.json defines presets:

{
	color: "#000"
	slug: "black",
	name: "Black"
}

And theme defines:

{
	color: "#001"
	slug: "black",
	name: "Black"
}

If we search the color property value of "#1" without the recursion we would return the theme preset, but we should not return it because the variable was overwritten and now there is no variable with color "#1".
The recursion makes sure that when we find a preset, there is no other higher priority preset with the same slug and different property because if there is the preset should be ignored.

Copy link
Member

Choose a reason for hiding this comment

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

Nice one 👍 I've pushed some tests in c78a322 to make sure this behavior is documented and retained.

presetValueValue
) {
// Block presets take priority above root level presets.
const orderedPresetsByOrigin = [
get( features, [ 'blocks', blockName, ...presetPath ] ),
get( features, presetPath ),
];
for ( const presetByOrigin of orderedPresetsByOrigin ) {
if ( presetByOrigin ) {
// Preset origins ordered by priority.
const origins = [ 'user', 'theme', 'core' ];
for ( const origin of origins ) {
const presets = presetByOrigin[ origin ];
if ( presets ) {
const presetObject = find(
presets,
( preset ) =>
preset[ presetProperty ] === presetValueValue
);
if ( presetObject ) {
if ( presetProperty === 'slug' ) {
return presetObject;
}
// if there is a highest priority preset with the same slug but different value the preset we found was overwritten and should be ignored.
const highestPresetObjectWithSameSlug = findInPresetsBy(
features,
blockName,
presetPath,
'slug',
presetObject.slug
);
if (
highestPresetObjectWithSameSlug[
presetProperty
] === presetObject[ presetProperty ]
) {
return presetObject;
}
return undefined;
}
}
}
}
}
}

function getValueFromPresetVariable(
features,
blockName,
variable,
[ presetType, slug ]
) {
const metadata = find( PRESET_METADATA, [ 'cssVarInfix', presetType ] );
if ( ! metadata ) {
return variable;
}

const presetObject = findInPresetsBy(
features,
blockName,
metadata.path,
'slug',
slug
);

if ( presetObject ) {
const { valueKey } = metadata;
const result = presetObject[ valueKey ];
return getValueFromVariable( features, blockName, result );
}

return variable;
}

function getValueFromCustomVariable( features, blockName, variable, path ) {
const result =
get( features, [ 'blocks', blockName, 'custom', ...path ] ) ??
get( features, [ 'custom', ...path ] );
if ( ! result ) {
return variable;
}
// A variable may reference another variable so we need recursion until we find the value.
return getValueFromVariable( features, blockName, result );
}

export function getValueFromVariable( features, blockName, variable ) {
if ( ! variable || ! isString( variable ) ) {
return variable;
}
const USER_VALUE_PREFIX = 'var:';
const THEME_VALUE_PREFIX = 'var(--wp--';
const THEME_VALUE_SUFFIX = ')';

let parsedVar;

if ( variable.startsWith( USER_VALUE_PREFIX ) ) {
parsedVar = variable.slice( USER_VALUE_PREFIX.length ).split( '|' );
} else if (
variable.startsWith( THEME_VALUE_PREFIX ) &&
variable.endsWith( THEME_VALUE_SUFFIX )
) {
parsedVar = variable
.slice( THEME_VALUE_PREFIX.length, -THEME_VALUE_SUFFIX.length )
.split( '--' );
} else {
// We don't know how to parse the value: either is raw of uses complex CSS such as `calc(1px * var(--wp--variable) )`
return variable;
}

const [ type, ...path ] = parsedVar;
if ( type === 'preset' ) {
return getValueFromPresetVariable(
features,
blockName,
variable,
path
);
}
if ( type === 'custom' ) {
return getValueFromCustomVariable(
Copy link
Member

@ramonjd ramonjd Sep 3, 2021

Choose a reason for hiding this comment

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

Following up on my comment, is the intention that something like styles['core/paragraph'].typography.lineHeight won't return a result if its value is a CSS var, e.g., --wp--custom--line-height--test and there is no matching value under settings.custom['line-height'].test?

If so, maybe we could return undefined from getValueFromCustomVariable() if no result is found, or check if variable === valueFromCustomVariable in this if statement and return early.

Not sure if that'll affect anything else 😄 , but doing this locally prevents the var(... appearing in the control input for me.

Or I guess we could handle it in the line-height hook itself, by checking for a number-type value. That might get more complex if we ever introduce units to line-height values though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also could only reproduce the issue when the custom variable isn't actually defined under the settings (eg var(--wp--custom--line-height--my-missing-var)), or if it maps to an object (eg var(--wp--custom--line-height)`, which looks to be what @scruffian hit below).

I tested out returning undefined from getValueFromCustomVariable() and it looks solid to me, if we also check for that second case when it maps to an object:

if ( ! result || ! isString( result ) ) {
	return undefined;
}

That tested well for me, including with the recursion with custom variables pointing to other custom variables etc.

features,
blockName,
variable,
path
);
}
return variable;
}

export function getPresetVariableFromValue(
features,
blockName,
presetPropertyName,
presetPropertyValue
) {
if ( ! presetPropertyValue ) {
return presetPropertyValue;
}

const cssVarInfix =
STYLE_PROPERTIES_TO_CSS_VAR_INFIX[ presetPropertyName ] ||
kebabCase( presetPropertyName );

const metadata = find( PRESET_METADATA, [ 'cssVarInfix', cssVarInfix ] );
if ( ! metadata ) {
// The property doesn't have preset data
// so the value should be returned as it is.
return presetPropertyValue;
}
const { valueKey, path } = metadata;

const presetObject = findInPresetsBy(
features,
blockName,
path,
valueKey,
presetPropertyValue
);

if ( ! presetObject ) {
// Value wasn't found in the presets,
// so it must be a custom value.
return presetPropertyValue;
}

return `var:preset|${ cssVarInfix }|${ presetObject.slug }`;
}
Loading