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

Jump to bottom

Add interactivity states UI support via Styles Engine #41708

Closed
wants to merge 15 commits into from
Closed

Add interactivity states UI support via Styles Engine #41708

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
11d647f
Initial commit.
ramonjd Jun 9, 2022
3350278
Lint and other stuff zzzz
ramonjd Jun 9, 2022
61f00f6
Removed unused `value_func`
ramonjd Jun 9, 2022
e33be11
Testing adding button element and transition states.
ramonjd Jun 9, 2022
4221e86
Add support for focus for links and disabled for buttons
getdave Jun 10, 2022
cbedaba
Update tests to assert on focus and disabled state styles
getdave Jun 10, 2022
6838a16
DRY up by dynamically generating state definitions
getdave Jun 10, 2022
38d3bb2
Hack: Filter safe CSS rules to enable transition
getdave Jun 10, 2022
5783d9f
Basic link hover key controls
getdave May 26, 2022
b0d7958
Adds classname
getdave May 26, 2022
91bb650
Update block attr data structure
getdave Jun 13, 2022
879d300
Utilise style engine
getdave Jun 13, 2022
d5f261e
Remove debugging
getdave Jun 13, 2022
3e02bea
Fix spacing on selector before bracket
getdave Jun 13, 2022
6248a02
Generate rules for psuedo selectors and output
getdave Jun 14, 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
47 changes: 30 additions & 17 deletions lib/block-supports/elements.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,26 +96,39 @@ function gutenberg_render_elements_support_styles( $pre_render, $block ) {
* should take advantage of WP_Theme_JSON_Gutenberg::compute_style_properties
* and work for any element and style.
*/
$skip_link_color_serialization = gutenberg_should_skip_block_supports_serialization( $block_type, 'color', 'link' );
// $skip_link_color_serialization = gutenberg_should_skip_block_supports_serialization( $block_type, 'color', 'link' );

if ( $skip_link_color_serialization ) {
return null;
// if ( $skip_link_color_serialization ) {
// return null;
// }
$class_name = gutenberg_get_elements_class_name( $block );
// $link_block_styles = isset( $element_block_styles['link'] ) ? $element_block_styles['link'] : null;

$css_styles = '';

// $style_definition = _wp_array_get( WP_Style_Engine::BLOCK_STYLE_DEFINITIONS_METADATA, $style_definition_path, null );

if ( ! is_array( $element_block_styles ) ) {
return;
}
$class_name = gutenberg_get_elements_class_name( $block );
$link_block_styles = isset( $element_block_styles['link'] ) ? $element_block_styles['link'] : null;

if ( $link_block_styles ) {
$styles = gutenberg_style_engine_generate(
$link_block_styles,
array(
'selector' => ".$class_name a",
'css_vars' => true,
)
);

if ( ! empty( $styles['css'] ) ) {
gutenberg_enqueue_block_support_styles( $styles['css'] );
}
// Currently this is `elements -> link -> DEFS`.
// In the future we should extend $element_block_styles
// to include all DEFS from all supported elements.
$block_styles = array(
'elements' => $element_block_styles,
);

$style_defs = gutenberg_style_engine_generate(
$block_styles,
array(
'selector' => ".$class_name",
'css_vars' => true,
)
);
Comment on lines +122 to +128
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will now include any psuedo selectors


if ( ! empty( $style_defs['css'] ) ) {
gutenberg_enqueue_block_support_styles( $style_defs['css'] );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This enqueues the <style> tag within the editor which appears directly below the block in the DOM.

}

return null;
Expand Down
9 changes: 9 additions & 0 deletions lib/load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ function gutenberg_is_experiment_enabled( $name ) {
require __DIR__ . '/demo.php';
require __DIR__ . '/experiments-page.php';

add_filter(
'safe_style_css',
function( $safe_rules ) {
$safe_rules[] = 'transition';
return $safe_rules;
}
);

// Copied package PHP files.
if ( file_exists( __DIR__ . '/../build/style-engine/class-wp-style-engine-gutenberg.php' ) ) {
require_once __DIR__ . '/../build/style-engine/class-wp-style-engine-gutenberg.php';
Expand All @@ -166,3 +174,4 @@ function gutenberg_is_experiment_enabled( $name ) {
require __DIR__ . '/block-supports/spacing.php';
require __DIR__ . '/block-supports/dimensions.php';
require __DIR__ . '/block-supports/duotone.php';

46 changes: 45 additions & 1 deletion packages/block-editor/src/hooks/color.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,13 @@ const resetAllLinkFilter = ( attributes ) => ( {
),
} );

const resetAllLinkHoverFilter = ( attributes ) => ( {
style: clearColorFromStyles(
[ 'elements', 'link', 'states', 'hover', 'color', 'text' ],
attributes.style
),
} );

/**
* Clears all background color related properties including gradients from
* supplied block attributes.
Expand Down Expand Up @@ -216,7 +223,6 @@ export function addSaveProps( props, blockType, attributes ) {
backgroundColor ||
style?.color?.background ||
( hasGradient && ( gradient || style?.color?.gradient ) );

const newClassName = classnames(
props.className,
textClass,
Expand All @@ -232,6 +238,9 @@ export function addSaveProps( props, blockType, attributes ) {
'has-background': serializeHasBackground && hasBackground,
'has-link-color':
shouldSerialize( 'link' ) && style?.elements?.link?.color,
'has-link-hover-color':
shouldSerialize( 'link' ) &&
style?.elements?.link?.states?.hover?.color,
}
);
props.className = newClassName ? newClassName : undefined;
Expand Down Expand Up @@ -284,6 +293,7 @@ const getLinkColorFromAttributeValue = ( colors, value ) => {
*/
export function ColorEdit( props ) {
const { name: blockName, attributes } = props;

// Some color settings have a special handling for deprecated flags in `useSetting`,
// so we can't unwrap them by doing const { ... } = useSetting('color')
// until https://github.com/WordPress/gutenberg/issues/37094 is fixed.
Expand Down Expand Up @@ -443,6 +453,26 @@ export function ColorEdit( props ) {
};
};

const onChangeLinkHoverColor = ( value ) => {
const colorObject = getColorObjectByColorValue( allSolids, value );
const newLinkColorValue = colorObject?.slug
? `var:preset|color|${ colorObject.slug }`
: value;

const newStyle = cleanEmptyObject(
immutableSet(
localAttributes.current?.style,
[ 'elements', 'link', 'states', 'hover', 'color', 'text' ],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This structure is intended to marry up the the nesting within the states key of Theme JSON. This allows the Style Engine to pick out the correct color for processing on server and client side.

It's all very hard coded at this point.

newLinkColorValue
)
);
props.setAttributes( { style: newStyle } );
localAttributes.current = {
...localAttributes.current,
...{ style: newStyle },
};
};

const enableContrastChecking =
Platform.OS === 'web' && ! gradient && ! style?.color?.gradient;

Expand Down Expand Up @@ -508,6 +538,20 @@ export function ColorEdit( props ) {
isShownByDefault: defaultColorControls?.link,
resetAllFilter: resetAllLinkFilter,
},
{
label: __( 'Link Hover' ),
onColorChange: onChangeLinkHoverColor,
colorValue: getLinkColorFromAttributeValue(
allSolids,
style?.elements?.link?.states?.hover?.color
?.text
),
clearable: !! style?.elements?.link?.states
?.hover?.color?.text,
isShownByDefault:
defaultColorControls?.link?.states?.hover,
resetAllFilter: resetAllLinkHoverFilter,
},
]
: [] ),
] }
Expand Down
56 changes: 48 additions & 8 deletions packages/block-editor/src/hooks/style.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { get, has, isEmpty, kebabCase, omit } from 'lodash';
import { get, has, isEmpty, kebabCase, omit, set } from 'lodash';
import classnames from 'classnames';

/**
Expand Down Expand Up @@ -60,8 +60,9 @@ function compileStyleValue( uncompiledValue ) {
/**
* Returns the inline styles to add depending on the style object
*
* @param {Object} styles Styles configuration.
* @param {Object} styles Styles configuration.
*
* @param options
* @return {Object} Flattened CSS variables declaration.
*/
export function getInlineStyles( styles = {} ) {
Expand Down Expand Up @@ -98,28 +99,67 @@ export function getInlineStyles( styles = {} ) {
// The goal is to move everything to server side generated engine styles
// This is temporary as we absorb more and more styles into the engine.
const extraRules = getCSSRules( styles );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

getCSSRules will now generate the rules so that any states rules will have the psuedo selector as the selector prop of the rule.


extraRules.forEach( ( rule ) => {
output[ rule.key ] = rule.value;
let pseudoSelector = '';
// Key value data structure cannot represent pseudo selectors.
// Create a nested "states" key for pseudo selector rules.
if ( rule?.selector?.startsWith( ':' ) ) {
// Primitive check for pseudo selector. In future
// we should make this a formal prop of the style rule from getCSSRules.
pseudoSelector = rule.selector.replace( ':', '' );
set( output, [ 'states', pseudoSelector, rule.key ], rule.value );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doing this changes the contract implied by getInlineStyles because now there is a random states key in it whereas before it was just rule -> value.

Not sure how we avoid this. We need to convey the information about pseudo selectors but the data structure isn't conducive to this. Ideas welcomed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ramonjd The key / value data structure here isn't conducive to handling pseudo selectors, but we need it to in order to output the correct rules within the editor.

What do you think of my approach? Are we abusing things too heavily here? Or should we change the returned data structure and update all instances within Gutenberg?

This function isn't a public API so we should be ok in that regard.

Your expertise would be greatly appreciated.

Copy link
Member

Choose a reason for hiding this comment

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

Hi @getdave

Thanks for getting things going here.

It's a tough one. I remember hooks/style.js threw some chilli into the soup when I first look at porting elements over to the JS style engine.

The key / value data structure here isn't conducive to handling pseudo selectors

Totally.

I can't help but recommend a more iterative approach here, that is, first let's get basic support for elements in the style engine in, then deal with new functionality. Similar to what we've been looking at over in #41619

I'm not saying it's the panacea, but it has helped to smooth out progress, and gives us time to consider holistic approaches.

Along those lines I've started on the JS part for elements over here: #41732

I'll also test this out with the new :hover object as well, which might necessitate a new ruleset or constant to inform the style engine about eligible elements.

} else {
output[ rule.key ] = rule.value;
}
} );

return output;
}

function generateElementStyleSelector(
selector,
element,
styles,
pseudoSelector = ''
) {
return [
`.editor-styles-wrapper .${ selector } ${ element }${ pseudoSelector } {`,
...Object.entries( styles ).map(
( [ cssProperty, value ] ) =>
`\t${ kebabCase( cssProperty ) }: ${ value };`
),
'}',
];
}

function compileElementsStyles( selector, elements = {} ) {
return Object.entries( elements )
.map( ( [ element, styles ] ) => {
const elementStyles = getInlineStyles( styles );

if ( ! isEmpty( elementStyles ) ) {
// The .editor-styles-wrapper selector is required on elements styles. As it is
// added to all other editor styles, not providing it causes reset and global
// styles to override element styles because of higher specificity.
return [
`.editor-styles-wrapper .${ selector } ${ ELEMENTS[ element ] }{`,
...Object.entries( elementStyles ).map(
( [ cssProperty, value ] ) =>
`\t${ kebabCase( cssProperty ) }: ${ value };`
// Default selectors
...generateElementStyleSelector(
selector,
ELEMENTS[ element ],
omit( elementStyles, [ 'states' ] )
),
// State "pseudo selectors"
...Object.keys( elementStyles?.states )?.flatMap(
( stateKey ) => {
return generateElementStyleSelector(
selector,
ELEMENTS[ element ],
elementStyles?.states[ stateKey ],
`:${ stateKey }`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think a util function here which converts from stateKey (hover) to pseudo selector (:hover) would help communicate the intent of the code better.

);
}
),
'}',
].join( '\n' );
}
return '';
Expand Down
Loading