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.

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

Pattern block: Ensure consistent editing of overrides in Write Mode #65408

Merged
merged 4 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
18 changes: 18 additions & 0 deletions packages/block-editor/src/store/private-selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -684,3 +684,21 @@ export function getClosestAllowedInsertionPointForPattern(
export function getInsertionPoint( state ) {
return state.insertionPoint;
}

/**
* Retrieves the number of parent pattern blocks.
*
* @param {Object} state Global application state.
* @param {string} clientId The block client ID.
*
* @return {number} The number of parent pattern blocks.
*/
export function getParentPatternCount( state, clientId ) {
Copy link
Contributor Author

@talldan talldan Sep 17, 2024

Choose a reason for hiding this comment

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

Might be an idea to make this one memoized, and also check that the memoization of selectors that use getBlockEditingMode is still correct 🤔

const parents = getBlockParents( state, clientId );
return parents.reduce( ( count, parent ) => {
if ( getBlockName( state, parent ) === 'core/block' ) {
return count + 1;
}
return count;
}, 0 );
}
37 changes: 37 additions & 0 deletions packages/block-editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
getSectionRootClientId,
isSectionBlock,
getParentSectionBlock,
getParentPatternCount,
} from './private-selectors';

/**
Expand Down Expand Up @@ -2972,6 +2973,42 @@ export const getBlockEditingMode = createRegistrySelector(
clientId = '';
}

// Handle pattern blocks (core/block) and the content of those blocks.
const parentPatternCount = getParentPatternCount( state, clientId );
if ( parentPatternCount > 0 ) {
// Disable nested patterns.
if ( parentPatternCount > 1 ) {
return 'disabled';
}

// Make the outer pattern block content only mode.
if (
getBlockName( state, clientId ) === 'core/block' &&
parentPatternCount === 0
) {
return 'contentOnly';
}

// Disable pattern content editing in zoom-out mode.
const _isZoomOut =
__unstableGetEditorMode( state ) === 'zoom-out';
if ( _isZoomOut ) {
return 'disabled';
}

// If the block has a binding of any kind, allow content only editing.
const attributes = getBlockAttributes( state, clientId );
if (
Object.keys( attributes.metadata?.bindings ?? {} )?.length >
0
) {
return 'contentOnly';
}

// Otherwise, the block is part of the pattern source and should not be editable.
return 'disabled';
}

// In zoom-out mode, override the behavior set by
// __unstableSetBlockEditingMode to only allow editing the top-level
// sections.
Expand Down
3 changes: 2 additions & 1 deletion packages/block-editor/src/store/test/private-selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,6 @@ describe( 'private selectors', () => {
'e178812d-ce5e-48c7-a945-8ae4ffcbbb7c',
],
] ),

order: new Map( [
[
'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337',
Expand All @@ -443,6 +442,7 @@ describe( 'private selectors', () => {
],
[ '', [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337' ] ],
] ),
byClientId: new Map( [] ),
},
blockEditingModes: new Map( [
[ '', 'disabled' ],
Expand Down Expand Up @@ -495,6 +495,7 @@ describe( 'private selectors', () => {
],
[ '', [ 'ef45d5fd-5234-4fd5-ac4f-c3736c7f9337' ] ],
] ),
byClientId: new Map( [] ),
},
blockEditingModes: new Map( [
[ '', 'disabled' ],
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/store/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -3079,7 +3079,7 @@ describe( 'selectors', () => {
byClientId: new Map(
Object.entries( {
block1: { name: 'core/test-block-ancestor' },
block2: { name: 'core/block' },
block2: { name: 'core/group' },
Copy link
Contributor Author

@talldan talldan Sep 17, 2024

Choose a reason for hiding this comment

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

This test was failing because it tries to insert a block within a child of a core/block block. This wouldn't be possible by a user in trunk (as it's disallowed by the block's 'edit') and it also failed in unit tests in this PR now that the block editing mode is enforced at selector level.

I've switched it to a group instead.

block3: { name: 'core/test-block-parent' },
} )
),
Expand Down
57 changes: 6 additions & 51 deletions packages/block-library/src/block/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import clsx from 'clsx';
* WordPress dependencies
*/
import { useSelect, useDispatch } from '@wordpress/data';
import { useRef, useMemo, useEffect } from '@wordpress/element';
import { useRef, useMemo } from '@wordpress/element';
import {
useEntityRecord,
store as coreStore,
Expand Down Expand Up @@ -37,12 +37,10 @@ import { getBlockBindingsSource } from '@wordpress/blocks';
/**
* Internal dependencies
*/
import { name as patternBlockName } from './index';
import { unlock } from '../lock-unlock';

const { useLayoutClasses } = unlock( blockEditorPrivateApis );
const { isOverridableBlock, hasOverridableBlocks } =
unlock( patternsPrivateApis );
const { hasOverridableBlocks } = unlock( patternsPrivateApis );

const fullAlignments = [ 'full', 'wide', 'left', 'right' ];

Expand Down Expand Up @@ -75,22 +73,6 @@ const useInferredLayout = ( blocks, parentLayout ) => {
}, [ blocks, parentLayout ] );
};

function setBlockEditMode( setEditMode, blocks, mode ) {
blocks.forEach( ( block ) => {
const editMode =
mode ||
( isOverridableBlock( block ) ? 'contentOnly' : 'disabled' );
setEditMode( block.clientId, editMode );

setBlockEditMode(
setEditMode,
block.innerBlocks,
// Disable editing for nested patterns.
block.name === patternBlockName ? 'disabled' : mode
);
} );
}

function RecursionWarning() {
const blockProps = useBlockProps();
return (
Expand Down Expand Up @@ -171,7 +153,6 @@ function ReusableBlockEdit( {
name,
attributes: { ref, content },
__unstableParentLayout: parentLayout,
clientId: patternClientId,
setAttributes,
} ) {
const { record, hasResolved } = useEntityRecord(
Expand All @@ -184,49 +165,24 @@ function ReusableBlockEdit( {
} );
const isMissing = hasResolved && ! record;

const { setBlockEditingMode, __unstableMarkLastChangeAsPersistent } =
const { __unstableMarkLastChangeAsPersistent } =
useDispatch( blockEditorStore );

const {
innerBlocks,
onNavigateToEntityRecord,
editingMode,
hasPatternOverridesSource,
} = useSelect(
const { onNavigateToEntityRecord, hasPatternOverridesSource } = useSelect(
( select ) => {
const { getBlocks, getSettings, getBlockEditingMode } =
select( blockEditorStore );
const { getSettings } = select( blockEditorStore );
// For editing link to the site editor if the theme and user permissions support it.
return {
innerBlocks: getBlocks( patternClientId ),
onNavigateToEntityRecord:
getSettings().onNavigateToEntityRecord,
editingMode: getBlockEditingMode( patternClientId ),
hasPatternOverridesSource: !! getBlockBindingsSource(
'core/pattern-overrides'
),
};
},
[ patternClientId ]
[]
);

// Sync the editing mode of the pattern block with the inner blocks.
useEffect( () => {
setBlockEditMode(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice to see this gone :)

setBlockEditingMode,
innerBlocks,
// Disable editing if the pattern itself is disabled.
editingMode === 'disabled' || ! hasPatternOverridesSource
? 'disabled'
: undefined
);
}, [
editingMode,
innerBlocks,
setBlockEditingMode,
hasPatternOverridesSource,
] );

const canOverrideBlocks = useMemo(
() => hasPatternOverridesSource && hasOverridableBlocks( blocks ),
[ hasPatternOverridesSource, blocks ]
Expand All @@ -244,7 +200,6 @@ function ReusableBlockEdit( {
} );

const innerBlocksProps = useInnerBlocksProps( blockProps, {
templateLock: 'all',
layout,
value: blocks,
onInput: NOOP,
Expand Down
Loading