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

Always pass layout to inner blocks #47477

Merged
merged 7 commits into from
Jan 31, 2023
Merged

Always pass layout to inner blocks #47477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6bd0112
Always pass layout to inner blocks
tellthemachines Jan 27, 2023
b5cc279
Fix migration eligibility rule.
tellthemachines Jan 30, 2023
f301327
Use stable fallback object.
tellthemachines Jan 30, 2023
7dde489
Add deprecation fixture.
tellthemachines Jan 30, 2023
ebe258d
Use existing EMPTY_OBJECT pattern
tellthemachines Jan 30, 2023
f1acfb5
Check if block has layout support before adding layout to context.
tellthemachines Jan 31, 2023
e28c0d7
Simplify deprecation fixture markup.
tellthemachines Jan 31, 2023
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
16 changes: 15 additions & 1 deletion packages/block-editor/src/components/block-edit/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
import { useMemo } from '@wordpress/element';

import { hasBlockSupport } from '@wordpress/blocks';
/**
* Internal dependencies
*/
Expand All @@ -20,11 +21,24 @@ import { BlockEditContextProvider, useBlockEditContext } from './context';
export { useBlockEditContext };

export default function BlockEdit( props ) {
const { name, isSelected, clientId, __unstableLayoutClassNames } = props;
const {
name,
isSelected,
clientId,
attributes = {},
__unstableLayoutClassNames,
} = props;
const { layout = null } = attributes;
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be super edge-casey, but I was wondering if we should check whether or not the block has opted-in to the layout support before passing down its layout attribute in the context? Because this will apply to all blocks, if a 3rd party block happens to use an attribute also named layout, then it will be passed down here, too, without a check for the layout block support.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, let's do that!

const layoutSupport = hasBlockSupport(
name,
'__experimentalLayout',
false
);
const context = {
name,
isSelected,
clientId,
layout: layoutSupport ? layout : null,
__unstableLayoutClassNames,
};
return (
Expand Down
35 changes: 25 additions & 10 deletions packages/block-editor/src/components/inner-blocks/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ import { useBlockEditContext } from '../block-edit/context';
import useBlockSync from '../provider/use-block-sync';
import { store as blockEditorStore } from '../../store';
import useBlockDropZone from '../use-block-drop-zone';
import useSetting from '../use-setting';

const EMPTY_OBJECT = {};

/**
* InnerBlocks is a component which allows a single block to have multiple blocks
Expand All @@ -53,7 +56,7 @@ function UncontrolledInnerBlocks( props ) {
renderAppender,
orientation,
placeholder,
__experimentalLayout,
layout,
} = props;

useNestedSettingsUpdate(
Expand All @@ -64,7 +67,7 @@ function UncontrolledInnerBlocks( props ) {
templateLock,
captureToolbars,
orientation,
__experimentalLayout
layout
);

useInnerBlockTemplateSync(
Expand All @@ -82,17 +85,25 @@ function UncontrolledInnerBlocks( props ) {
[ clientId ]
);

const { allowSizingOnChildren = false } =
getBlockSupport( name, '__experimentalLayout' ) || {};
const defaultLayoutBlockSupport =
getBlockSupport( name, '__experimentalLayout' ) || EMPTY_OBJECT;

const { allowSizingOnChildren = false } = defaultLayoutBlockSupport;

const defaultLayout = useSetting( 'layout' ) || EMPTY_OBJECT;

const layout = useMemo(
const usedLayout = layout || defaultLayoutBlockSupport;

const memoedLayout = useMemo(
() => ( {
...__experimentalLayout,
// Default layout will know about any content/wide size defined by the theme.
...defaultLayout,
andrewserong marked this conversation as resolved.
Show resolved Hide resolved
...usedLayout,
...( allowSizingOnChildren && {
allowSizingOnChildren: true,
} ),
} ),
[ __experimentalLayout, allowSizingOnChildren ]
[ defaultLayout, usedLayout, allowSizingOnChildren ]
);

// This component needs to always be synchronous as it's the one changing
Expand All @@ -103,7 +114,7 @@ function UncontrolledInnerBlocks( props ) {
rootClientId={ clientId }
renderAppender={ renderAppender }
__experimentalAppenderTagName={ __experimentalAppenderTagName }
__experimentalLayout={ layout }
__experimentalLayout={ memoedLayout }
wrapperRef={ wrapperRef }
placeholder={ placeholder }
/>
Expand Down Expand Up @@ -152,8 +163,11 @@ const ForwardedInnerBlocks = forwardRef( ( props, ref ) => {
export function useInnerBlocksProps( props = {}, options = {} ) {
const { __unstableDisableLayoutClassNames, __unstableDisableDropZone } =
options;
const { clientId, __unstableLayoutClassNames: layoutClassNames = '' } =
useBlockEditContext();
const {
clientId,
layout = null,
__unstableLayoutClassNames: layoutClassNames = '',
} = useBlockEditContext();
const isSmallScreen = useViewportMatch( 'medium', '<' );
const { __experimentalCaptureToolbars, hasOverlay } = useSelect(
( select ) => {
Expand Down Expand Up @@ -199,6 +213,7 @@ export function useInnerBlocksProps( props = {}, options = {} ) {

const innerBlocksProps = {
__experimentalCaptureToolbars,
layout,
...options,
};
const InnerBlocks =
Expand Down
9 changes: 0 additions & 9 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 @@ -67,22 +67,13 @@ export default {
info: alignmentInfo[ alignment ],
} ) );
}
const { contentSize, wideSize } = layout;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just double-checking — is the idea that this can be removed now because of the layout object being passed down? From testing, the issue of the wide width post template showing up using content size appears to have returned in this PR. I think the flow layout might still need the logic here (or elsewhere), so that the alignwide and alignfull classnames can be injected.

With TwentyTwentyTwo theme applied, it looks like the post template is being rendered as wide width on the site frontend, but content width in the site editor:

Editor (note 650px) Site frontend (note 1000px)
image image

It looks like on the site frontend the post template block's output has both is-layout-flow and alignwide classes as expected, however in the site editor the alignwide class is not present.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That code needs to be removed because it's causing align wide and full controls to always show, even if the parent block doesn't have a content width. The problem is that the layout we're extracting content and wide size from there is the full theme layout, not the block one, so it always includes those properties if they're set by the theme.

The specific problem those lines were fixing should now be fixed by the Query block deprecation. I only checked with the test markup from your PR, but will have a look at TT2 now.


const alignments = [
{ name: 'left' },
{ name: 'center' },
{ name: 'right' },
];

if ( contentSize ) {
alignments.unshift( { name: 'full' } );
}

if ( wideSize ) {
alignments.unshift( { name: 'wide', info: alignmentInfo.wide } );
}

alignments.unshift( { name: 'none', info: alignmentInfo.none } );

return alignments;
Expand Down
4 changes: 2 additions & 2 deletions packages/block-library/src/buttons/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const DEFAULT_BLOCK = {
};

function ButtonsEdit( { attributes, className } ) {
const { fontSize, layout = {}, style } = attributes;
const { fontSize, style } = attributes;
const blockProps = useBlockProps( {
className: classnames( className, {
'has-custom-font-size': fontSize || style?.typography?.fontSize,
Expand All @@ -59,7 +59,7 @@ function ButtonsEdit( { attributes, className } ) {
{ className: preferredStyle && `is-style-${ preferredStyle }` },
],
],
__experimentalLayout: layout,

templateInsertUpdatesSelection: true,
} );

Expand Down
14 changes: 1 addition & 13 deletions packages/block-library/src/comments-pagination/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
Warning,
} from '@wordpress/block-editor';
import { useSelect } from '@wordpress/data';
import { getBlockSupport } from '@wordpress/blocks';
import { PanelBody } from '@wordpress/components';

/**
Expand All @@ -29,21 +28,11 @@ const ALLOWED_BLOCKS = [
'core/comments-pagination-next',
];

const getDefaultBlockLayout = ( blockTypeOrName ) => {
const layoutBlockSupportConfig = getBlockSupport(
blockTypeOrName,
'__experimentalLayout'
);
return layoutBlockSupportConfig?.default;
};

export default function QueryPaginationEdit( {
attributes: { paginationArrow, layout },
attributes: { paginationArrow },
setAttributes,
clientId,
name,
} ) {
const usedLayout = layout || getDefaultBlockLayout( name );
const hasNextPreviousBlocks = useSelect( ( select ) => {
const { getBlocks } = select( blockEditorStore );
const innerBlocks = getBlocks( clientId );
Expand All @@ -64,7 +53,6 @@ export default function QueryPaginationEdit( {
const innerBlocksProps = useInnerBlocksProps( blockProps, {
template: TEMPLATE,
allowedBlocks: ALLOWED_BLOCKS,
__experimentalLayout: usedLayout,
} );

// Get the Discussion settings
Expand Down
2 changes: 0 additions & 2 deletions packages/block-library/src/gallery/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ const linkOptions = [
];
const ALLOWED_MEDIA_TYPES = [ 'image' ];
const allowedBlocks = [ 'core/image' ];
const LAYOUT = { type: 'default', alignments: [] };

const PLACEHOLDER_TEXT = Platform.isNative
? __( 'ADD MEDIA' )
Expand Down Expand Up @@ -531,7 +530,6 @@ function GalleryEdit( props ) {
allowedBlocks,
orientation: 'horizontal',
renderAppender: false,
__experimentalLayout: LAYOUT,
...nativeInnerBlockProps,
} );

Expand Down
4 changes: 3 additions & 1 deletion packages/block-library/src/group/deprecated.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,9 @@ const deprecated = [
);
},
isEligible: ( { layout } ) =>
! layout || layout.inherit || layout.contentSize,
! layout ||
layout.inherit ||
( layout.contentSize && layout.type !== 'constrained' ),
migrate: ( attributes ) => {
const { layout = null } = attributes;
if ( ! layout ) {
Expand Down
1 change: 0 additions & 1 deletion packages/block-library/src/group/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ function GroupEdit( {
renderAppender: hasInnerBlocks
? undefined
: InnerBlocks.ButtonBlockAppender,
__experimentalLayout: layoutSupportEnabled ? usedLayout : undefined,
__unstableDisableLayoutClassNames: ! layoutSupportEnabled,
}
);
Expand Down
6 changes: 0 additions & 6 deletions packages/block-library/src/navigation/edit/inner-blocks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,6 @@ const DEFAULT_BLOCK = {
name: 'core/navigation-link',
};

const LAYOUT = {
type: 'default',
alignments: [],
};

export default function NavigationInnerBlocks( {
clientId,
hasCustomPlaceholder,
Expand Down Expand Up @@ -131,7 +126,6 @@ export default function NavigationInnerBlocks( {
parentOrChildHasSelection
? InnerBlocks.ButtonBlockAppender
: false,
__experimentalLayout: LAYOUT,
placeholder: showPlaceholder ? placeholder : undefined,
}
);
Expand Down
15 changes: 2 additions & 13 deletions packages/block-library/src/post-content/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,11 @@
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { useSelect } from '@wordpress/data';
import {
useBlockProps,
useInnerBlocksProps,
useSetting,
__experimentalRecursionProvider as RecursionProvider,
__experimentalUseHasRecursion as useHasRecursion,
store as blockEditorStore,
Warning,
} from '@wordpress/block-editor';
import { useEntityProp, useEntityBlockEditor } from '@wordpress/core-data';
Expand Down Expand Up @@ -39,16 +36,9 @@ function ReadOnlyContent( { userCanEdit, postType, postId } ) {
);
}

function EditableContent( { layout, context = {} } ) {
function EditableContent( { context = {} } ) {
const { postType, postId } = context;
const themeSupportsLayout = useSelect( ( select ) => {
const { getSettings } = select( blockEditorStore );
return getSettings()?.supportsLayout;
}, [] );
const defaultLayout = useSetting( 'layout' ) || {};
const usedLayout = ! layout?.type
? { ...defaultLayout, ...layout, type: 'default' }
: { ...defaultLayout, ...layout };

const [ blocks, onInput, onChange ] = useEntityBlockEditor(
'postType',
postType,
Expand All @@ -61,7 +51,6 @@ function EditableContent( { layout, context = {} } ) {
value: blocks,
onInput,
onChange,
__experimentalLayout: themeSupportsLayout ? usedLayout : undefined,
}
);
return <div { ...props } />;
Expand Down
14 changes: 1 addition & 13 deletions packages/block-library/src/query-pagination/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
store as blockEditorStore,
} from '@wordpress/block-editor';
import { useSelect } from '@wordpress/data';
import { getBlockSupport } from '@wordpress/blocks';
import { PanelBody } from '@wordpress/components';

/**
Expand All @@ -28,21 +27,11 @@ const ALLOWED_BLOCKS = [
'core/query-pagination-next',
];

const getDefaultBlockLayout = ( blockTypeOrName ) => {
const layoutBlockSupportConfig = getBlockSupport(
blockTypeOrName,
'__experimentalLayout'
);
return layoutBlockSupportConfig?.default;
};

export default function QueryPaginationEdit( {
attributes: { paginationArrow, layout },
attributes: { paginationArrow },
setAttributes,
clientId,
name,
} ) {
const usedLayout = layout || getDefaultBlockLayout( name );
const hasNextPreviousBlocks = useSelect( ( select ) => {
const { getBlocks } = select( blockEditorStore );
const innerBlocks = getBlocks( clientId );
Expand All @@ -61,7 +50,6 @@ export default function QueryPaginationEdit( {
const innerBlocksProps = useInnerBlocksProps( blockProps, {
template: TEMPLATE,
allowedBlocks: ALLOWED_BLOCKS,
__experimentalLayout: usedLayout,
} );
return (
<>
Expand Down
Loading