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

Inserter: Fix 'Browse All' in Quick Inserter #26443

Merged
merged 10 commits into from
Nov 10, 2020
30 changes: 19 additions & 11 deletions docs/designers-developers/developers/data/data-core-block-editor.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,16 +160,22 @@ _Returns_

<a name="getBlockInsertionPoint" href="#getBlockInsertionPoint">#</a> **getBlockInsertionPoint**

Returns the insertion point, the index at which the new inserted block would
be placed. Defaults to the last index.
Returns the insertion point. This will be:

1) The insertion point manually set using setInsertionPoint() or
showInsertionPoint(); or
2) The point after the current block selection, if there is a selection; or
3) The point at the end of the block list.

Components like <Inserter> will default to inserting blocks at this point.

_Parameters_

- _state_ `Object`: Editor state.
- _state_ `Object`: Global application state.

_Returns_

- `Object`: Insertion point object with `rootClientId`, `index`.
- `Object`: Insertion point object with `rootClientId` and `index`.

<a name="getBlockListSettings" href="#getBlockListSettings">#</a> **getBlockListSettings**

Expand Down Expand Up @@ -812,15 +818,16 @@ _Returns_

<a name="isBlockInsertionPointVisible" href="#isBlockInsertionPointVisible">#</a> **isBlockInsertionPointVisible**

Returns true if we should show the block insertion point.
Whether or not the insertion point should be shown to users. This is set
using showInsertionPoint() or hideInsertionPoint().

_Parameters_

- _state_ `Object`: Global application state.

_Returns_

- `?boolean`: Whether the insertion point is visible or not.
- `?boolean`: Whether the insertion point should be shown.

<a name="isBlockMultiSelected" href="#isBlockMultiSelected">#</a> **isBlockMultiSelected**

Expand Down Expand Up @@ -1048,7 +1055,7 @@ _Parameters_

<a name="hideInsertionPoint" href="#hideInsertionPoint">#</a> **hideInsertionPoint**

Returns an action object hiding the insertion point.
Hides the insertion point for users.

_Returns_

Expand Down Expand Up @@ -1373,13 +1380,14 @@ _Returns_

<a name="showInsertionPoint" href="#showInsertionPoint">#</a> **showInsertionPoint**

Returns an action object used in signalling that the insertion point should
be shown.
Sets the insertion point and shows it to users.

Components like <Inserter> will default to inserting blocks at this point.

_Parameters_

- _rootClientId_ `?string`: Optional root client ID of block list on which to insert.
- _index_ `?number`: Index at which block should be inserted.
- _rootClientId_ `?string`: Root client ID of block list in which to insert. Use `undefined` for the root block list.
- _index_ `number`: Index at which block should be inserted.

_Returns_

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
/**
* External dependencies
*/
import { pick } from 'lodash';

/**
* WordPress dependencies
*/
Expand All @@ -14,10 +9,17 @@ import { speak } from '@wordpress/a11y';
/**
* @typedef WPInserterConfig
*
* @property {string=} rootClientId Inserter Root Client ID.
* @property {string=} clientId Inserter Client ID.
* @property {boolean} isAppender Whether the inserter is an appender or not.
* @property {boolean} selectBlockOnInsert Whether the block should be selected on insert.
* @property {string=} rootClientId If set, insertion will be into the
* block with this ID.
* @property {number=} insertionIndex If set, insertion will be into this
* explicit position.
* @property {string=} clientId If set, insertion will be after the
* block with this ID.
* @property {boolean=} isAppender Whether the inserter is an appender
* or not.
* @property {boolean=} selectBlockOnInsert Whether the block should be
* selected on insert.
* @property {Function=} onSelect Called after insertion.
*/

/**
Expand All @@ -27,77 +29,74 @@ import { speak } from '@wordpress/a11y';
* @return {Array} Insertion Point State (rootClientID, onInsertBlocks and onToggle).
*/
function useInsertionPoint( {
onSelect,
rootClientId,
insertionIndex,
clientId,
isAppender,
selectBlockOnInsert,
insertionIndex,
onSelect,
} ) {
const {
selectedBlock,
destinationRootClientId,
getSelectedBlock,
getBlockIndex,
getBlockSelectionEnd,
getBlockOrder,
destinationIndex,
} = useSelect(
( select ) => {
const {
getSettings,
getBlockRootClientId,
getBlockSelectionEnd: _getBlockSelectionEnd,
getSelectedBlock,
getBlockIndex,
getBlockOrder,
getBlockInsertionPoint,
} = select( 'core/block-editor' );

let destRootClientId = rootClientId;
if ( ! destRootClientId && ! clientId && ! isAppender ) {
const end = _getBlockSelectionEnd();
if ( end ) {
destRootClientId = getBlockRootClientId( end );
let _destinationRootClientId, _destinationIndex;

if ( rootClientId || insertionIndex || clientId || isAppender ) {
Copy link
Contributor

@talldan talldan Oct 26, 2020

Choose a reason for hiding this comment

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

There was a relevant comment here about these props - #25763 (comment).

So the idea would to refactor inserters to just use rootClientId and blockIndex. The insertion point state seems to fit well with that, which is nice (no action required here 😄 ).

Copy link
Member Author

Choose a reason for hiding this comment

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

I like that idea. It's definitely a little crazy right now.

// If any of these arguments are set, we're in "manual mode"
// meaning the insertion point is set by the caller.

_destinationRootClientId = rootClientId;

if ( insertionIndex ) {
// Insert into a specific index.
_destinationIndex = insertionIndex;
} else if ( clientId ) {
// Insert after a specific client ID.
_destinationIndex = getBlockIndex(
clientId,
_destinationRootClientId
);
} else {
// Insert at the end of the list.
_destinationIndex = getBlockOrder(
_destinationRootClientId
).length;
}
} else {
// Otherwise, we're in "auto mode" where the insertion point is
// decided by getBlockInsertionPoint().
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we can simplify further and make this the default mode: Maybe by "setting the insertion point" when we open the inserter (all inserters)

const insertionPoint = getBlockInsertionPoint();
_destinationRootClientId = insertionPoint.rootClientId;
_destinationIndex = insertionPoint.index;
}

return {
hasPatterns: !! getSettings().__experimentalBlockPatterns
?.length,
destinationRootClientId: destRootClientId,
...pick( select( 'core/block-editor' ), [
'getSelectedBlock',
'getBlockIndex',
'getBlockSelectionEnd',
'getBlockOrder',
] ),
selectedBlock: getSelectedBlock(),
destinationRootClientId: _destinationRootClientId,
destinationIndex: _destinationIndex,
};
},
[ isAppender, clientId, rootClientId ]
[ rootClientId, insertionIndex, clientId, isAppender ]
);

const {
replaceBlocks,
insertBlocks,
showInsertionPoint,
hideInsertionPoint,
} = useDispatch( 'core/block-editor' );

function getInsertionIndex() {
if ( insertionIndex !== undefined ) {
return insertionIndex;
}

// If the clientId is defined, we insert at the position of the block.
if ( clientId ) {
return getBlockIndex( clientId, destinationRootClientId );
}

// If there's a selected block, and the selected block is not the destination root block, we insert after the selected block.
const end = getBlockSelectionEnd();
if ( ! isAppender && end ) {
return getBlockIndex( end, destinationRootClientId ) + 1;
}

// Otherwise, we insert at the end of the current rootClientId
return getBlockOrder( destinationRootClientId ).length;
}

const onInsertBlocks = ( blocks, meta ) => {
const selectedBlock = getSelectedBlock();
if (
! isAppender &&
selectedBlock &&
Expand All @@ -107,7 +106,7 @@ function useInsertionPoint( {
} else {
insertBlocks(
blocks,
getInsertionIndex(),
destinationIndex,
destinationRootClientId,
selectBlockOnInsert,
meta
Expand All @@ -131,8 +130,7 @@ function useInsertionPoint( {

const onToggleInsertionPoint = ( show ) => {
if ( show ) {
const index = getInsertionIndex();
showInsertionPoint( destinationRootClientId, index );
showInsertionPoint( destinationRootClientId, destinationIndex );
} else {
hideInsertionPoint();
}
Expand Down
37 changes: 15 additions & 22 deletions packages/block-editor/src/components/inserter/quick-inserter.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,17 +154,18 @@ export default function QuickInserter( {
[ filterValue, patterns ]
);

const setInserterIsOpened = useSelect(
( select ) =>
select( 'core/block-editor' ).getSettings()
.__experimentalSetIsInserterOpened,
[]
);

const previousBlockClientId = useSelect(
( select ) =>
select( 'core/block-editor' ).getPreviousBlockClientId( clientId ),
[ clientId ]
const { setInserterIsOpened, blockIndex } = useSelect(
( select ) => {
const { getSettings, getBlockIndex } = select(
'core/block-editor'
);
return {
setInserterIsOpened: getSettings()
.__experimentalSetIsInserterOpened,
blockIndex: getBlockIndex( clientId, rootClientId ),
};
},
[ clientId, rootClientId ]
);

useEffect( () => {
Expand All @@ -173,7 +174,7 @@ export default function QuickInserter( {
}
}, [ setInserterIsOpened ] );

const { selectBlock } = useDispatch( 'core/block-editor' );
const { __unstableSetInsertionPoint } = useDispatch( 'core/block-editor' );

// Announce search results on change
useEffect( () => {
Expand All @@ -189,17 +190,9 @@ export default function QuickInserter( {
debouncedSpeak( resultsFoundMessage );
}, [ filterValue, debouncedSpeak ] );

// When clicking Browse All select the appropriate block so as
// the insertion point can work as expected
const onBrowseAll = () => {
// We have to select the previous block because the menu inserter
// inserts the new block after the selected one.
// Ideally, this selection shouldn't focus the block to avoid the setTimeout.
selectBlock( previousBlockClientId );
// eslint-disable-next-line @wordpress/react-no-unsafe-timeout
setTimeout( () => {
setInserterIsOpened( true );
} );
__unstableSetInsertionPoint( rootClientId, blockIndex );
setInserterIsOpened( true );
};

// Disable reason (no-autofocus): The inserter menu is a modal display, not one which
Expand Down
34 changes: 28 additions & 6 deletions packages/block-editor/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -523,12 +523,34 @@ export function* insertBlocks(
}

/**
* Returns an action object used in signalling that the insertion point should
* be shown.
* Sets the insertion point without showing it to users.
*
* @param {?string} rootClientId Optional root client ID of block list on
* which to insert.
* @param {?number} index Index at which block should be inserted.
* Components like <Inserter> will default to inserting blocks at this point.
*
* @param {?string} rootClientId Root client ID of block list in which to
* insert. Use `undefined` for the root block
* list.
* @param {number} index Index at which block should be inserted.
*
* @return {Object} Action object.
*/
export function __unstableSetInsertionPoint( rootClientId, index ) {
return {
type: 'SET_INSERTION_POINT',
rootClientId,
index,
};
}

/**
* Sets the insertion point and shows it to users.
*
* Components like <Inserter> will default to inserting blocks at this point.
*
* @param {?string} rootClientId Root client ID of block list in which to
* insert. Use `undefined` for the root block
* list.
* @param {number} index Index at which block should be inserted.
*
* @return {Object} Action object.
*/
Expand All @@ -541,7 +563,7 @@ export function showInsertionPoint( rootClientId, index ) {
}

/**
* Returns an action object hiding the insertion point.
* Hides the insertion point for users.
*
* @return {Object} Action object.
*/
Expand Down
Loading