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

Block Support: Add height block support feature #32499

Closed
wants to merge 8 commits into from
Closed

Block Support: Add height block support feature #32499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e294e91
Add height dimensions block support
aaronrobertshaw Feb 9, 2022
74d8124
Initial updates to extend theme.json classes post 5.9
aaronrobertshaw Feb 16, 2022
a457e8e
New settings (works in WordPress 5.9)
oandregal Feb 17, 2022
8d0b11e
New styles (works in WordPress 5.9)
oandregal Feb 17, 2022
fbc677d
Fix for WordPress 5.9: gutenberg_get_global_stylesheet should call th…
oandregal Feb 17, 2022
c4b286b
Fix lint issues
oandregal Feb 17, 2022
34f96ed
Move functions to the compat/6.0 folder
oandregal Feb 17, 2022
0023551
Add `get_theme_item` to global styles controller
aaronrobertshaw Feb 18, 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
10 changes: 10 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 @@ -55,6 +55,16 @@ Settings related to colors.

---

### dimensions

Settings related to dimensions.

| Property | Type | Default | Props |
| --- | --- | --- |--- |
| height | boolean | false | |

---

### layout

Settings related to layout.
Expand Down
18 changes: 14 additions & 4 deletions lib/block-supports/dimensions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ function gutenberg_register_dimensions_support( $block_type ) {
}

$has_dimensions_support = gutenberg_block_has_support( $block_type, array( '__experimentalDimensions' ), false );
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for being late for the conversation. I've noticed that in #32392 we renamed a lot of things from spacing to dimensions. Couldn't look at it deeply yet, but wanted to flag that changing keys in block.json or theme.json is problematic if they already landed in WordPress core.

For example, here we're changing spacing to __experimentalDimensions in block.json. But spacing has already landed in WordPress core, so we need to keep spacing working as before. If we absolutely need to rename it, we can add some code that converts spacing into dimensions. If it's just a name preference, I'd advise to keep using what we have (spacing) and don't make our work harder for this.

At the UI level, it can be labeled differently if we need to and we also can change it as many times as we need.

FYI @jorgefilipecosta @youknowriad

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 gone through a first pass at #32392 I may have found an issue https://github.com/WordPress/gutenberg/pull/32392/files?file-filters%5B%5D=.js&file-filters%5B%5D=.json&file-filters%5B%5D=.php#r686752790

If we keep the spacing key, I'd suggest we rename the things back to spacing internally, although at the UI level we name them "dimensions".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Couldn't look at it deeply yet, but wanted to flag that changing keys in block.json or theme.json is problematic if they already landed in WordPress core.

Thanks @nosolosw, I'd thought of that and thought I'd maintained that backwards compatibility. Happy to change whatever I've messed up. I'm missing where that is though.

For example, here we're changing spacing to __experimentalDimensions in block.json. But spacing has already landed in WordPress core, so we need to keep spacing working as before.

I've actually not changed spacing to __experimentalDimensions in the block.json or theme.json. In the line below this comment you can still see the check for the spacing portion of the overall dimensions themed block support.

My plan was to introduce the new specific dimensions related supports under "dimensions" itself. It didn't make sense to me to have height, width etc under spacing.

Some points about where the separation of dimensions and spacing keys/support occurs are:

  • lib/class-wp-theme-json-gutenberg.php adds dimensions in addition to, rather than replacing spacing
  • lib/theme.json adds the setting for dimensions.customHeight separate to spacing
  • packages/block-editor/src/hooks/dimensions.js checks separate block.json flags for spacing vs __experimentalDimensions support
  • packages/block-editor/src/hooks/style.js dimensions key was added alongside spacing for the inline style generation
  • The padding/margin styles are still under spacing within the block attributes style object
  • The written tests also indicate that the spacing keys were still maintained.

Your comment regarding the key used when registering the block support in dimensions.php was well made and I have a fix for that specifically in #34030.

Is there anything else I have missed? Thanks for your help as always!

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 quick turnaround, Aaron. I have a second thought about the other PR. I can review this one properly once we clarify whether what I've brought up is an issue or not.

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've replied on the other PR seeking a little clarification on the desired changes. Hopefully, I'll have that fixed up tomorrow. Then will make the required changes here.

There was also some work based on this PR around adding min-height support and using it to unify two "dimensions" panels in the Cover block. It looks like that may also need some further discussion although the hope was to have that sorted before the next release.

Really appreciate your insight on the dimensions/spacing backwards compatibility issue with core. Thanks!

// Future block supports such as height & width will be added here.

if ( $has_dimensions_support ) {
$block_type->attributes['style'] = array(
Expand All @@ -44,14 +43,24 @@ function gutenberg_register_dimensions_support( $block_type ) {
*
* @return array Block dimensions CSS classes and inline styles.
*/
function gutenberg_apply_dimensions_support( $block_type, $block_attributes ) { // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable
function gutenberg_apply_dimensions_support( $block_type, $block_attributes ) {
if ( gutenberg_skip_dimensions_serialization( $block_type ) ) {
return array();
}

$styles = array();

// Height support to be added in near future.
// Height.
$has_height_support = gutenberg_block_has_support( $block_type, array( '__experimentalDimensions', 'height' ), false );

if ( $has_height_support ) {
$height_value = _wp_array_get( $block_attributes, array( 'style', 'dimensions', 'height' ), null );

if ( null !== $height_value ) {
$styles[] = sprintf( 'height: %s;', $height_value );
}
}

// Width support to be added in near future.

return empty( $styles ) ? array() : array( 'style' => implode( ' ', $styles ) );
Expand All @@ -63,10 +72,11 @@ function gutenberg_apply_dimensions_support( $block_type, $block_attributes ) {
*
* @param WP_Block_type $block_type Block type.
*
* @return boolean Whether to serialize spacing support styles & classes.
* @return boolean Whether to serialize dimensions support styles & classes.
*/
function gutenberg_skip_dimensions_serialization( $block_type ) {
$dimensions_support = _wp_array_get( $block_type->supports, array( '__experimentalDimensions' ), false );

return is_array( $dimensions_support ) &&
array_key_exists( '__experimentalSkipSerialization', $dimensions_support ) &&
$dimensions_support['__experimentalSkipSerialization'];
Expand Down
4 changes: 2 additions & 2 deletions lib/block-supports/layout.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ function gutenberg_render_layout_support_flag( $block_content, $block ) {
return $block_content;
}

$block_gap = wp_get_global_settings( array( 'spacing', 'blockGap' ) );
$default_layout = wp_get_global_settings( array( 'layout' ) );
$block_gap = gutenberg_get_global_settings( array( 'spacing', 'blockGap' ) );
$default_layout = gutenberg_get_global_settings( array( 'layout' ) );
$has_block_gap_support = isset( $block_gap ) ? null !== $block_gap : false;
$default_block_layout = _wp_array_get( $block_type->supports, array( '__experimentalLayout', 'default' ), array() );
$used_layout = isset( $block['attrs']['layout'] ) ? $block['attrs']['layout'] : $default_block_layout;
Expand Down
8 changes: 4 additions & 4 deletions lib/compat/wordpress-5.9/class-wp-theme-json-5-9.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1540,7 +1540,7 @@ public function get_svg_filters( $origins ) {
}

/**
* Returns whether a presets should be overriden or not.
* Returns whether a presets should be overridden or not.
*
* @param array $theme_json The theme.json like structure to inspect.
* @param array $path Path to inspect.
Expand All @@ -1555,8 +1555,8 @@ protected static function should_override_preset( $theme_json, $path, $override
// The relationship between whether to override the defaults
// and whether the defaults are enabled is inverse:
//
// - If defaults are enabled => theme presets should not be overriden
// - If defaults are disabled => theme presets should be overriden
// - If defaults are enabled => theme presets should not be overridden
// - If defaults are disabled => theme presets should be overridden
//
// For example, a theme sets defaultPalette to false,
// making the default palette hidden from the user.
Expand Down Expand Up @@ -1645,7 +1645,7 @@ protected function get_name_from_defaults( $slug, $base_path ) {
* Removes the preset values whose slug is equal to any of given slugs.
*
* @param array $node The node with the presets to validate.
* @param array $slugs The slugs that should not be overriden.
* @param array $slugs The slugs that should not be overridden.
*
* @return array The new node
*/
Expand Down
194 changes: 0 additions & 194 deletions lib/compat/wordpress-5.9/get-global-styles-and-settings.php
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion lib/compat/wordpress-5.9/global-styles-css-custom-properties.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*/
function wp_enqueue_global_styles_css_custom_properties() {
wp_register_style( 'global-styles-css-custom-properties', false, array(), true, true );
wp_add_inline_style( 'global-styles-css-custom-properties', wp_get_global_stylesheet( array( 'variables' ) ) );
wp_add_inline_style( 'global-styles-css-custom-properties', gutenberg_get_global_stylesheet( array( 'variables' ) ) );
wp_enqueue_style( 'global-styles-css-custom-properties' );
}
add_filter( 'enqueue_block_editor_assets', 'wp_enqueue_global_styles_css_custom_properties' );
Expand Down
2 changes: 1 addition & 1 deletion lib/compat/wordpress-5.9/script-loader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function gutenberg_enqueue_global_styles_assets() {
return;
}

$stylesheet = wp_get_global_stylesheet();
$stylesheet = gutenberg_get_global_stylesheet();

if ( empty( $stylesheet ) ) {
return;
Expand Down
3 changes: 3 additions & 0 deletions lib/compat/wordpress-5.9/theme.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@
],
"text": true
},
"dimensions": {
"height": false
},
"spacing": {
"blockGap": null,
"margin": false,
Expand Down
48 changes: 48 additions & 0 deletions lib/compat/wordpress-6.0/class-gutenberg-rest-global-styles-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,52 @@ public function get_theme_items( $request ) {

return $response;
}

/**
* Returns the given theme global styles config.
*
* @since 5.9.0
*
* @param WP_REST_Request $request The request instance.
* @return WP_REST_Response|WP_Error
*/
public function get_theme_item( $request ) {
if ( wp_get_theme()->get_stylesheet() !== $request['stylesheet'] ) {
// This endpoint only supports the active theme for now.
return new WP_Error(
'rest_theme_not_found',
__( 'Theme not found.', 'gutenberg' ),
array( 'status' => 404 )
);
}

$theme = WP_Theme_JSON_Resolver_Gutenberg::get_merged_data( 'theme' );
$data = array();
$fields = $this->get_fields_for_response( $request );

if ( rest_is_field_included( 'settings', $fields ) ) {
$data['settings'] = $theme->get_settings();
}

if ( rest_is_field_included( 'styles', $fields ) ) {
$raw_data = $theme->get_raw_data();
$data['styles'] = isset( $raw_data['styles'] ) ? $raw_data['styles'] : array();
}

$context = ! empty( $request['context'] ) ? $request['context'] : 'view';
$data = $this->add_additional_fields_to_object( $data, $request );
$data = $this->filter_response_by_context( $data, $context );

$response = rest_ensure_response( $data );

$links = array(
'self' => array(
'href' => rest_url( sprintf( '%s/%s/themes/%s', $this->namespace, $this->rest_base, $request['stylesheet'] ) ),
),
);

$response->add_links( $links );

return $response;
}
}
Loading