-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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' }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 I've switched it to a group instead. |
||
block3: { name: 'core/test-block-parent' }, | ||
} ) | ||
), | ||
|
@@ -4668,4 +4668,114 @@ describe( 'getBlockEditingMode', () => { | |
) | ||
).toBe( 'disabled' ); | ||
} ); | ||
|
||
describe( 'pattern blocks', () => { | ||
const patternBlockState = { | ||
settings: {}, | ||
blocks: { | ||
byClientId: new Map( | ||
Object.entries( { | ||
'pattern-a': { name: 'core/block' }, | ||
'pattern-b': { name: 'core/block' }, | ||
'heading-a': { name: 'core/heading' }, | ||
'paragraph-a': { name: 'core/paragraph' }, | ||
'paragraph-b': { name: 'core/paragraph' }, | ||
} ) | ||
), | ||
order: new Map( | ||
Object.entries( { | ||
'': [ 'pattern-a' ], | ||
'pattern-a': [ | ||
'heading-a', | ||
'paragraph-a', | ||
'pattern-b', | ||
], | ||
'pattern-b': [ 'paragraph-b' ], | ||
} ) | ||
), | ||
parents: new Map( | ||
Object.entries( { | ||
'paragraph-b': 'pattern-b', | ||
'pattern-b': 'pattern-a', | ||
'paragraph-a': 'pattern-a', | ||
'heading-a': 'pattern-a', | ||
'pattern-a': '', | ||
} ) | ||
), | ||
blockListSettings: { | ||
'pattern-a': {}, | ||
'pattern-b': {}, | ||
}, | ||
attributes: new Map( | ||
Object.entries( { | ||
'paragraph-a': { | ||
metadata: { | ||
bindings: { | ||
__default: { | ||
source: 'core/pattern-overrides', | ||
}, | ||
}, | ||
}, | ||
}, | ||
'paragraph-b': { | ||
metadata: { | ||
bindings: { | ||
__default: { | ||
source: 'core/pattern-overrides', | ||
}, | ||
}, | ||
}, | ||
}, | ||
} ) | ||
), | ||
}, | ||
}; | ||
|
||
it( 'should return contentOnly for the outer pattern block', () => { | ||
expect( | ||
getBlockEditingMode( patternBlockState, 'pattern-a' ) | ||
).toBe( 'contentOnly' ); | ||
} ); | ||
|
||
it( 'should return contentOnly for blocks with bindings in the outer pattern', () => { | ||
expect( | ||
getBlockEditingMode( patternBlockState, 'paragraph-a' ) | ||
).toBe( 'contentOnly' ); | ||
} ); | ||
|
||
it( 'should return disabled for unbound blocks', () => { | ||
expect( | ||
getBlockEditingMode( patternBlockState, 'heading-a' ) | ||
).toBe( 'disabled' ); | ||
} ); | ||
|
||
it( 'should return disabled for the nested pattern', () => { | ||
expect( | ||
getBlockEditingMode( patternBlockState, 'pattern-a' ) | ||
).toBe( 'contentOnly' ); | ||
} ); | ||
|
||
it( 'should return disabled for blocks with bindings in the nested pattern', () => { | ||
expect( | ||
getBlockEditingMode( patternBlockState, 'paragraph-b' ) | ||
).toBe( 'disabled' ); | ||
} ); | ||
|
||
it( 'should disable all inner blocks of the outer pattern in zoom out mode with the outer pattern in content only mode', () => { | ||
const state = { | ||
...patternBlockState, | ||
editorMode: 'zoom-out', | ||
}; | ||
expect( getBlockEditingMode( state, 'pattern-a' ) ).toBe( | ||
'contentOnly' | ||
); | ||
[ 'paragraph-a', 'paragraph-b', 'heading-a', 'pattern-b' ].forEach( | ||
( block ) => { | ||
expect( getBlockEditingMode( state, block ) ).toBe( | ||
'disabled' | ||
); | ||
} | ||
); | ||
} ); | ||
} ); | ||
} ); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
|
@@ -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' ]; | ||
|
||
|
@@ -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 ( | ||
|
@@ -171,7 +153,6 @@ function ReusableBlockEdit( { | |
name, | ||
attributes: { ref, content }, | ||
__unstableParentLayout: parentLayout, | ||
clientId: patternClientId, | ||
setAttributes, | ||
} ) { | ||
const { record, hasResolved } = useEntityRecord( | ||
|
@@ -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( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ] | ||
|
@@ -244,7 +200,6 @@ function ReusableBlockEdit( { | |
} ); | ||
|
||
const innerBlocksProps = useInnerBlocksProps( blockProps, { | ||
templateLock: 'all', | ||
layout, | ||
value: blocks, | ||
onInput: NOOP, | ||
|
There was a problem hiding this comment.
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 🤔