Skip to content

Commit

Permalink
Add mechanism to avoid forced child selection on blocks with template…
Browse files Browse the repository at this point in the history
…s. (#10696)


Currently, if a block contains a template when we insert that block the block that gets focused is one of the child blocks specified in the template. We don't offer a way to disable this behavior.

This current behavior makes sense in most cases (e.g: columns) but for some cases,
we may want to keep the focus on the parent. E.g. In PR #9416 @afercia pointed some a11y concerns in automatically selecting the child block on the InnerBlocks area.

Blocks may have their own edition area and an InnerBlocks are at the end. Automatically selecting a block in the InnerBlocks area, has some accessible concerns has the parent block edition area may be unnoticeable for screen reader users.

This PR adds a flag to the InnerBlocks component that allows blocks to disable the behavior of automatically selecting the children. In order for this flag to be possible another flag was added in insertBlock(s) action that allows for blocks to be inserted without a selection update happening.
  • Loading branch information
jorgefilipecosta authored Nov 7, 2018
1 parent 7c63d10 commit 16a718a
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 25 deletions.
8 changes: 4 additions & 4 deletions docs/data/data-core-editor.md
Original file line number Diff line number Diff line change
Expand Up @@ -1532,8 +1532,8 @@ inserted, optionally at a specific index respective a root block list.

* block: Block object to insert.
* index: Index at which block should be inserted.
* rootClientId: Optional root client ID of block list on which
to insert.
* rootClientId: Optional root client ID of block list on which to insert.
* updateSelection: If true block selection will be updated. If false, block selection will not change. Defaults to true.

### insertBlocks

Expand All @@ -1544,8 +1544,8 @@ be inserted, optionally at a specific index respective a root block list.

* blocks: Block objects to insert.
* index: Index at which block should be inserted.
* rootClientId: Optional root client ID of block list on
which to insert.
* rootClientId: Optional root cliente ID of block list on which to insert.
* updateSelection: If true block selection will be updated. If false, block selection will not change. Defaults to true.

### showInsertionPoint

Expand Down
7 changes: 7 additions & 0 deletions packages/editor/src/components/inner-blocks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,13 @@ const TEMPLATE = [ [ 'core/columns', {}, [

The previous example creates an InnerBlocks area containing two columns one with an image and the other with a paragraph.

### `templateInsertUpdatesSelection`
* **Type:** `Boolean`
* **Default:** `true`

If true when child blocks in the template are inserted the selection is updated.
If false the selection should not be updated when child blocks specified in the template are inserted.

### `templateLock`
* **Type:** `String|Boolean`

Expand Down
4 changes: 2 additions & 2 deletions packages/editor/src/components/inner-blocks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,15 @@ InnerBlocks = compose( [
insertBlocks,
updateBlockListSettings,
} = dispatch( 'core/editor' );
const { block, clientId } = ownProps;
const { block, clientId, templateInsertUpdatesSelection = true } = ownProps;

return {
replaceInnerBlocks( blocks ) {
const clientIds = map( block.innerBlocks, 'clientId' );
if ( clientIds.length ) {
replaceBlocks( clientIds, blocks );
} else {
insertBlocks( blocks, undefined, clientId );
insertBlocks( blocks, undefined, clientId, templateInsertUpdatesSelection );
}
},
updateNestedSettings( settings ) {
Expand Down
23 changes: 12 additions & 11 deletions packages/editor/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -295,35 +295,36 @@ export function moveBlockToPosition( clientId, fromRootClientId, toRootClientId,
* Returns an action object used in signalling that a single block should be
* inserted, optionally at a specific index respective a root block list.
*
* @param {Object} block Block object to insert.
* @param {?number} index Index at which block should be inserted.
* @param {?string} rootClientId Optional root client ID of block list on which
* to insert.
* @param {Object} block Block object to insert.
* @param {?number} index Index at which block should be inserted.
* @param {?string} rootClientId Optional root client ID of block list on which to insert.
* @param {?boolean} updateSelection If true block selection will be updated. If false, block selection will not change. Defaults to true.
*
* @return {Object} Action object.
*/
export function insertBlock( block, index, rootClientId ) {
return insertBlocks( [ block ], index, rootClientId );
export function insertBlock( block, index, rootClientId, updateSelection = true ) {
return insertBlocks( [ block ], index, rootClientId, updateSelection );
}

/**
* Returns an action object used in signalling that an array of blocks should
* be inserted, optionally at a specific index respective a root block list.
*
* @param {Object[]} blocks Block objects to insert.
* @param {?number} index Index at which block should be inserted.
* @param {?string} rootClientId Optional root client ID of block list on
* which to insert.
* @param {Object[]} blocks Block objects to insert.
* @param {?number} index Index at which block should be inserted.
* @param {?string} rootClientId Optional root cliente ID of block list on which to insert.
* @param {?boolean} updateSelection If true block selection will be updated. If false, block selection will not change. Defaults to true.
*
* @return {Object} Action object.
*/
export function insertBlocks( blocks, index, rootClientId ) {
export function insertBlocks( blocks, index, rootClientId, updateSelection = true ) {
return {
type: 'INSERT_BLOCKS',
blocks: castArray( blocks ),
index,
rootClientId,
time: Date.now(),
updateSelection,
};
}

Expand Down
20 changes: 12 additions & 8 deletions packages/editor/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -705,14 +705,18 @@ export function blockSelection( state = {
end: action.clientId,
initialPosition: action.initialPosition,
};
case 'INSERT_BLOCKS':
return {
...state,
start: action.blocks[ 0 ].clientId,
end: action.blocks[ 0 ].clientId,
initialPosition: null,
isMultiSelecting: false,
};
case 'INSERT_BLOCKS': {
if ( action.updateSelection ) {
return {
...state,
start: action.blocks[ 0 ].clientId,
end: action.blocks[ 0 ].clientId,
initialPosition: null,
isMultiSelecting: false,
};
}
return state;
}
case 'REMOVE_BLOCKS':
if ( ! action.clientIds || ! action.clientIds.length || action.clientIds.indexOf( state.start ) === -1 ) {
return state;
Expand Down
2 changes: 2 additions & 0 deletions packages/editor/src/store/test/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ describe( 'actions', () => {
index,
rootClientId: 'testclientid',
time: expect.any( Number ),
updateSelection: true,
} );
} );
} );
Expand All @@ -204,6 +205,7 @@ describe( 'actions', () => {
index,
rootClientId: 'testclientid',
time: expect.any( Number ),
updateSelection: true,
} );
} );
} );
Expand Down
19 changes: 19 additions & 0 deletions packages/editor/src/store/test/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1567,6 +1567,7 @@ describe( 'state', () => {
clientId: 'ribs',
name: 'core/freeform',
} ],
updateSelection: true,
} );

expect( state3 ).toEqual( {
Expand All @@ -1577,6 +1578,24 @@ describe( 'state', () => {
} );
} );

it( 'should not select inserted block if updateSelection flag is false', () => {
const original = deepFreeze( { start: 'a', end: 'b' } );

const state3 = blockSelection( original, {
type: 'INSERT_BLOCKS',
blocks: [ {
clientId: 'ribs',
name: 'core/freeform',
} ],
updateSelection: false,
} );

expect( state3 ).toEqual( {
start: 'a',
end: 'b',
} );
} );

it( 'should not update the state if the block moved is already selected', () => {
const original = deepFreeze( { start: 'ribs', end: 'ribs' } );
const state = blockSelection( original, {
Expand Down

0 comments on commit 16a718a

Please sign in to comment.