-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Introduce Block type level lock control #32457
Changes from all commits
a272973
93d1d75
4bd6022
7e346b9
1605281
91397c8
1d5cd17
212a80a
c748df7
bb8f09d
c8994a4
15fac68
6f8dcbc
ef47e8e
036e904
e1a71ee
a3f4aed
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 |
---|---|---|
|
@@ -48,6 +48,62 @@ _Returns_ | |
|
||
- `boolean`: Whether the given block type is allowed to be inserted. | ||
|
||
### canMoveBlock | ||
|
||
Determines if the given block is allowed to be moved. | ||
|
||
_Parameters_ | ||
|
||
- _state_ `Object`: Editor state. | ||
- _clientId_ `string`: The block client Id. | ||
- _rootClientId_ `?string`: Optional root client ID of block list. | ||
|
||
_Returns_ | ||
|
||
- `boolean`: Whether the given block is allowed to be moved. | ||
|
||
### canMoveBlocks | ||
|
||
Determines if the given blocks are allowed to be moved. | ||
|
||
_Parameters_ | ||
|
||
- _state_ `Object`: Editor state. | ||
- _clientIds_ `string`: The block client IDs to be moved. | ||
- _rootClientId_ `?string`: Optional root client ID of block list. | ||
|
||
_Returns_ | ||
|
||
- `boolean`: Whether the given blocks are allowed to be moved. | ||
|
||
### canRemoveBlock | ||
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. The canRemoveBlock block should also be used on the transforms verification. If a block can not be removed I should also not be able to transform it into another block and right now we can. 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 is now resolved with 15fac68 |
||
|
||
Determines if the given block is allowed to be deleted. | ||
|
||
_Parameters_ | ||
|
||
- _state_ `Object`: Editor state. | ||
- _clientId_ `string`: The block client Id. | ||
- _rootClientId_ `?string`: Optional root client ID of block list. | ||
|
||
_Returns_ | ||
|
||
- `boolean`: Whether the given block is allowed to be removed. | ||
|
||
### canRemoveBlocks | ||
|
||
Determines if the given blocks are allowed to be removed. | ||
|
||
_Parameters_ | ||
|
||
- _state_ `Object`: Editor state. | ||
- _clientIds_ `string`: The block client IDs to be removed. | ||
- _rootClientId_ `?string`: Optional root client ID of block list. | ||
|
||
_Returns_ | ||
|
||
- `boolean`: Whether the given blocks are allowed to be removed. | ||
|
||
### didAutomaticChange | ||
|
||
Returns true if the last change was an automatic change, false otherwise. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,7 @@ function BlockMover( { | |
isFirst, | ||
isLast, | ||
clientIds, | ||
isLocked, | ||
canMove, | ||
isHidden, | ||
rootClientId, | ||
orientation, | ||
|
@@ -37,7 +37,7 @@ function BlockMover( { | |
const onFocus = () => setIsFocused( true ); | ||
const onBlur = () => setIsFocused( false ); | ||
|
||
if ( isLocked || ( isFirst && isLast && ! rootClientId ) ) { | ||
if ( ! canMove || ( isFirst && isLast && ! rootClientId ) ) { | ||
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.
|
||
return null; | ||
} | ||
|
||
|
@@ -100,7 +100,7 @@ export default withSelect( ( select, { clientIds } ) => { | |
getBlock, | ||
getBlockIndex, | ||
getBlockListSettings, | ||
getTemplateLock, | ||
canMoveBlocks, | ||
getBlockOrder, | ||
getBlockRootClientId, | ||
} = select( blockEditorStore ); | ||
|
@@ -119,7 +119,7 @@ export default withSelect( ( select, { clientIds } ) => { | |
|
||
return { | ||
blockType: block ? getBlockType( block.name ) : null, | ||
isLocked: getTemplateLock( rootClientId ) === 'all', | ||
canMove: canMoveBlocks( clientIds, rootClientId ), | ||
rootClientId, | ||
firstIndex, | ||
isFirst, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,42 +10,40 @@ import { BlockMover } from '../index'; | |
|
||
describe( 'Block Mover Picker', () => { | ||
it( 'renders without crashing', () => { | ||
const wrapper = shallow( <BlockMover />, { | ||
context: { | ||
isFirst: false, | ||
isLast: true, | ||
isLocked: false, | ||
numberOfBlocks: 2, | ||
firstIndex: 1, | ||
const props = { | ||
isFirst: false, | ||
isLast: true, | ||
canMove: true, | ||
numberOfBlocks: 2, | ||
firstIndex: 1, | ||
|
||
onMoveDown: jest.fn(), | ||
onMoveUp: jest.fn(), | ||
onLongPress: jest.fn(), | ||
onMoveDown: jest.fn(), | ||
onMoveUp: jest.fn(), | ||
onLongPress: jest.fn(), | ||
|
||
rootClientId: '', | ||
isStackedHorizontally: true, | ||
}, | ||
} ); | ||
rootClientId: '', | ||
isStackedHorizontally: true, | ||
}; | ||
const wrapper = shallow( <BlockMover { ...props } /> ); | ||
expect( wrapper ).toBeTruthy(); | ||
} ); | ||
|
||
it( 'should match snapshot', () => { | ||
const wrapper = shallow( <BlockMover />, { | ||
context: { | ||
isFirst: false, | ||
isLast: true, | ||
isLocked: false, | ||
numberOfBlocks: 2, | ||
firstIndex: 1, | ||
const props = { | ||
isFirst: false, | ||
isLast: true, | ||
canMove: true, | ||
numberOfBlocks: 2, | ||
firstIndex: 1, | ||
|
||
onMoveDown: jest.fn(), | ||
onMoveUp: jest.fn(), | ||
onLongPress: jest.fn(), | ||
onMoveDown: jest.fn(), | ||
onMoveUp: jest.fn(), | ||
onLongPress: jest.fn(), | ||
|
||
rootClientId: '', | ||
isStackedHorizontally: true, | ||
}, | ||
} ); | ||
rootClientId: '', | ||
isStackedHorizontally: true, | ||
}; | ||
const wrapper = shallow( <BlockMover { ...props } /> ); | ||
Comment on lines
+13
to
+46
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. Those tests were never running correctly, they're fixed now. |
||
expect( wrapper ).toMatchSnapshot(); | ||
} ); | ||
} ); |
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.
Cool work.
It looks like the documentation needs to be fixed. It differs from the PR's description and feels broken here: