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

Move layout custom properties to filter. #37438

Closed
wants to merge 1 commit into from
Closed

Move layout custom properties to filter. #37438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 1 addition & 20 deletions lib/block-supports/layout.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,45 +96,26 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support
$style .= "flex-wrap: $flex_wrap;";
if ( 'horizontal' === $layout_orientation ) {
$style .= 'align-items: center;';
if ( ! empty( $layout['setCascadingProperties'] ) && $layout['setCascadingProperties'] ) {
$style .= '--layout-direction: row;';
}
/**
* Add this style only if is not empty for backwards compatibility,
* since we intend to convert blocks that had flex layout implemented
* by custom css.
*/
if ( ! empty( $layout['justifyContent'] ) && array_key_exists( $layout['justifyContent'], $justify_content_options ) ) {
$style .= "justify-content: {$justify_content_options[ $layout['justifyContent'] ]};";
if ( ! empty( $layout['setCascadingProperties'] ) && $layout['setCascadingProperties'] ) {
// --layout-justification-setting allows children to inherit the value regardless or row or column direction.
$style .= "--layout-justification-setting: {$justify_content_options[ $layout['justifyContent'] ]};";
$style .= "--layout-wrap: $flex_wrap;";
$style .= "--layout-justify: {$justify_content_options[ $layout['justifyContent'] ]};";
$style .= '--layout-align: center;';
}
}
} else {
$style .= 'flex-direction: column;';
if ( ! empty( $layout['setCascadingProperties'] ) && $layout['setCascadingProperties'] ) {
$style .= '--layout-direction: column;';
}
if ( ! empty( $layout['justifyContent'] ) && array_key_exists( $layout['justifyContent'], $justify_content_options ) ) {
$style .= "align-items: {$justify_content_options[ $layout['justifyContent'] ]};";
if ( ! empty( $layout['setCascadingProperties'] ) && $layout['setCascadingProperties'] ) {
// --layout-justification-setting allows children to inherit the value regardless or row or column direction.
$style .= "--layout-justification-setting: {$justify_content_options[ $layout['justifyContent'] ]};";
$style .= '--layout-justify: initial;';
$style .= "--layout-align: {$justify_content_options[ $layout['justifyContent'] ]};";
}
}
}
$style .= '}';

$style .= "$selector > * { margin: 0; }";
}

return $style;
return apply_filters( 'layout_styles', $style, $selector, $layout );
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need extra filters (aka new APIs to support)? Can't we rely on render_block for the backend and editor.BlockListBlock for the frontend?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also potentially we don't need any filter, we can just add the logic to the "render_callback" of the navigation block in the backend and to "edit" function of the navigation block in the frontend?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

render_block and editor.BlockListBlock won't work because we can't get the layout properties from them. Would it help to name the filter __unstable while we figure out what works best in terms of API? Or is that naming convention not applicable to filters?

Also potentially we don't need any filter, we can just add the logic to the "render_callback" of the navigation block in the backend and to "edit" function of the navigation block in the frontend?

We could do something along these lines, but if we want to keep all the logic in the Nav block it might be easier (and cleaner) to just output all the items-justified- etc. classnames and hook the CSS off those.

}

/**
Expand Down
68 changes: 28 additions & 40 deletions packages/block-editor/src/layouts/flex.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
arrowDown,
} from '@wordpress/icons';
import { Button, ToggleControl, Flex, FlexItem } from '@wordpress/components';
import { applyFilters } from '@wordpress/hooks';

/**
* Internal dependencies
Expand Down Expand Up @@ -85,10 +86,7 @@ export default {
);
},
save: function FlexLayoutStyle( { selector, layout } ) {
const {
orientation = 'horizontal',
setCascadingProperties = false,
} = layout;
const { orientation = 'horizontal' } = layout;
const blockGapSupport = useSetting( 'spacing.blockGap' );
const hasBlockGapStylesSupport = blockGapSupport !== null;
const justifyContent =
Expand All @@ -97,53 +95,43 @@ export default {
const flexWrap = flexWrapOptions.includes( layout.flexWrap )
? layout.flexWrap
: 'wrap';
let rowOrientation = `
const rowOrientation = `
flex-direction: row;
align-items: center;
justify-content: ${ justifyContent };
`;
if ( setCascadingProperties ) {
// --layout-justification-setting allows children to inherit the value
// regardless or row or column direction.
rowOrientation += `
--layout-justification-setting: ${ justifyContent };
--layout-direction: row;
--layout-wrap: ${ flexWrap };
--layout-justify: ${ justifyContent };
--layout-align: center;
`;
}
const alignItems =
alignItemsMap[ layout.justifyContent ] || alignItemsMap.left;
let columnOrientation = `
const columnOrientation = `
flex-direction: column;
align-items: ${ alignItems };
`;
if ( setCascadingProperties ) {
columnOrientation += `
--layout-justification-setting: ${ alignItems };
--layout-direction: column;
--layout-justify: initial;
--layout-align: ${ alignItems };
`;
const styleContent = `
${ appendSelectors( selector ) } {
display: flex;
gap: ${
hasBlockGapStylesSupport
? 'var( --wp--style--block-gap, 0.5em )'
: '0.5em'
};
flex-wrap: ${ flexWrap };
${ orientation === 'horizontal' ? rowOrientation : columnOrientation }
}
return (
<style>{ `
${ appendSelectors( selector ) } {
display: flex;
gap: ${
hasBlockGapStylesSupport
? 'var( --wp--style--block-gap, 0.5em )'
: '0.5em'
};
flex-wrap: ${ flexWrap };
${ orientation === 'horizontal' ? rowOrientation : columnOrientation }
}

${ appendSelectors( selector, '> *' ) } {
margin: 0;
}
` }</style>
${ appendSelectors( selector, '> *' ) } {
margin: 0;
}
`;
return (
<style>
{ applyFilters(
'blockEditor.FlexLayoutStyle',
styleContent,
selector,
appendSelectors,
layout
) }
</style>
);
},
getOrientation( layout ) {
Expand Down
3 changes: 1 addition & 2 deletions packages/block-library/src/navigation/block.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@
"allowSwitching": false,
"allowInheriting": false,
"default": {
"type": "flex",
"setCascadingProperties": true
"type": "flex"
}
}
},
Expand Down
2 changes: 0 additions & 2 deletions packages/block-library/src/navigation/deprecated.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ const migrateWithLayout = ( attributes ) => {
Object.assign( updatedAttributes, {
layout: {
type: 'flex',
setCascadingProperties: 'true',
...( itemsJustification && {
justifyContent: itemsJustification,
} ),
Expand Down Expand Up @@ -135,7 +134,6 @@ const v6 = {
allowInheriting: false,
default: {
type: 'flex',
setCascadingProperties: true,
},
},
},
Expand Down
56 changes: 56 additions & 0 deletions packages/block-library/src/navigation/hooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Used with the default, horizontal flex orientation.
const justifyContentMap = {
left: 'flex-start',
right: 'flex-end',
center: 'center',
'space-between': 'space-between',
};

// Used with the vertical (column) flex orientation.
const alignItemsMap = {
left: 'flex-start',
right: 'flex-end',
center: 'center',
};

export function addStylesToLayout(
styleContent,
selector,
appendSelectors,
layout
) {
if ( ! layout ) {
return styleContent;
}

const { orientation = 'horizontal' } = layout;
const justifyContent =
justifyContentMap[ layout.justifyContent ] || justifyContentMap.left;
const alignItems =
alignItemsMap[ layout.justifyContent ] || alignItemsMap.left;

let customProperties = ``;

if ( orientation === 'horizontal' ) {
// --layout-justification-setting allows children to inherit the value
// regardless or row or column direction.
customProperties += `
--layout-justification-setting: ${ justifyContent };
--layout-direction: row;
--layout-wrap: ${ layout.flexWrap || 'wrap' };
--layout-justify: ${ justifyContent };
--layout-align: center;
`;
} else {
customProperties += `
--layout-justification-setting: ${ alignItems };
--layout-direction: column;
--layout-justify: initial;
--layout-align: ${ alignItems };
`;
}

return `${ styleContent } ${ appendSelectors( selector ) } {
${ customProperties }
}`;
}
9 changes: 9 additions & 0 deletions packages/block-library/src/navigation/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
import { __ } from '@wordpress/i18n';
import { navigation as icon } from '@wordpress/icons';
import { addFilter } from '@wordpress/hooks';

/**
* Internal dependencies
Expand All @@ -11,6 +12,7 @@ import metadata from './block.json';
import edit from './edit';
import save from './save';
import deprecated from './deprecated';
import { addStylesToLayout } from './hooks';

const { name } = metadata;

Expand Down Expand Up @@ -50,3 +52,10 @@ export const settings = {
save,
deprecated,
};

// Importing this file includes side effects. This is whitelisted in block-library/package.json under sideEffects
addFilter(
'blockEditor.FlexLayoutStyle',
'core/navigation',
addStylesToLayout
);
45 changes: 45 additions & 0 deletions packages/block-library/src/navigation/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,3 +605,48 @@ function block_core_navigation_typographic_presets_backcompatibility( $parsed_bl
}

add_filter( 'render_block_data', 'block_core_navigation_typographic_presets_backcompatibility' );

function block_core_navigation_add_styles_to_layout( $style, $selector, $layout ) {
if ( ! isset( $layout['type'] ) || 'flex' !== $layout['type'] ) {
return $style;
}

$custom_properties = '';
$layout_orientation = isset( $layout['orientation'] ) ? $layout['orientation'] : 'horizontal';
$justify_content_options = array(
'left' => 'flex-start',
'right' => 'flex-end',
'center' => 'center',
);
$flex_wrap_options = array( 'wrap', 'nowrap' );
$flex_wrap = ! empty( $layout['flexWrap'] ) && in_array( $layout['flexWrap'], $flex_wrap_options, true ) ?
$layout['flexWrap'] :
'wrap';

if ( 'horizontal' === $layout_orientation ) {
$custom_properties .= '--layout-direction: row;';

if ( ! empty( $layout['justifyContent'] ) && array_key_exists( $layout['justifyContent'], $justify_content_options ) ) {
// --layout-justification-setting allows children to inherit the value regardless or row or column direction.
$custom_properties .= "--layout-justification-setting: {$justify_content_options[ $layout['justifyContent'] ]};";
$custom_properties .= "--layout-wrap: $flex_wrap;";
$custom_properties .= "--layout-justify: {$justify_content_options[ $layout['justifyContent'] ]};";
$custom_properties .= '--layout-align: center;';
}
} else {
$custom_properties .= '--layout-direction: column;';

if ( ! empty( $layout['justifyContent'] ) && array_key_exists( $layout['justifyContent'], $justify_content_options ) ) {
// --layout-justification-setting allows children to inherit the value regardless or row or column direction.
$custom_properties .= "--layout-justification-setting: {$justify_content_options[ $layout['justifyContent'] ]};";
$custom_properties .= '--layout-justify: initial;';
$custom_properties .= "--layout-align: {$justify_content_options[ $layout['justifyContent'] ]};";
}
}

$style .= "$selector { $custom_properties }";

return $style;
}

add_filter( 'layout_styles', 'block_core_navigation_add_styles_to_layout', 10, 3 );
1 change: 0 additions & 1 deletion test/integration/fixtures/blocks/core__navigation__deprecated-1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
"overlayMenu": "mobile",
"layout": {
"type": "flex",
"setCascadingProperties": "true",
"orientation": "horizontal"
}
},
Expand Down
2 changes: 1 addition & 1 deletion test/integration/fixtures/blocks/core__navigation__deprecated-1.serialized.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<!-- wp:navigation {"layout":{"type":"flex","setCascadingProperties":"true","orientation":"horizontal"}} -->
<!-- wp:navigation {"layout":{"type":"flex","orientation":"horizontal"}} -->
<!-- wp:navigation-link {"label":"WordPress","url":"https://www.wordpress.org/"} /-->
<!-- /wp:navigation -->
1 change: 0 additions & 1 deletion test/integration/fixtures/blocks/core__navigation__deprecated-v4.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"fontFamily": "cambria-georgia",
"layout": {
"type": "flex",
"setCascadingProperties": "true",
"orientation": "horizontal"
}
},
Expand Down
2 changes: 1 addition & 1 deletion test/integration/fixtures/blocks/core__navigation__deprecated-v4.serialized.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<!-- wp:navigation {"overlayMenu":"never","fontFamily":"cambria-georgia","layout":{"type":"flex","setCascadingProperties":"true","orientation":"horizontal"}} /-->
<!-- wp:navigation {"overlayMenu":"never","fontFamily":"cambria-georgia","layout":{"type":"flex","orientation":"horizontal"}} /-->
1 change: 0 additions & 1 deletion test/integration/fixtures/blocks/core__navigation__deprecated-v5.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
"overlayMenu": "never",
"layout": {
"type": "flex",
"setCascadingProperties": "true",
"justifyContent": "center",
"orientation": "vertical"
}
Expand Down
2 changes: 1 addition & 1 deletion test/integration/fixtures/blocks/core__navigation__deprecated-v5.serialized.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<!-- wp:navigation {"overlayMenu":"never","layout":{"type":"flex","setCascadingProperties":"true","justifyContent":"center","orientation":"vertical"}} /-->
<!-- wp:navigation {"overlayMenu":"never","layout":{"type":"flex","justifyContent":"center","orientation":"vertical"}} /-->
1 change: 0 additions & 1 deletion test/integration/fixtures/blocks/core__navigation__deprecated.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
},
"layout": {
"type": "flex",
"setCascadingProperties": "true",
"orientation": "horizontal"
}
},
Expand Down
2 changes: 1 addition & 1 deletion test/integration/fixtures/blocks/core__navigation__deprecated.serialized.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<!-- wp:navigation {"style":{"typography":{"textTransform":"lowercase","textDecoration":"line-through","fontStyle":"italic","fontWeight":"100"}},"layout":{"type":"flex","setCascadingProperties":"true","orientation":"horizontal"}} -->
<!-- wp:navigation {"style":{"typography":{"textTransform":"lowercase","textDecoration":"line-through","fontStyle":"italic","fontWeight":"100"}},"layout":{"type":"flex","orientation":"horizontal"}} -->
<!-- wp:navigation-link {"label":"WordPress","url":"https://www.wordpress.org/"} /-->
<!-- /wp:navigation -->