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 Custom CSS support to the Group block #25413

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
72 changes: 72 additions & 0 deletions lib/block-supports/custom-css.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
<?php
/**
* Align block support flag.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Align block support flag.
* Custom-CSS block support flag.

*
* @package gutenberg
*/

/**
* Registers the custom css block attribute for block types that support it.
*
* @param WP_Block_Type $block_type Block Type.
*/
function gutenberg_register_custom_css_support( $block_type ) {
$has_custom_css_support = false;
if ( property_exists( $block_type, 'supports' ) ) {
$has_custom_css_support = gutenberg_experimental_get( $block_type->supports, array( 'customCSS' ), false );
}
if ( $has_custom_css_support ) {
if ( ! $block_type->attributes ) {
$block_type->attributes = array();
}

if ( ! array_key_exists( 'css', $block_type->attributes ) ) {
$block_type->attributes['css'] = array(
'type' => 'string',
);
}
}
}

/**
* Add generated CSS class and the stylesheet.
*
* @param array $attributes Comprehensive list of attributes to be applied.
* @param array $block_attributes Block attributes.
* @param WP_Block_Type $block_type Block Type.
*
* @return array Block custom CSS classes.
*/
function gutenberg_apply_custom_css_support( $attributes, $block_attributes, $block_type ) {
$has_custom_css_support = false;
if ( property_exists( $block_type, 'supports' ) ) {
$has_custom_css_support = gutenberg_experimental_get( $block_type->supports, array( 'customCSS' ), false );
}
if ( $has_custom_css_support && isset( $block_attributes['css'] ) && ! empty( $block_attributes[ 'css' ] ) ) {
$generated_class_name = uniqid( 'wp-block-css-' );
wp_add_inline_style(
'wp-block-custom-css',
str_replace( ':self', '.' . $generated_class_name, $block_attributes[ 'css' ] )
);
$attributes['css_classes'][] = $generated_class_name;
}

return $attributes;
}

/**
* This style is used to enqueue the dynamic block custom CSS styles.
*/
function gutenberg_register_custom_css_style() {
wp_register_style('wp-block-custom-css', false);
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'm using an empty style to enqueue the different block custom CSS.
Let me know if there are better ways to do that.

Copy link
Member

Choose a reason for hiding this comment

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

Ideally we wouldn't enqueue anything... These would be a lot better as inline styles in the element itself. 👍
If the user adds styles to a specific block, then they want those styles applied and they should therefore override any global CSS from stylesheets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally we wouldn't enqueue anything... These would be a lot better as inline styles in the element itself. 👍

This assumes there's a way to alter the markup of the block and add this somewhere there. IMO it's a bit more fragile because of the DOM changes needed.

Copy link
Member

@aristath aristath Sep 17, 2020

Choose a reason for hiding this comment

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

I was thinking we could handle this the same way we handle extra, user-defined classes... The logic we follow for that is pretty robust and we could do the same for custom-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.

We don't have extra markup inside the block wrapper though, also, we can't assume the wrapper accept children or is a self closing tag.

Copy link
Member

@aristath aristath Sep 18, 2020

Choose a reason for hiding this comment

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

The way I was envisioning it would be a bit more restrictive...
So users wouldn't be able to type full CSS like :self{color:red;}. Instead they'd only write the rules (no selector) like color:red;.
That would then be properly added to the element as inline CSS using style="" in the wrapper itself, there's no need to alter any other markup, children etc. It would be a lot easier to sanitize too (basically esc_attr on the PHP side and an equivalent in JS).
That's why I envisioned it similar to the "Additional CSS class(es)" field... The implementation would be pretty much identical.
So the field wouldn't be so much a "custom CSS" field... it would be "custom styles" - the difference being it will only contain the actual styles so no selectors etc, can't be considered full CSS since there's nothing cascading about it.
Would also address most of @ZebulanStanphill's concerns on #25413 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I thought about that, the problem with that though, is that you can alter pseudo-states and things like that.

Copy link
Member

Choose a reason for hiding this comment

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

True... But if someone is knowledgeble enough to know what pseudo-elements are, they can surely add a custom class to their element and then add whatever CSS they want... They can even add CSS in a custom HTML block. 😉
A feature for custom-styles should appeal and be usable by the masses. For more advanced features there are more advanced solutions to adding 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.

It's added to the "Advanced" section for a reason. I think that's where the disagreement lies, I do think it's a feature for advanced users regardless of how it's implemented. It's something to be used to create designs that are not possible with regular controls. Imagine creative things like animations, filters, rotations... in predefined block patterns.

You might be able to achieve these things with dedicated plugins... but for adhoc situations where you need these things, you don't want to install a plugin that supports animations, rotations for Core blocks and create potential invalidation.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with Riad that this feature is useful as it stands, even when there are other more generic tools to achieve similar results.

This feature allows styles to be tied specifically to a block, regardless of the chosen selectors. Providing :self is very useful, but it is even better when we allow advanced use cases — not just pseudo-states and nested elements — like (why not) selecting elements outside of :self.

}

add_action( 'init', 'gutenberg_register_custom_css_style' );

/**
* Enqueue an empty style to the footer.
*/
function gutenberg_enqueue_custom_css_style() {
wp_enqueue_style( 'wp-block-custom-css' );
}
add_action( 'get_footer', 'gutenberg_enqueue_custom_css_style' );
15 changes: 13 additions & 2 deletions lib/block-supports/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ function gutenberg_register_block_supports() {
gutenberg_register_colors_support( $block_type );
gutenberg_register_typography_support( $block_type );
gutenberg_register_custom_classname_support( $block_type );
gutenberg_register_custom_css_support( $block_type );
}
}

Expand All @@ -37,16 +38,26 @@ function gutenberg_apply_block_supports( $block_content, $block ) {

$block_type = WP_Block_Type_Registry::get_instance()->get_registered( $block['blockName'] );
// If no render_callback, assume styles have been previously handled.
if ( ! $block_type || ! $block_type->render_callback ) {
// Custom CSS is also a server-side hook even for static blocks.
$has_custom_css_support = false;
if ( property_exists( $block_type, 'supports' ) ) {
$has_custom_css_support = gutenberg_experimental_get( $block_type->supports, array( 'customCSS' ), false );
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally, this shouldn't be necessary here and block support application should be "opt-in" maybe? or use a dedicated function like proposed by @mcsf

Copy link
Contributor

Choose a reason for hiding this comment

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

There are a few ways to interpret this, and I think you and I discussed more than one way to handle it. Can you detail your suggestions?

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'm just saying that the function you proposed wp_block_supports should make this useless once implemented but I guess it will force us to have a render_callback for the group block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thinking more here, it seems the "automatic" approach we have now make this easier, otherwise we'll have to "duplicate" the group "save" function on the backend somehow?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm super undecided here. I feel like this is probably not the solution we want for the future, but I feel out of good ideas. That's also why I stressed further below that we should keep the feature experimental. That way I'm not keeping your PR from being merged. :)


if (
! ( $block_type && $block_type->render_callback ) &&
! $has_custom_css_support
) {
Comment on lines +47 to +50
Copy link
Contributor

Choose a reason for hiding this comment

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

Regardless of the solution in the previous comment, this piece here is definitely something to address: all the other transformations after the early return are for dynamic blocks, and gutenberg_apply_custom_css_support is the elusive exception. Static-friendly hooks and dynamic-only hooks should be invoked in clearly different places.

return $block_content;
}

$attributes = array();
$attributes = gutenberg_apply_generated_classname_support( $attributes, $block['attrs'], $block_type );
$attributes = gutenberg_apply_colors_support( $attributes, $block['attrs'], $block_type );
$attributes = gutenberg_apply_typography_support( $attributes, $block['attrs'], $block_type );
$attributes = gutenberg_apply_alignment_support( $attributes, $block['attrs'], $block_type );
$attributes = gutenberg_apply_custom_classname_support( $attributes, $block['attrs'], $block_type );
$attributes = gutenberg_apply_custom_css_support( $attributes, $block['attrs'], $block_type );

if ( ! count( $attributes ) ) {
return $block_content;
Expand Down
1 change: 1 addition & 0 deletions lib/load.php
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,4 @@ function gutenberg_is_experiment_enabled( $name ) {
require dirname( __FILE__ ) . '/block-supports/typography.php';
require dirname( __FILE__ ) . '/block-supports/custom-classname.php';
require dirname( __FILE__ ) . '/block-supports/generated-classname.php';
require dirname( __FILE__ ) . '/block-supports/custom-css.php';
100 changes: 100 additions & 0 deletions packages/block-editor/src/hooks/custom-css.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/**
* External dependencies
*/
import { has, replace } from 'lodash';

/**
* WordPress dependencies
*/
import { createHigherOrderComponent } from '@wordpress/compose';
import { addFilter } from '@wordpress/hooks';
import { hasBlockSupport } from '@wordpress/blocks';
import { TextareaControl } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { useEffect } from '@wordpress/element';

/**
* Internal dependencies
*/
import InspectorAdvancedControls from '../components/inspector-advanced-controls';

/**
* Filters registered block settings, extending attributes to include `css`.
*
* @param {Object} settings Original block settings
* @return {Object} Filtered block settings
*/
export function addAttribute( settings ) {
// allow blocks to specify their own attribute definition with default values if needed.
if ( has( settings.attributes, [ 'css', 'type' ] ) ) {
return settings;
}
if ( hasBlockSupport( settings, 'customCSS' ) ) {
// Gracefully handle if settings.attributes is undefined.
settings.attributes = {
...settings.attributes,
css: {
type: 'string',
},
};
}

return settings;
}

/**
* Override the default edit UI to include new toolbar controls for block
* alignment, if block defines support.
*
* @param {Function} BlockEdit Original component
* @return {Function} Wrapped component
*/
export const withToolbarControls = createHigherOrderComponent(
( BlockEdit ) => ( props ) => {
const { name: blockName, attributes, clientId } = props;
const hasSupport = hasBlockSupport( blockName, 'customCSS', false );

const updateCustomCSS = ( css ) => {
props.setAttributes( { css } );
};

useEffect( () => {
if ( ! hasSupport && ! attributes.css ) {
return;
}
const node = document.createElement( 'style' );
node.innerHTML = replace(
attributes.css,
/:self/g,
Comment on lines +66 to +68
Copy link
Contributor

Choose a reason for hiding this comment

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

Why lodash and not String::replace? Anyway, I pushed a commit to make sure we replaced all occurrences (using the RegExp g flag).

`[data-block="${ clientId }"]`
);
document.body.appendChild( node );

return () => document.body.removeChild( node );
}, [ hasSupport, attributes.css ] );

return [
hasSupport && (
<InspectorAdvancedControls key="css-controls">
<TextareaControl
label={ __( 'Custom CSS' ) }
help={ __(
'Use :self selector to refer to the current block'
) }
value={ attributes.css || '' }
onChange={ updateCustomCSS }
/>
</InspectorAdvancedControls>
),
<BlockEdit key="edit" { ...props } />,
];
},
'withToolbarControls'
);

addFilter( 'blocks.registerBlockType', 'core/css/addAttribute', addAttribute );
addFilter(
'editor.BlockEdit',
'core/editor/css/with-toolbar-controls',
withToolbarControls
);
1 change: 1 addition & 0 deletions packages/block-editor/src/hooks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import { AlignmentHookSettingsProvider } from './align';
import './anchor';
import './custom-class-name';
import './custom-css';
import './generated-class-name';
import './style';
import './color';
Expand Down
3 changes: 2 additions & 1 deletion packages/block-library/src/group/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"gradients": true,
"linkColor": true
},
"__experimentalPadding": true
"__experimentalPadding": true,
"customCSS": true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I make it experimental to start with?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make it experimental. That way we have some more time to settle the matter of automatic / opt-in / opt-out solutions for front-end transformations.

}
}