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 all commits
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
57 changes: 30 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,22 @@ 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'] || isset( $global_layout_settings['wideSize'] ) && $global_layout_settings['wideSize'] ) {
$class_names[] = 'has-global-content-size';
}
}

if ( $use_global_padding ) {
$class_names[] = 'has-global-padding';
Expand Down
35 changes: 25 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,31 @@ 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'] || isset( $settings['layout']['wideSize'] ) && $settings['layout']['wideSize'] ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just another idle thought from working on #42544, would it be worth moving this block to the bottom of the get_layout_styles function within the static::ROOT_BLOCK_SELECTOR === $selector if statement? Mostly, I was wondering if a theme disables layout styles, should these CSS variable declarations also no longer be output?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly, I was wondering if a theme disables layout styles, should these CSS variable declarations also no longer be output?

Yes, that makes total sense!

I've transferred most of this logic to #42763 where I'm trying to solve the problem via a new layout type. It's working neatly so far because it makes the extra classname/selector unnecessary. If we decide not to go forward with that approach, I'll go on with this one, otherwise it's easier to apply this feedback directly on the other one 😄

$content_size = isset( $settings['layout']['contentSize'] ) ? $settings['layout']['contentSize'] : $settings['layout']['wideSize'];
$content_size = static::is_safe_css_declaration( 'max-width', $content_size ) ? $content_size : 'initial';
$wide_size = isset( $settings['layout']['wideSize'] ) ? $settings['layout']['wideSize'] : $settings['layout']['contentSize'];
$wide_size = static::is_safe_css_declaration( 'max-width', $wide_size ) ? $wide_size : 'initial';
$block_rules .= '--wp--style--global--content-size: ' . $content_size . ';';
$block_rules .= '--wp--style--global--wide-size: ' . $wide_size . ';';
Comment on lines +759 to +760
Copy link
Contributor

Choose a reason for hiding this comment

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

Since theme.json settings aren't run through remove_insecure_styles should we add a static::is_safe_css_declaration check before outputting here? I think we'd be able to call it with max-width as the first param, and $content_size or $wide_size as the second param?

}

$block_rules .= '}';
}

// 2. Generate and append the rules that use the general selector.
Expand Down Expand Up @@ -1264,7 +1279,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-content-size) > .alignwide",
"rules": {
"max-width": "var(--wp--style--global--wide-size)"
}
}
],
"spacingStyles": [
Expand Down
5 changes: 4 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,9 @@ function useLayoutClasses( layout, layoutDefinitions ) {
layoutDefinitions?.[ layout?.type || 'default' ]?.className
);
}
if ( layout?.inherit ) {
layoutClassnames.push( 'has-global-content-size' );
}

if ( ( layout?.inherit || layout?.contentSize ) && rootPaddingAlignment ) {
layoutClassnames.push( 'has-global-padding' );
Expand Down Expand Up @@ -275,7 +278,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