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

Try using flex layout on the Navigation block #36169

Merged
merged 6 commits into from
Nov 5, 2021
Merged

Try using flex layout on the Navigation block #36169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7f0d935
Try using flex layout on the Navigation block
tellthemachines Nov 3, 2021
3f6120e
Fix justification for overlay menu
tellthemachines Nov 4, 2021
0f2832c
Add deprecation and update fixtures
tellthemachines Nov 4, 2021
4d82968
Fix menu position in site editor.
tellthemachines Nov 5, 2021
0f2e9a1
Don't use editor variables in front end styles.
tellthemachines Nov 5, 2021
e387c62
Move overlay styles back to inner container.
tellthemachines Nov 5, 2021
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
3 changes: 3 additions & 0 deletions lib/block-supports/layout.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,14 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support
*/
if ( ! empty( $layout['justifyContent'] ) && array_key_exists( $layout['justifyContent'], $justify_content_options ) ) {
$style .= "justify-content: {$justify_content_options[ $layout['justifyContent'] ]};";
// --justification-setting allows children to inherit the value regardless or row or column direction.
$style .= "--justification-setting: {$justify_content_options[ $layout['justifyContent'] ]};";
}
} else {
$style .= 'flex-direction: column;';
if ( ! empty( $layout['justifyContent'] ) && array_key_exists( $layout['justifyContent'], $justify_content_options ) ) {
$style .= "align-items: {$justify_content_options[ $layout['justifyContent'] ]};";
$style .= "--justification-setting: {$justify_content_options[ $layout['justifyContent'] ]};";
}
}
$style .= '}';
Expand Down
4 changes: 4 additions & 0 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 @@ -94,16 +94,20 @@ export default {
const flexWrap = flexWrapOptions.includes( layout.flexWrap )
? layout.flexWrap
: 'wrap';
// --justification-setting allows children to inherit the value
// regardless or row or column direction.
const rowOrientation = `
flex-direction: row;
align-items: center;
justify-content: ${ justifyContent };
--justification-setting: ${ justifyContent };
`;
const alignItems =
alignItemsMap[ layout.justifyContent ] || alignItemsMap.left;
const columnOrientation = `
flex-direction: column;
align-items: ${ alignItems };
--justification-setting: ${ alignItems };
`;
return (
<style>{ `
Expand Down
14 changes: 7 additions & 7 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 @@ -14,10 +14,6 @@
"navigationMenuId": {
"type": "number"
},
"orientation": {
"type": "string",
"default": "horizontal"
},
"textColor": {
"type": "string"
},
Expand All @@ -36,9 +32,6 @@
"rgbBackgroundColor": {
"type": "string"
},
"itemsJustification": {
"type": "string"
},
"showSubmenuIcon": {
"type": "boolean",
"default": true
Expand Down Expand Up @@ -116,6 +109,13 @@
"__experimentalDefaultControls": {
"blockGap": true
}
},
"__experimentalLayout": {
"allowSwitching": false,
"allowInheriting": false,
"default": {
"type": "flex"
}
}
},
"viewScript": "file:./view.min.js",
Expand Down
132 changes: 129 additions & 3 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 @@ -21,6 +21,123 @@ const TYPOGRAPHY_PRESET_DEPRECATION_MAP = {
textTransform: 'var:preset|text-transform|',
};

const migrateWithLayout = ( attributes ) => {
if ( !! attributes.layout ) {
return attributes;
}

const { itemsJustification, orientation } = attributes;

const updatedAttributes = {
...attributes,
};

if ( itemsJustification || orientation ) {
Object.assign( updatedAttributes, {
layout: {
type: 'flex',
justifyContent: itemsJustification || 'left',
orientation: orientation || 'horizontal',
},
} );
delete updatedAttributes.itemsJustification;
delete updatedAttributes.orientation;
}

return updatedAttributes;
};

const v5 = {
attributes: {
navigationMenuId: {
type: 'number',
},
orientation: {
type: 'string',
default: 'horizontal',
},
textColor: {
type: 'string',
},
customTextColor: {
type: 'string',
},
rgbTextColor: {
type: 'string',
},
backgroundColor: {
type: 'string',
},
customBackgroundColor: {
type: 'string',
},
rgbBackgroundColor: {
type: 'string',
},
itemsJustification: {
type: 'string',
},
showSubmenuIcon: {
type: 'boolean',
default: true,
},
openSubmenusOnClick: {
type: 'boolean',
default: false,
},
overlayMenu: {
type: 'string',
default: 'never',
},
__unstableLocation: {
type: 'string',
},
overlayBackgroundColor: {
type: 'string',
},
customOverlayBackgroundColor: {
type: 'string',
},
overlayTextColor: {
type: 'string',
},
customOverlayTextColor: {
type: 'string',
},
},
supports: {
align: [ 'wide', 'full' ],
anchor: true,
html: false,
inserter: true,
typography: {
fontSize: true,
lineHeight: true,
__experimentalFontStyle: true,
__experimentalFontWeight: true,
__experimentalTextTransform: true,
__experimentalFontFamily: true,
__experimentalTextDecoration: true,
__experimentalDefaultControls: {
fontSize: true,
},
},
spacing: {
blockGap: true,
units: [ 'px', 'em', 'rem', 'vh', 'vw' ],
__experimentalDefaultControls: {
blockGap: true,
},
},
},
save() {
return <InnerBlocks.Content />;
},
isEligible: ( { itemsJustification, orientation } ) =>
!! itemsJustification || !! orientation,
migrate: migrateWithLayout,
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 cautious about this because of the discussion here - #35663.

I'm yet to write a migration and don't really fully understand them. A question I'd have is why v4 needs migrateFontFamily, but v5 doesn't? The definition of the two deprecations for font family looks identical.

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 more or less followed the logic used in #35819, as I realised in that PR that if the legacy justification mechanism isn't migrated, it's impossible to justify an old block using the new layout controls, because they're always overridden by the legacy styles. This may not apply exactly to nav as it's still experimental, but I added it in to be on the safe side.

I think this is something we can safely look at later though.

};

const v4 = {
attributes: {
orientation: {
Expand Down Expand Up @@ -101,7 +218,7 @@ const v4 = {
save() {
return <InnerBlocks.Content />;
},
migrate: migrateFontFamily,
migrate: compose( migrateWithLayout, migrateFontFamily ),
isEligible( { style } ) {
return style?.typography?.fontFamily;
},
Expand Down Expand Up @@ -142,6 +259,7 @@ const migrateTypographyPresets = function ( attributes ) {
};

const deprecated = [
v5,
v4,
// Remove `isResponsive` attribute.
{
Expand Down Expand Up @@ -217,7 +335,11 @@ const deprecated = [
isEligible( attributes ) {
return attributes.isResponsive;
},
migrate: compose( migrateFontFamily, migrateIsResponsive ),
migrate: compose(
migrateWithLayout,
migrateFontFamily,
migrateIsResponsive
),
save() {
return <InnerBlocks.Content />;
},
Expand Down Expand Up @@ -287,7 +409,11 @@ const deprecated = [
}
return false;
},
migrate: compose( migrateFontFamily, migrateTypographyPresets ),
migrate: compose(
migrateWithLayout,
migrateFontFamily,
migrateTypographyPresets
),
},
{
attributes: {
Expand Down
20 changes: 0 additions & 20 deletions packages/block-library/src/navigation/edit/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
} from '@wordpress/element';
import {
InspectorControls,
JustifyToolbar,
BlockControls,
useBlockProps,
__experimentalUseNoRecursiveRenders as useNoRecursiveRenders,
Expand Down Expand Up @@ -99,7 +98,6 @@ function Navigation( {
// These props are used by the navigation editor to override specific
// navigation block settings.
hasSubmenuIndicatorSetting = true,
hasItemJustificationControls = true,
hasColorSettings = true,
customPlaceholder: CustomPlaceholder = null,
customAppender: CustomAppender = null,
Expand Down Expand Up @@ -300,11 +298,6 @@ function Navigation( {
? CustomPlaceholder
: Placeholder;

const justifyAllowedControls =
orientation === 'vertical'
? [ 'left', 'center', 'right' ]
: [ 'left', 'center', 'right', 'space-between' ];

return (
<EntityProvider
kind="postType"
Expand All @@ -331,19 +324,6 @@ function Navigation( {
</ToolbarDropdownMenu>
</ToolbarGroup>
) }
{ hasItemJustificationControls && (
<JustifyToolbar
value={ itemsJustification }
allowedControls={ justifyAllowedControls }
onChange={ ( value ) =>
setAttributes( { itemsJustification: value } )
}
popoverProps={ {
position: 'bottom right',
isAlternate: true,
} }
/>
) }
<ToolbarGroup>{ listViewToolbarButton }</ToolbarGroup>
{ isDraftNavigationMenu && (
<ToolbarGroup>
Expand Down
2 changes: 0 additions & 2 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 @@ -11,15 +11,13 @@ import metadata from './block.json';
import edit from './edit';
import save from './save';
import deprecated from './deprecated';
import variations from './variations';

const { name } = metadata;

export { metadata, name };

export const settings = {
icon,
variations,
example: {
innerBlocks: [
{
Expand Down
2 changes: 0 additions & 2 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 @@ -295,8 +295,6 @@ function( $block ) {
$classes = array_merge(
$colors['css_classes'],
$font_sizes['css_classes'],
( isset( $attributes['orientation'] ) && 'vertical' === $attributes['orientation'] ) ? array( 'is-vertical' ) : array(),
isset( $attributes['itemsJustification'] ) ? array( 'items-justified-' . $attributes['itemsJustification'] ) : array(),
$is_responsive_menu ? array( 'is-responsive' ) : array()
);

Expand Down
Loading