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

Add inherit content size to base styles #42664

Closed
wants to merge 4 commits into from
Closed

Add inherit content size to base styles #42664

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fcfce15
Add inherit content size to base styles
tellthemachines Jul 25, 2022
2e9719f
Use only one classname for both content sizes.
tellthemachines Jul 26, 2022
8f219a1
Content and wide values fallback to each other.
tellthemachines Jul 26, 2022
3addf03
Check if safe CSS values.
tellthemachines Jul 27, 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
61 changes: 34 additions & 27 deletions lib/block-supports/layout.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support
if ( 'default' === $layout_type ) {
$content_size = isset( $layout['contentSize'] ) ? $layout['contentSize'] : '';
$wide_size = isset( $layout['wideSize'] ) ? $layout['wideSize'] : '';
$inherit = isset( $layout['inherit'] ) ? $layout['inherit'] : '';

$all_max_width_value = $content_size ? $content_size : $wide_size;
$wide_max_width_value = $wide_size ? $wide_size : $content_size;
Expand All @@ -62,25 +63,24 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support
$style .= '}';

$style .= "$selector > .alignwide { max-width: " . esc_html( $wide_max_width_value ) . ';}';
$style .= "$selector .alignfull { max-width: none; }";

if ( isset( $block_spacing ) ) {
$block_spacing_values = gutenberg_style_engine_get_block_supports_styles(
array(
'spacing' => $block_spacing,
)
);

// Handle negative margins for alignfull children of blocks with custom padding set.
// They're added separately because padding might only be set on one side.
if ( isset( $block_spacing_values['declarations']['padding-right'] ) ) {
$padding_right = $block_spacing_values['declarations']['padding-right'];
$style .= "$selector > .alignfull { margin-right:calc($padding_right * -1); }";
}
if ( isset( $block_spacing_values['declarations']['padding-left'] ) ) {
$padding_left = $block_spacing_values['declarations']['padding-left'];
$style .= "$selector > .alignfull { margin-left: calc($padding_left * -1); }";
}
}

if ( ( $content_size || $wide_size || $inherit ) && isset( $block_spacing ) ) {
$block_spacing_values = gutenberg_style_engine_get_block_supports_styles(
array(
'spacing' => $block_spacing,
)
);

// Handle negative margins for alignfull children of blocks with custom padding set.
// They're added separately because padding might only be set on one side.
if ( isset( $block_spacing_values['declarations']['padding-right'] ) ) {
$padding_right = $block_spacing_values['declarations']['padding-right'];
$style .= "$selector > .alignfull { margin-right:calc($padding_right * -1); }";
}
if ( isset( $block_spacing_values['declarations']['padding-left'] ) ) {
$padding_left = $block_spacing_values['declarations']['padding-left'];
$style .= "$selector > .alignfull { margin-left: calc($padding_left * -1); }";
}
}

Expand Down Expand Up @@ -181,19 +181,26 @@ function gutenberg_render_layout_support_flag( $block_content, $block ) {
$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;
$class_names = array();
$layout_definitions = _wp_array_get( $global_layout_settings, array( 'definitions' ), array() );
$block_classname = wp_get_block_default_classname( $block['blockName'] );
$container_class = wp_unique_id( 'wp-container-' );
$layout_classname = '';
$use_global_padding = gutenberg_get_global_settings( array( 'useRootPaddingAwareAlignments' ) ) && ( isset( $used_layout['inherit'] ) && $used_layout['inherit'] || isset( $used_layout['contentSize'] ) && $used_layout['contentSize'] );

if ( isset( $used_layout['inherit'] ) && $used_layout['inherit'] ) {
if ( ! $global_layout_settings ) {
return $block_content;
}
$used_layout = $global_layout_settings;
Copy link
Contributor

Choose a reason for hiding this comment

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

For some reason I really like this line removal. To me it makes the whole behaviour in this PR feel like the layout support is a bit more logical and less magical, so it's nice we can remove this! 👍

}

$class_names = array();
$layout_definitions = _wp_array_get( $global_layout_settings, array( 'definitions' ), array() );
$block_classname = wp_get_block_default_classname( $block['blockName'] );
$container_class = wp_unique_id( 'wp-container-' );
$layout_classname = '';
$use_global_padding = gutenberg_get_global_settings( array( 'useRootPaddingAwareAlignments' ) ) && ( isset( $used_layout['inherit'] ) && $used_layout['inherit'] || isset( $used_layout['contentSize'] ) && $used_layout['contentSize'] );
if ( isset( $global_layout_settings['contentSize'] ) && $global_layout_settings['contentSize'] ) {
$class_names[] = 'has-global-content-size';
}

if ( isset( $global_layout_settings['wideSize'] ) && $global_layout_settings['wideSize'] ) {
$class_names[] = 'has-global-wide-size';
}
}

if ( $use_global_padding ) {
$class_names[] = 'has-global-padding';
Expand Down
33 changes: 23 additions & 10 deletions lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -736,16 +736,29 @@ function( $pseudo_selector ) use ( $selector ) {
}
}

/*
* Reset default browser margin on the root body element.
* This is set on the root selector **before** generating the ruleset
* from the `theme.json`. This is to ensure that if the `theme.json` declares
* `margin` in its `spacing` declaration for the `body` element then these
* user-generated values take precedence in the CSS cascade.
* @link https://github.com/WordPress/gutenberg/issues/36147.
*/
if ( static::ROOT_BLOCK_SELECTOR === $selector ) {
$block_rules .= 'body { margin: 0; }';
/*
* Reset default browser margin on the root body element.
* This is set on the root selector **before** generating the ruleset
* from the `theme.json`. This is to ensure that if the `theme.json` declares
* `margin` in its `spacing` declaration for the `body` element then these
* user-generated values take precedence in the CSS cascade.
* @link https://github.com/WordPress/gutenberg/issues/36147.
*/
$block_rules .= 'body { margin: 0;';

/*
* If there are content and wide widths in theme.json, output them
* as custom properties on the body element so all blocks can use them.
*/
if ( isset( $settings['layout']['contentSize'] ) && $settings['layout']['contentSize'] ) {
$block_rules .= '--wp--style--global--content-size: ' . $settings['layout']['contentSize'] . ';';
}
if ( isset( $settings['layout']['wideSize'] ) && $settings['layout']['wideSize'] ) {
$block_rules .= '--wp--style--global--wide-size: ' . $settings['layout']['wideSize'] . ';';
}

$block_rules .= '}';
}

// 2. Generate and append the rules that use the general selector.
Expand Down Expand Up @@ -1264,7 +1277,7 @@ protected function get_layout_styles( $block_metadata ) {
$has_fallback_gap_support = ! $has_block_gap_support; // This setting isn't useful yet: it exists as a placeholder for a future explicit fallback gap styles support.
$node = _wp_array_get( $this->theme_json, $block_metadata['path'], array() );
$layout_definitions = _wp_array_get( $this->theme_json, array( 'settings', 'layout', 'definitions' ), array() );
$layout_selector_pattern = '/^[a-zA-Z0-9\-\.\ *+>]*$/'; // Allow alphanumeric classnames, spaces, wildcard, sibling, and child combinator selectors.
$layout_selector_pattern = '/^[a-zA-Z0-9\-\.\ *+>:\(\)]*$/'; // Allow alphanumeric classnames, spaces, wildcard, sibling, child combinator and pseudo class selectors.

// Gap styles will only be output if the theme has block gap support, or supports a fallback gap.
// Default layout gap styles will be skipped for themes that do not explicitly opt-in to blockGap with a `true` or `false` value.
Expand Down
14 changes: 14 additions & 0 deletions lib/compat/wordpress-6.1/theme.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,20 @@
"margin-left": "auto !important",
"margin-right": "auto !important"
}
},
{
"selector": ":where(.has-global-content-size) > :where(:not(.alignleft):not(.alignright):not(.alignfull))",
"rules": {
"max-width": "var(--wp--style--global--content-size)",
"margin-left": "auto !important",
"margin-right": "auto !important"
}
},
{
"selector": ":where(.has-global-wide-size) > .alignwide",
"rules": {
"max-width": "var(--wp--style--global--wide-size)"
}
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 wondering if we can use the same classname for these rules, since they have different child selectors.

Would it work to have a single has-global-content-sizes classname, so the second rule would become: :where(.has-global-content-sizes) > .alignwide?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we should be able to do that. Relatedly, I'm thinking that if there's only one of these sizes set, we should set the other to the same value. We already use this logic in layout.php so makes sense to use it here too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, can't seem to get fallback values to work for the variables defined in theme.json. I wonder if we're disallowing commas in the values or something?

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 suspect safecss_filter_attr has a problem with the commas, although its docs say it supports var. I'll have a look on Trac and see if there's any ticket about that.

In the meantime, I made the fallback work by always setting values for both variables to whichever value exists.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, always setting both values looks like a good way to handle the fallback for now!

}
],
"spacingStyles": [
Expand Down
6 changes: 5 additions & 1 deletion packages/block-editor/src/hooks/layout.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ function useLayoutClasses( layout, layoutDefinitions ) {
layoutDefinitions?.[ layout?.type || 'default' ]?.className
);
}
if ( layout?.inherit ) {
layoutClassnames.push( 'has-global-content-size' );
layoutClassnames.push( 'has-global-wide-size' );
}

if ( ( layout?.inherit || layout?.contentSize ) && rootPaddingAlignment ) {
layoutClassnames.push( 'has-global-padding' );
Expand Down Expand Up @@ -275,7 +279,7 @@ export const withLayoutStyles = createHigherOrderComponent(
const { default: defaultBlockLayout } =
getBlockSupport( name, layoutBlockSupportKey ) || {};
const usedLayout = layout?.inherit
? defaultThemeLayout
? { ...defaultThemeLayout, ...layout }
: layout || defaultBlockLayout || {};
const layoutClasses = shouldRenderLayoutStyles
? useLayoutClasses( usedLayout, defaultThemeLayout?.definitions )
Expand Down
7 changes: 2 additions & 5 deletions packages/block-editor/src/layouts/flow.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export default {
hasBlockGapSupport,
layoutDefinitions,
} ) {
const { contentSize, wideSize } = layout;
const { contentSize, wideSize, inherit = false } = layout;
const blockGapStyleValue = getGapBoxControlValueFromStyle(
style?.spacing?.blockGap
);
Expand All @@ -129,7 +129,7 @@ export default {
: '';

let output =
!! contentSize || !! wideSize
( !! contentSize || !! wideSize ) && ! inherit
? `
${ appendSelectors(
selector,
Expand All @@ -142,9 +142,6 @@ export default {
${ appendSelectors( selector, '> .alignwide' ) } {
max-width: ${ wideSize ?? contentSize };
}
${ appendSelectors( selector, '> .alignfull' ) } {
max-width: none;
}
`
: '';

Expand Down