diff --git a/docs/designers-developers/developers/data/data-core-block-editor.md b/docs/designers-developers/developers/data/data-core-block-editor.md index 5bdea7a18a34e5..e3771108df1015 100644 --- a/docs/designers-developers/developers/data/data-core-block-editor.md +++ b/docs/designers-developers/developers/data/data-core-block-editor.md @@ -158,6 +158,32 @@ Returns the number of blocks currently present in the post. Number of blocks in the post. +### getSelectionStart + +Returns the current selection start block client ID, attribute key and text +offset. + +*Parameters* + + * state: Block editor state. + +*Returns* + +Selection start information. + +### getSelectionEnd + +Returns the current selection end block client ID, attribute key and text +offset. + +*Parameters* + + * state: Block editor state. + +*Returns* + +Selection end information. + ### getBlockSelectionStart Returns the current block selection start. This value may be null, and it @@ -1042,6 +1068,18 @@ Returns an action object used in signalling that the caret has entered formatted Returns an action object used in signalling that the user caret has exited formatted text. +### selectionChange + +Returns an action object used in signalling that the user caret has changed +position. + +*Parameters* + + * clientId: The selected block client ID. + * attributeKey: The selected block attribute key. + * startOffset: The start offset. + * endOffset: The end offset. + ### insertDefaultBlock Returns an action object used in signalling that a new block of the default diff --git a/packages/block-editor/src/components/block-controls/test/__snapshots__/index.js.snap b/packages/block-editor/src/components/block-controls/test/__snapshots__/index.js.snap index ba44cc9445f656..b3a9114f732726 100644 --- a/packages/block-editor/src/components/block-controls/test/__snapshots__/index.js.snap +++ b/packages/block-editor/src/components/block-controls/test/__snapshots__/index.js.snap @@ -5,11 +5,9 @@ exports[`BlockControls should render a dynamic toolbar of controls 1`] = ` value={ Object { "clientId": undefined, - "focusedElement": null, "isSelected": true, "name": undefined, "onFocus": undefined, - "setFocusedElement": [Function], } } > diff --git a/packages/block-editor/src/components/block-edit/index.js b/packages/block-editor/src/components/block-edit/index.js index 582e3a9f3ed17c..cb98989d852558 100644 --- a/packages/block-editor/src/components/block-edit/index.js +++ b/packages/block-editor/src/components/block-edit/index.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import memize from 'memize'; + /** * WordPress dependencies */ @@ -10,40 +15,28 @@ import Edit from './edit'; import { BlockEditContextProvider } from './context'; class BlockEdit extends Component { - constructor( props ) { - super( props ); - - this.setFocusedElement = this.setFocusedElement.bind( this ); - - this.state = { - focusedElement: null, - setFocusedElement: this.setFocusedElement, - }; + constructor() { + super( ...arguments ); + + // It is important to return the same object if props haven't changed + // to avoid unnecessary rerenders. + // See https://reactjs.org/docs/context.html#caveats. + this.propsToContext = memize( + this.propsToContext.bind( this ), + { maxSize: 1 } + ); } - setFocusedElement( focusedElement ) { - this.setState( ( prevState ) => { - if ( prevState.focusedElement === focusedElement ) { - return null; - } - return { focusedElement }; - } ); - } - - static getDerivedStateFromProps( props ) { - const { clientId, name, isSelected, onFocus } = props; - - return { - name, - isSelected, - clientId, - onFocus, - }; + propsToContext( name, isSelected, clientId, onFocus ) { + return { name, isSelected, clientId, onFocus }; } render() { + const { name, isSelected, clientId, onFocus } = this.props; + const value = this.propsToContext( name, isSelected, clientId, onFocus ); + return ( - + ); diff --git a/packages/block-editor/src/components/block-list/block.js b/packages/block-editor/src/components/block-list/block.js index b3c743d4a397c0..50da98eb10359e 100644 --- a/packages/block-editor/src/components/block-list/block.js +++ b/packages/block-editor/src/components/block-list/block.js @@ -12,7 +12,6 @@ import { focus, isTextField, placeCaretAtHorizontalEdge, - placeCaretAtVerticalEdge, } from '@wordpress/dom'; import { BACKSPACE, DELETE, ENTER } from '@wordpress/keycodes'; import { @@ -154,13 +153,7 @@ export class BlockListBlock extends Component { return; } - target.focus(); - - // In reverse case, need to explicitly place caret position. - if ( isReverse ) { - placeCaretAtHorizontalEdge( target, true ); - placeCaretAtVerticalEdge( target, true ); - } + placeCaretAtHorizontalEdge( target, isReverse ); } setAttributes( attributes ) { diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js index e6754796ba165f..564edf112a1a77 100644 --- a/packages/block-editor/src/components/rich-text/index.js +++ b/packages/block-editor/src/components/rich-text/index.js @@ -5,7 +5,6 @@ import classnames from 'classnames'; import { find, isNil, - isEqual, omit, pickBy, } from 'lodash'; @@ -104,7 +103,7 @@ function createPrepareEditableTree( props ) { } export class RichText extends Component { - constructor( { value, onReplace, multiline } ) { + constructor( { value, onReplace, multiline, selectionStart, selectionEnd } ) { super( ...arguments ); if ( multiline === true || multiline === 'p' || multiline === 'li' ) { @@ -133,7 +132,6 @@ export class RichText extends Component { this.onKeyDown = this.onKeyDown.bind( this ); this.onPaste = this.onPaste.bind( this ); this.onCreateUndoLevel = this.onCreateUndoLevel.bind( this ); - this.setFocusedElement = this.setFocusedElement.bind( this ); this.onInput = this.onInput.bind( this ); this.onCompositionEnd = this.onCompositionEnd.bind( this ); this.onSelectionChange = this.onSelectionChange.bind( this ); @@ -152,7 +150,6 @@ export class RichText extends Component { { maxSize: 1 } ); - this.savedContent = value; this.patterns = getPatterns( { onReplace, valueToFormat: this.valueToFormat, @@ -164,6 +161,11 @@ export class RichText extends Component { this.usedDeprecatedChildrenSource = Array.isArray( value ); this.lastHistoryValue = value; + + // Internal values that are update synchronously, unlike props. + this.value = value; + this.selectionStart = selectionStart; + this.selectionEnd = selectionEnd; } componentWillUnmount() { @@ -187,20 +189,15 @@ export class RichText extends Component { } } - setFocusedElement() { - if ( this.props.setFocusedElement ) { - this.props.setFocusedElement( this.props.instanceId ); - } - } - /** * Get the current record (value and selection) from props and state. * * @return {Object} The current record (value and selection). */ getRecord() { - const { formats, replacements, text } = this.formatToValue( this.props.value ); - const { start, end, activeFormats } = this.state; + const { value, selectionStart: start, selectionEnd: end } = this.props; + const { formats, replacements, text } = this.formatToValue( value ); + const { activeFormats } = this.state; return { formats, replacements, text, start, end, activeFormats }; } @@ -375,11 +372,13 @@ export class RichText extends Component { */ onFocus() { const { unstableOnFocus } = this.props; + if ( unstableOnFocus ) { unstableOnFocus(); } this.recalculateBoundaryStyle(); + this.onSelectionChange(); document.addEventListener( 'selectionchange', this.onSelectionChange ); } @@ -420,7 +419,8 @@ export class RichText extends Component { } const value = this.createRecord(); - const { activeFormats = [], start } = this.state; + const { activeFormats = [] } = this.state; + const start = this.selectionStart; // Update the formats between the last and new caret position. const change = updateFormats( { @@ -462,7 +462,7 @@ export class RichText extends Component { const value = this.createRecord(); const { start, end } = value; - if ( start !== this.state.start || end !== this.state.end ) { + if ( start !== this.selectionStart || end !== this.selectionEnd ) { const { isCaretWithinFormattedText } = this.props; const activeFormats = getActiveFormats( value ); @@ -472,8 +472,11 @@ export class RichText extends Component { this.props.onExitFormattedText(); } - this.setState( { start, end, activeFormats } ); + this.setState( { activeFormats } ); this.applyRecord( { ...value, activeFormats }, { domOnly: true } ); + this.props.onSelectionChange( start, end ); + this.selectionStart = start; + this.selectionEnd = end; if ( activeFormats.length > 0 ) { this.recalculateBoundaryStyle(); @@ -517,9 +520,12 @@ export class RichText extends Component { changeHandler( record.formats, record.text ); } ); - this.savedContent = this.valueToFormat( record ); - this.props.onChange( this.savedContent ); - this.setState( { start, end, activeFormats } ); + this.value = this.valueToFormat( record ); + this.props.onChange( this.value ); + this.setState( { activeFormats } ); + this.props.onSelectionChange( start, end ); + this.selectionStart = start; + this.selectionEnd = end; if ( ! withoutHistory ) { this.onCreateUndoLevel(); @@ -528,12 +534,12 @@ export class RichText extends Component { onCreateUndoLevel() { // If the content is the same, no level needs to be created. - if ( this.lastHistoryValue === this.savedContent ) { + if ( this.lastHistoryValue === this.value ) { return; } this.props.onCreateUndoLevel(); - this.lastHistoryValue = this.savedContent; + this.lastHistoryValue = this.value; } /** @@ -804,13 +810,17 @@ export class RichText extends Component { } const newPos = value.start + ( isReverse ? -1 : 1 ); + const newActiveFormats = isReverse ? formatsBefore.length : formatsAfter.length; - this.setState( { start: newPos, end: newPos } ); + this.setState( { selectedFormat: newActiveFormats } ); + this.props.onSelectionChange( newPos, newPos ); + this.selectionStart = newPos; + this.selectionEnd = newPos; this.applyRecord( { ...value, start: newPos, end: newPos, - activeFormats: isReverse ? formatsBefore : formatsAfter, + activeFormats: newActiveFormats, } ); } @@ -889,53 +899,44 @@ export class RichText extends Component { componentDidUpdate( prevProps ) { const { tagName, value, isSelected } = this.props; + const record = this.getRecord(); - if ( + // Check if the content changed. + let shouldReapply = ( tagName === prevProps.tagName && value !== prevProps.value && - value !== this.savedContent - ) { - // Handle deprecated `children` and `node` sources. - // The old way of passing a value with the `node` matcher required - // the value to be mapped first, creating a new array each time, so - // a shallow check wouldn't work. We need to check deep equality. - // This is only executed for a deprecated API and will eventually be - // removed. - if ( Array.isArray( value ) && isEqual( value, this.savedContent ) ) { - return; - } - - const record = this.formatToValue( value ); - - if ( isSelected ) { - const prevRecord = this.formatToValue( prevProps.value ); - const length = getTextContent( prevRecord ).length; - record.start = length; - record.end = length; - } + value !== this.value + ); - this.applyRecord( record ); - this.savedContent = value; - } + // Check if the selection changed. + shouldReapply = shouldReapply || ( + isSelected && ! prevProps.isSelected && ( + this.selectionStart !== record.start || + this.selectionEnd !== record.end + ) + ); const prefix = 'format_prepare_props_'; const predicate = ( v, key ) => key.startsWith( prefix ); const prepareProps = pickBy( this.props, predicate ); const prevPrepareProps = pickBy( prevProps, predicate ); - // If any format prepare props update, reapply value. - if ( ! isShallowEqual( prepareProps, prevPrepareProps ) ) { - const record = this.formatToValue( value ); + // Check if any format props changed. + shouldReapply = shouldReapply || + ! isShallowEqual( prepareProps, prevPrepareProps ); - // Maintain the previous selection if the instance is currently - // selected. - if ( isSelected ) { - record.start = this.state.start; - record.end = this.state.end; + if ( shouldReapply ) { + if ( ! isSelected ) { + delete record.start; + delete record.end; } this.applyRecord( record ); } + + this.value = value; + this.selectionStart = record.start; + this.selectionEnd = record.end; } /** @@ -1053,7 +1054,7 @@ export class RichText extends Component { const record = this.getRecord(); return ( -
+
{ isSelected && this.multilineTag === 'li' && ( { - // When explicitly set as not selected, do nothing. - if ( ownProps.isSelected === false ) { - return { - clientId: context.clientId, - }; - } - // When explicitly set as selected, use the value stored in the context instead. - if ( ownProps.isSelected === true ) { - return { - isSelected: context.isSelected, - clientId: context.clientId, - }; - } - - // Ensures that only one RichText component can be focused. - return { - isSelected: context.isSelected && context.focusedElement === ownProps.instanceId, - setFocusedElement: context.setFocusedElement, - clientId: context.clientId, - }; - } ), - withSelect( ( select ) => { + withBlockEditContext( ( { clientId } ) => ( { clientId } ) ), + withSelect( ( select, { + clientId, + instanceId, + identifier = instanceId, + isSelected, + } ) => { // This should probably be moved to the block editor settings. const { canUserUseUnfilteredHTML } = select( 'core/editor' ); - const { isCaretWithinFormattedText } = select( 'core/block-editor' ); + const { + isCaretWithinFormattedText, + getSelectionStart, + getSelectionEnd, + } = select( 'core/block-editor' ); const { getFormatTypes } = select( 'core/rich-text' ); + const selectionStart = getSelectionStart(); + const selectionEnd = getSelectionEnd(); + + if ( isSelected === undefined ) { + isSelected = ( + selectionStart.clientId === clientId && + selectionStart.attributeKey === identifier + ); + } + return { canUserUseUnfilteredHTML: canUserUseUnfilteredHTML(), isCaretWithinFormattedText: isCaretWithinFormattedText(), formatTypes: getFormatTypes(), + selectionStart: isSelected ? selectionStart.offset : undefined, + selectionEnd: isSelected ? selectionEnd.offset : undefined, + isSelected, }; } ), - withDispatch( ( dispatch ) => { + withDispatch( ( dispatch, { + clientId, + instanceId, + identifier = instanceId, + } ) => { const { __unstableMarkLastChangeAsPersistent, enterFormattedText, exitFormattedText, + selectionChange, } = dispatch( 'core/block-editor' ); return { onCreateUndoLevel: __unstableMarkLastChangeAsPersistent, onEnterFormattedText: enterFormattedText, onExitFormattedText: exitFormattedText, + onSelectionChange( start, end ) { + selectionChange( clientId, identifier, start, end ); + }, }; } ), withSafeTimeout, diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 16910972b1f12b..6915280885ad42 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -597,6 +597,27 @@ export function exitFormattedText() { }; } +/** + * Returns an action object used in signalling that the user caret has changed + * position. + * + * @param {string} clientId The selected block client ID. + * @param {string} attributeKey The selected block attribute key. + * @param {number} startOffset The start offset. + * @param {number} endOffset The end offset. + * + * @return {Object} Action object. + */ +export function selectionChange( clientId, attributeKey, startOffset, endOffset ) { + return { + type: 'SELECTION_CHANGE', + clientId, + attributeKey, + startOffset, + endOffset, + }; +} + /** * Returns an action object used in signalling that a new block of the default * type should be added to the block list. diff --git a/packages/block-editor/src/store/effects.js b/packages/block-editor/src/store/effects.js index d61ce67fe68df8..0f5d359b89f9f0 100644 --- a/packages/block-editor/src/store/effects.js +++ b/packages/block-editor/src/store/effects.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import { findKey } from 'lodash'; + /** * WordPress dependencies */ @@ -9,6 +14,7 @@ import { synchronizeBlocksWithTemplate, } from '@wordpress/blocks'; import { _n, sprintf } from '@wordpress/i18n'; +import { create, toHTMLString, insert, remove } from '@wordpress/rich-text'; /** * Internal dependencies @@ -18,6 +24,7 @@ import { selectBlock, setTemplateValidity, resetBlocks, + selectionChange, } from './actions'; import { getBlock, @@ -26,6 +33,7 @@ import { getTemplateLock, getTemplate, isValidTemplate, + getSelectionStart, } from './selectors'; /** @@ -64,10 +72,10 @@ export default { const state = store.getState(); const [ firstBlockClientId, secondBlockClientId ] = action.blocks; const blockA = getBlock( state, firstBlockClientId ); - const blockType = getBlockType( blockA.name ); + const blockAType = getBlockType( blockA.name ); // Only focus the previous block if it's not mergeable - if ( ! blockType.merge ) { + if ( ! blockAType.merge ) { dispatch( selectBlock( blockA.clientId ) ); return; } @@ -75,6 +83,24 @@ export default { // We can only merge blocks with similar types // thus, we transform the block to merge first const blockB = getBlock( state, secondBlockClientId ); + const blockBType = getBlockType( blockB.name ); + + // A robust way to retain selection position through various transforms + // is to insert a special character at the position and then recover it. + const START_OF_SELECTED_AREA = '\u0086'; + const { attributeKey, offset } = getSelectionStart( state ); + const html = blockB.attributes[ attributeKey ]; + const multilineTagB = blockBType.attributes[ attributeKey ].multiline; + const value = insert( create( { + html, + multilineTag: multilineTagB, + } ), START_OF_SELECTED_AREA, offset, offset ); + + blockB.attributes[ attributeKey ] = toHTMLString( { + value, + multilineTag: multilineTagB, + } ); + const blocksWithTheSameType = blockA.name === blockB.name ? [ blockB ] : switchToBlockType( blockB, blockA.name ); @@ -85,12 +111,30 @@ export default { } // Calling the merge to update the attributes and remove the block to be merged - const updatedAttributes = blockType.merge( + const updatedAttributes = blockAType.merge( blockA.attributes, blocksWithTheSameType[ 0 ].attributes ); - dispatch( selectBlock( blockA.clientId, -1 ) ); + const newAttributeKey = findKey( updatedAttributes, ( v ) => + typeof v === 'string' && v.indexOf( START_OF_SELECTED_AREA ) !== -1 + ); + const convertedHtml = updatedAttributes[ newAttributeKey ]; + const multilineTagA = blockAType.attributes[ newAttributeKey ].multiline; + const convertedValue = create( { html: convertedHtml, multilineTag: multilineTagA } ); + const newOffset = convertedValue.text.indexOf( START_OF_SELECTED_AREA ); + const newValue = remove( convertedValue, newOffset, newOffset + 1 ); + const newHtml = toHTMLString( { value: newValue, multilineTag: multilineTagA } ); + + updatedAttributes[ newAttributeKey ] = newHtml; + + dispatch( selectionChange( + blockA.clientId, + newAttributeKey, + newOffset, + newOffset + ) ); + dispatch( replaceBlocks( [ blockA.clientId, blockB.clientId ], [ diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index 0c7448d1a2a74c..3b2f160fadeba8 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -698,6 +698,15 @@ export function isCaretWithinFormattedText( state = false, action ) { return state; } +const BLOCK_SELECTION_EMPTY_OBJECT = {}; +const BLOCK_SELECTION_INITIAL_STATE = { + start: BLOCK_SELECTION_EMPTY_OBJECT, + end: BLOCK_SELECTION_EMPTY_OBJECT, + isMultiSelecting: false, + isEnabled: true, + initialPosition: null, +}; + /** * Reducer returning the block selection's state. * @@ -706,26 +715,10 @@ export function isCaretWithinFormattedText( state = false, action ) { * * @return {Object} Updated state. */ -export function blockSelection( state = { - start: null, - end: null, - isMultiSelecting: false, - isEnabled: true, - initialPosition: null, -}, action ) { +export function blockSelection( state = BLOCK_SELECTION_INITIAL_STATE, action ) { switch ( action.type ) { case 'CLEAR_SELECTED_BLOCK': - if ( state.start === null && state.end === null && ! state.isMultiSelecting ) { - return state; - } - - return { - ...state, - start: null, - end: null, - isMultiSelecting: false, - initialPosition: null, - }; + return BLOCK_SELECTION_INITIAL_STATE; case 'START_MULTI_SELECT': if ( state.isMultiSelecting ) { return state; @@ -748,71 +741,92 @@ export function blockSelection( state = { }; case 'MULTI_SELECT': return { - ...state, - start: action.start, - end: action.end, - initialPosition: null, + ...BLOCK_SELECTION_INITIAL_STATE, + isMultiSelecting: state.isMultiSelecting, + start: { clientId: action.start }, + end: { clientId: action.end }, }; case 'SELECT_BLOCK': - if ( action.clientId === state.start && action.clientId === state.end ) { + if ( + action.clientId === state.start.clientId && + action.clientId === state.end.clientId + ) { return state; } + return { - ...state, - start: action.clientId, - end: action.clientId, + ...BLOCK_SELECTION_INITIAL_STATE, initialPosition: action.initialPosition, + start: { clientId: action.clientId }, + end: { clientId: action.clientId }, }; case 'REPLACE_INNER_BLOCKS': // REPLACE_INNER_BLOCKS and INSERT_BLOCKS should follow the same logic. case 'INSERT_BLOCKS': { if ( action.updateSelection ) { return { - ...state, - start: action.blocks[ 0 ].clientId, - end: action.blocks[ 0 ].clientId, - initialPosition: null, - isMultiSelecting: false, + ...BLOCK_SELECTION_INITIAL_STATE, + start: { clientId: action.blocks[ 0 ].clientId }, + end: { clientId: action.blocks[ 0 ].clientId }, }; } + return state; } case 'REMOVE_BLOCKS': - if ( ! action.clientIds || ! action.clientIds.length || action.clientIds.indexOf( state.start ) === -1 ) { + if ( + ! action.clientIds || + ! action.clientIds.length || + action.clientIds.indexOf( state.start.clientId ) === -1 + ) { return state; } - return { - ...state, - start: null, - end: null, - initialPosition: null, - isMultiSelecting: false, - }; - case 'REPLACE_BLOCKS': - if ( action.clientIds.indexOf( state.start ) === -1 ) { + + return BLOCK_SELECTION_INITIAL_STATE; + case 'REPLACE_BLOCKS': { + if ( action.clientIds.indexOf( state.start.clientId ) === -1 ) { return state; } // If there are replacement blocks, assign last block as the next // selected block, otherwise set to null. const lastBlock = last( action.blocks ); - const nextSelectedBlockClientId = lastBlock ? lastBlock.clientId : null; - if ( nextSelectedBlockClientId === state.start && nextSelectedBlockClientId === state.end ) { + if ( ! lastBlock ) { + return BLOCK_SELECTION_INITIAL_STATE; + } + + if ( + lastBlock.clientId === state.start.clientId && + lastBlock.clientId === state.end.clientId + ) { return state; } return { - ...state, - start: nextSelectedBlockClientId, - end: nextSelectedBlockClientId, - initialPosition: null, - isMultiSelecting: false, + ...BLOCK_SELECTION_INITIAL_STATE, + start: { clientId: lastBlock.clientId }, + end: { clientId: lastBlock.clientId }, }; + } case 'TOGGLE_SELECTION': return { - ...state, + ...BLOCK_SELECTION_INITIAL_STATE, isEnabled: action.isSelectionEnabled, }; + case 'SELECTION_CHANGE': + return { + ...BLOCK_SELECTION_INITIAL_STATE, + start: { + clientId: action.clientId, + attributeKey: action.attributeKey, + offset: action.startOffset, + }, + end: { + clientId: action.clientId, + attributeKey: action.attributeKey, + offset: action.endOffset, + }, + }; } return state; diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js index d2f26e0df0e35a..be699167f3c6c7 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -318,6 +318,38 @@ export function getBlockCount( state, rootClientId ) { return getBlockOrder( state, rootClientId ).length; } +/** + * @typedef {WPBlockSelection} A block selection object. + * + * @property {string} clientId The selected block client ID. + * @property {string} attributeKey The selected block attribute key. + * @property {number} offset The selected block attribute offset. + */ + +/** + * Returns the current selection start block client ID, attribute key and text + * offset. + * + * @param {Object} state Block editor state. + * + * @return {WPBlockSelection} Selection start information. + */ +export function getSelectionStart( state ) { + return state.blockSelection.start; +} + +/** + * Returns the current selection end block client ID, attribute key and text + * offset. + * + * @param {Object} state Block editor state. + * + * @return {WPBlockSelection} Selection end information. + */ +export function getSelectionEnd( state ) { + return state.blockSelection.end; +} + /** * Returns the current block selection start. This value may be null, and it * may represent either a singular block selection or multi-selection start. @@ -328,7 +360,7 @@ export function getBlockCount( state, rootClientId ) { * @return {?string} Client ID of block selection start. */ export function getBlockSelectionStart( state ) { - return state.blockSelection.start; + return state.blockSelection.start.clientId; } /** @@ -341,7 +373,7 @@ export function getBlockSelectionStart( state ) { * @return {?string} Client ID of block selection end. */ export function getBlockSelectionEnd( state ) { - return state.blockSelection.end; + return state.blockSelection.end.clientId; } /** @@ -358,7 +390,7 @@ export function getSelectedBlockCount( state ) { return multiSelectedBlockCount; } - return state.blockSelection.start ? 1 : 0; + return state.blockSelection.start.clientId ? 1 : 0; } /** @@ -370,7 +402,7 @@ export function getSelectedBlockCount( state ) { */ export function hasSelectedBlock( state ) { const { start, end } = state.blockSelection; - return !! start && start === end; + return !! start.clientId && start.clientId === end.clientId; } /** @@ -386,7 +418,7 @@ export function getSelectedBlockClientId( state ) { // We need to check the block exists because the current blockSelection // reducer doesn't take into account when blocks are reset via undo. To be // removed when that's fixed. - return start && start === end && !! state.blocks.byClientId[ start ] ? start : null; + return start.clientId && start.clientId === end.clientId && !! state.blocks.byClientId[ start.clientId ] ? start.clientId : null; } /** @@ -552,7 +584,8 @@ export function getNextBlockClientId( state, startClientId ) { */ export function getSelectedBlocksInitialCaretPosition( state ) { const { start, end } = state.blockSelection; - if ( start !== end || ! start ) { + + if ( start.clientId !== end.clientId || ! start.clientId ) { return null; } @@ -569,25 +602,26 @@ export function getSelectedBlocksInitialCaretPosition( state ) { export const getSelectedBlockClientIds = createSelector( ( state ) => { const { start, end } = state.blockSelection; - if ( start === null || end === null ) { + + if ( start.clientId === undefined || end.clientId === undefined ) { return EMPTY_ARRAY; } - if ( start === end ) { - return [ start ]; + if ( start.clientId === end.clientId ) { + return [ start.clientId ]; } // Retrieve root client ID to aid in retrieving relevant nested block // order, being careful to allow the falsey empty string top-level root // by explicitly testing against null. - const rootClientId = getBlockRootClientId( state, start ); + const rootClientId = getBlockRootClientId( state, start.clientId ); if ( rootClientId === null ) { return EMPTY_ARRAY; } const blockOrder = getBlockOrder( state, rootClientId ); - const startIndex = blockOrder.indexOf( start ); - const endIndex = blockOrder.indexOf( end ); + const startIndex = blockOrder.indexOf( start.clientId ); + const endIndex = blockOrder.indexOf( end.clientId ); if ( startIndex > endIndex ) { return blockOrder.slice( endIndex, startIndex + 1 ); @@ -597,8 +631,8 @@ export const getSelectedBlockClientIds = createSelector( }, ( state ) => [ state.blocks.order, - state.blockSelection.start, - state.blockSelection.end, + state.blockSelection.start.clientId, + state.blockSelection.end.clientId, ], ); @@ -612,7 +646,8 @@ export const getSelectedBlockClientIds = createSelector( */ export function getMultiSelectedBlockClientIds( state ) { const { start, end } = state.blockSelection; - if ( start === end ) { + + if ( start.clientId === end.clientId ) { return EMPTY_ARRAY; } @@ -741,8 +776,8 @@ export const isAncestorMultiSelected = createSelector( }, ( state ) => [ state.blocks.order, - state.blockSelection.start, - state.blockSelection.end, + state.blockSelection.start.clientId, + state.blockSelection.end.clientId, ], ); /** @@ -759,10 +794,10 @@ export const isAncestorMultiSelected = createSelector( */ export function getMultiSelectedBlocksStartClientId( state ) { const { start, end } = state.blockSelection; - if ( start === end ) { + if ( start.clientId === end.clientId ) { return null; } - return start || null; + return start.clientId || null; } /** @@ -779,10 +814,10 @@ export function getMultiSelectedBlocksStartClientId( state ) { */ export function getMultiSelectedBlocksEndClientId( state ) { const { start, end } = state.blockSelection; - if ( start === end ) { + if ( start.clientId === end.clientId ) { return null; } - return end || null; + return end.clientId || null; } /** @@ -825,11 +860,11 @@ export function getBlockIndex( state, clientId, rootClientId ) { export function isBlockSelected( state, clientId ) { const { start, end } = state.blockSelection; - if ( start !== end ) { + if ( start.clientId !== end.clientId ) { return false; } - return start === clientId; + return start.clientId === clientId; } /** @@ -883,7 +918,7 @@ export function isBlockWithinSelection( state, clientId ) { */ export function hasMultiSelection( state ) { const { start, end } = state.blockSelection; - return start !== end; + return start.clientId !== end.clientId; } /** @@ -964,9 +999,9 @@ export function getBlockInsertionPoint( state ) { } const { end } = blockSelection; - if ( end ) { - rootClientId = getBlockRootClientId( state, end ) || undefined; - index = getBlockIndex( state, end, rootClientId ) + 1; + if ( end.clientId ) { + rootClientId = getBlockRootClientId( state, end.clientId ) || undefined; + index = getBlockIndex( state, end.clientId, rootClientId ) + 1; } else { index = getBlockOrder( state ).length; } diff --git a/packages/block-editor/src/store/test/effects.js b/packages/block-editor/src/store/test/effects.js index cdcf5460775891..b2a0d90dba6d02 100644 --- a/packages/block-editor/src/store/test/effects.js +++ b/packages/block-editor/src/store/test/effects.js @@ -23,6 +23,7 @@ import actions, { replaceBlocks, resetBlocks, selectBlock, + selectionChange, setTemplateValidity, } from '../actions'; import effects, { validateBlocksToTemplate } from '../effects'; @@ -32,7 +33,14 @@ import applyMiddlewares from '../middlewares'; import '../../'; describe( 'effects', () => { - const defaultBlockSettings = { save: () => 'Saved', category: 'common', title: 'block title' }; + const defaultBlockSettings = { + attributes: { + content: {}, + }, + save: () => 'Saved', + category: 'common', + title: 'block title', + }; describe( '.MERGE_BLOCKS', () => { const handler = effects.MERGE_BLOCKS; @@ -69,6 +77,9 @@ describe( 'effects', () => { it( 'should merge the blocks if blocks of the same type', () => { registerBlockType( 'core/test-block', { + attributes: { + content: {}, + }, merge( attributes, attributesToMerge ) { return { content: attributes.content + ' ' + attributesToMerge.content, @@ -92,11 +103,24 @@ describe( 'effects', () => { return blockA.clientId === clientId ? blockA : blockB; }; const dispatch = jest.fn(); - const getState = () => ( {} ); + const getState = () => ( { + blockSelection: { + start: { + clientId: blockB.clientId, + attributeKey: 'content', + offset: 0, + }, + }, + } ); handler( mergeBlocks( blockA.clientId, blockB.clientId ), { dispatch, getState } ); expect( dispatch ).toHaveBeenCalledTimes( 2 ); - expect( dispatch ).toHaveBeenCalledWith( selectBlock( 'chicken', -1 ) ); + expect( dispatch ).toHaveBeenCalledWith( selectionChange( + blockA.clientId, + 'content', + 'chicken'.length + 1, + 'chicken'.length + 1, + ) ); const lastCall = dispatch.mock.calls[ 1 ]; expect( lastCall ).toHaveLength( 1 ); const [ lastCallArgument ] = lastCall; @@ -114,6 +138,9 @@ describe( 'effects', () => { it( 'should not merge the blocks have different types without transformation', () => { registerBlockType( 'core/test-block', { + attributes: { + content: {}, + }, merge( attributes, attributesToMerge ) { return { content: attributes.content + ' ' + attributesToMerge.content, @@ -131,14 +158,22 @@ describe( 'effects', () => { }; const blockB = { clientId: 'ribs', - name: 'core/test-block2', + name: 'core/test-block-2', attributes: { content: 'ribs' }, }; selectors.getBlock = ( state, clientId ) => { return blockA.clientId === clientId ? blockA : blockB; }; const dispatch = jest.fn(); - const getState = () => ( {} ); + const getState = () => ( { + blockSelection: { + start: { + clientId: blockB.clientId, + attributeKey: 'content', + offset: 0, + }, + }, + } ); handler( mergeBlocks( blockA.clientId, blockB.clientId ), { dispatch, getState } ); expect( dispatch ).not.toHaveBeenCalled(); @@ -162,7 +197,7 @@ describe( 'effects', () => { } ); registerBlockType( 'core/test-block-2', { attributes: { - content: { + content2: { type: 'string', }, }, @@ -195,11 +230,24 @@ describe( 'effects', () => { return blockA.clientId === clientId ? blockA : blockB; }; const dispatch = jest.fn(); - const getState = () => ( {} ); + const getState = () => ( { + blockSelection: { + start: { + clientId: blockB.clientId, + attributeKey: 'content2', + offset: 0, + }, + }, + } ); handler( mergeBlocks( blockA.clientId, blockB.clientId ), { dispatch, getState } ); expect( dispatch ).toHaveBeenCalledTimes( 2 ); - // expect( dispatch ).toHaveBeenCalledWith( focusBlock( 'chicken', { offset: -1 } ) ); + expect( dispatch ).toHaveBeenCalledWith( selectionChange( + blockA.clientId, + 'content', + 'chicken'.length + 1, + 'chicken'.length + 1, + ) ); const expectedGenerator = replaceBlocks( [ 'chicken', 'ribs' ], [ { clientId: 'chicken', name: 'core/test-block', diff --git a/packages/block-editor/src/store/test/reducer.js b/packages/block-editor/src/store/test/reducer.js index fe63fc34cd65b1..7543fdad57dd8b 100644 --- a/packages/block-editor/src/store/test/reducer.js +++ b/packages/block-editor/src/store/test/reducer.js @@ -1614,8 +1614,8 @@ describe( 'state', () => { } ); expect( state ).toEqual( { - start: 'kumquat', - end: 'kumquat', + start: { clientId: 'kumquat' }, + end: { clientId: 'kumquat' }, initialPosition: -1, isMultiSelecting: false, isEnabled: true, @@ -1631,10 +1631,11 @@ describe( 'state', () => { } ); expect( state ).toEqual( { - start: 'ribs', - end: 'chicken', + start: { clientId: 'ribs' }, + end: { clientId: 'chicken' }, initialPosition: null, isMultiSelecting: false, + isEnabled: true, } ); } ); @@ -1647,29 +1648,30 @@ describe( 'state', () => { } ); expect( state ).toEqual( { - start: 'ribs', - end: 'chicken', + start: { clientId: 'ribs' }, + end: { clientId: 'chicken' }, initialPosition: null, isMultiSelecting: true, + isEnabled: true, } ); } ); it( 'should start multi selection', () => { - const original = deepFreeze( { start: 'ribs', end: 'ribs', isMultiSelecting: false } ); + const original = deepFreeze( { start: { clientId: 'ribs' }, end: { clientId: 'ribs' }, isMultiSelecting: false } ); const state = blockSelection( original, { type: 'START_MULTI_SELECT', } ); expect( state ).toEqual( { - start: 'ribs', - end: 'ribs', + start: { clientId: 'ribs' }, + end: { clientId: 'ribs' }, initialPosition: null, isMultiSelecting: true, } ); } ); it( 'should return same reference if already multi-selecting', () => { - const original = deepFreeze( { start: 'ribs', end: 'ribs', isMultiSelecting: true } ); + const original = deepFreeze( { start: { clientId: 'ribs' }, end: { clientId: 'ribs' }, isMultiSelecting: true } ); const state = blockSelection( original, { type: 'START_MULTI_SELECT', } ); @@ -1678,21 +1680,21 @@ describe( 'state', () => { } ); it( 'should end multi selection with selection', () => { - const original = deepFreeze( { start: 'ribs', end: 'chicken', isMultiSelecting: true } ); + const original = deepFreeze( { start: { clientId: 'ribs' }, end: { clientId: 'chicken' }, isMultiSelecting: true } ); const state = blockSelection( original, { type: 'STOP_MULTI_SELECT', } ); expect( state ).toEqual( { - start: 'ribs', - end: 'chicken', + start: { clientId: 'ribs' }, + end: { clientId: 'chicken' }, initialPosition: null, isMultiSelecting: false, } ); } ); it( 'should return same reference if already ended multi-selecting', () => { - const original = deepFreeze( { start: 'ribs', end: 'chicken', isMultiSelecting: false } ); + const original = deepFreeze( { start: { clientId: 'ribs' }, end: { clientId: 'chicken' }, isMultiSelecting: false } ); const state = blockSelection( original, { type: 'STOP_MULTI_SELECT', } ); @@ -1701,21 +1703,21 @@ describe( 'state', () => { } ); it( 'should end multi selection without selection', () => { - const original = deepFreeze( { start: 'ribs', end: 'ribs', isMultiSelecting: true } ); + const original = deepFreeze( { start: { clientId: 'ribs' }, end: { clientId: 'ribs' }, isMultiSelecting: true } ); const state = blockSelection( original, { type: 'STOP_MULTI_SELECT', } ); expect( state ).toEqual( { - start: 'ribs', - end: 'ribs', + start: { clientId: 'ribs' }, + end: { clientId: 'ribs' }, initialPosition: null, isMultiSelecting: false, } ); } ); it( 'should not update the state if the block is already selected', () => { - const original = deepFreeze( { start: 'ribs', end: 'ribs' } ); + const original = deepFreeze( { start: { clientId: 'ribs' }, end: { clientId: 'ribs' } } ); const state1 = blockSelection( original, { type: 'SELECT_BLOCK', @@ -1726,22 +1728,23 @@ describe( 'state', () => { } ); it( 'should unset multi selection', () => { - const original = deepFreeze( { start: 'ribs', end: 'chicken' } ); + const original = deepFreeze( { start: { clientId: 'ribs' }, end: { clientId: 'chicken' } } ); const state1 = blockSelection( original, { type: 'CLEAR_SELECTED_BLOCK', } ); expect( state1 ).toEqual( { - start: null, - end: null, + start: {}, + end: {}, initialPosition: null, isMultiSelecting: false, + isEnabled: true, } ); } ); it( 'should return same reference if clearing selection but no selection', () => { - const original = deepFreeze( { start: null, end: null, isMultiSelecting: false } ); + const original = blockSelection( undefined, {} ); const state1 = blockSelection( original, { type: 'CLEAR_SELECTED_BLOCK', @@ -1763,15 +1766,16 @@ describe( 'state', () => { } ); expect( state3 ).toEqual( { - start: 'ribs', - end: 'ribs', + start: { clientId: 'ribs' }, + end: { clientId: 'ribs' }, initialPosition: null, isMultiSelecting: false, + isEnabled: true, } ); } ); it( 'should not select inserted block if updateSelection flag is false', () => { - const original = deepFreeze( { start: 'a', end: 'b' } ); + const original = deepFreeze( { start: { clientId: 'a' }, end: { clientId: 'b' } } ); const state3 = blockSelection( original, { type: 'INSERT_BLOCKS', @@ -1783,13 +1787,13 @@ describe( 'state', () => { } ); expect( state3 ).toEqual( { - start: 'a', - end: 'b', + start: { clientId: 'a' }, + end: { clientId: 'b' }, } ); } ); it( 'should not update the state if the block moved is already selected', () => { - const original = deepFreeze( { start: 'ribs', end: 'ribs' } ); + const original = deepFreeze( { start: { clientId: 'ribs' }, end: { clientId: 'ribs' } } ); const state = blockSelection( original, { type: 'MOVE_BLOCKS_UP', clientIds: [ 'ribs' ], @@ -1799,7 +1803,7 @@ describe( 'state', () => { } ); it( 'should replace the selected block', () => { - const original = deepFreeze( { start: 'chicken', end: 'chicken' } ); + const original = deepFreeze( { start: { clientId: 'chicken' }, end: { clientId: 'chicken' } } ); const state = blockSelection( original, { type: 'REPLACE_BLOCKS', clientIds: [ 'chicken' ], @@ -1810,15 +1814,16 @@ describe( 'state', () => { } ); expect( state ).toEqual( { - start: 'wings', - end: 'wings', + start: { clientId: 'wings' }, + end: { clientId: 'wings' }, initialPosition: null, + isEnabled: true, isMultiSelecting: false, } ); } ); it( 'should not replace the selected block if we keep it at the end when replacing blocks', () => { - const original = deepFreeze( { start: 'wings', end: 'wings' } ); + const original = deepFreeze( { start: { clientId: 'wings' }, end: { clientId: 'wings' } } ); const state = blockSelection( original, { type: 'REPLACE_BLOCKS', clientIds: [ 'wings' ], @@ -1837,7 +1842,7 @@ describe( 'state', () => { } ); it( 'should replace the selected block if we keep it not at the end when replacing blocks', () => { - const original = deepFreeze( { start: 'chicken', end: 'chicken' } ); + const original = deepFreeze( { start: { clientId: 'chicken' }, end: { clientId: 'chicken' } } ); const state = blockSelection( original, { type: 'REPLACE_BLOCKS', clientIds: [ 'chicken' ], @@ -1853,15 +1858,16 @@ describe( 'state', () => { } ); expect( state ).toEqual( { - start: 'wings', - end: 'wings', + start: { clientId: 'wings' }, + end: { clientId: 'wings' }, initialPosition: null, isMultiSelecting: false, + isEnabled: true, } ); } ); it( 'should reset if replacing with empty set', () => { - const original = deepFreeze( { start: 'chicken', end: 'chicken' } ); + const original = deepFreeze( { start: { clientId: 'chicken' }, end: { clientId: 'chicken' } } ); const state = blockSelection( original, { type: 'REPLACE_BLOCKS', clientIds: [ 'chicken' ], @@ -1869,15 +1875,16 @@ describe( 'state', () => { } ); expect( state ).toEqual( { - start: null, - end: null, + start: {}, + end: {}, initialPosition: null, isMultiSelecting: false, + isEnabled: true, } ); } ); it( 'should keep the selected block', () => { - const original = deepFreeze( { start: 'chicken', end: 'chicken' } ); + const original = deepFreeze( { start: { clientId: 'chicken' }, end: { clientId: 'chicken' } } ); const state = blockSelection( original, { type: 'REPLACE_BLOCKS', clientIds: [ 'ribs' ], @@ -1892,8 +1899,8 @@ describe( 'state', () => { it( 'should remove the selection if we are removing the selected block', () => { const original = deepFreeze( { - start: 'chicken', - end: 'chicken', + start: { clientId: 'chicken' }, + end: { clientId: 'chicken' }, initialPosition: null, isMultiSelecting: false, } ); @@ -1903,17 +1910,18 @@ describe( 'state', () => { } ); expect( state ).toEqual( { - start: null, - end: null, + start: {}, + end: {}, initialPosition: null, isMultiSelecting: false, + isEnabled: true, } ); } ); it( 'should keep the selection if we are not removing the selected block', () => { const original = deepFreeze( { - start: 'chicken', - end: 'chicken', + start: { clientId: 'chicken' }, + end: { clientId: 'chicken' }, initialPosition: null, isMultiSelecting: false, } ); @@ -1927,8 +1935,8 @@ describe( 'state', () => { it( 'should update the selection on inner blocks replace if updateSelection is true', () => { const original = deepFreeze( { - start: 'chicken', - end: 'chicken', + start: { clientId: 'chicken' }, + end: { clientId: 'chicken' }, initialPosition: null, isMultiSelecting: false, } ); @@ -1945,17 +1953,18 @@ describe( 'state', () => { } ); expect( state ).toEqual( { - start: 'another-block', - end: 'another-block', + start: { clientId: 'another-block' }, + end: { clientId: 'another-block' }, initialPosition: null, isMultiSelecting: false, + isEnabled: true, } ); } ); it( 'should not update the selection on inner blocks replace if updateSelection is false', () => { const original = deepFreeze( { - start: 'chicken', - end: 'chicken', + start: { clientId: 'chicken' }, + end: { clientId: 'chicken' }, initialPosition: null, isMultiSelecting: false, } ); diff --git a/packages/block-editor/src/store/test/selectors.js b/packages/block-editor/src/store/test/selectors.js index 3bbe1abd7f3a85..c038a40621eff6 100644 --- a/packages/block-editor/src/store/test/selectors.js +++ b/packages/block-editor/src/store/test/selectors.js @@ -741,8 +741,8 @@ describe( 'selectors', () => { it( 'should return false if no selection', () => { const state = { blockSelection: { - start: null, - end: null, + start: {}, + end: {}, }, }; @@ -752,8 +752,8 @@ describe( 'selectors', () => { it( 'should return false if multi-selection', () => { const state = { blockSelection: { - start: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1', - end: '9db792c6-a25a-495d-adbd-97d56a4c4189', + start: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, + end: { clientId: '9db792c6-a25a-495d-adbd-97d56a4c4189' }, }, }; @@ -763,8 +763,8 @@ describe( 'selectors', () => { it( 'should return true if singular selection', () => { const state = { blockSelection: { - start: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1', - end: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1', + start: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, + end: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, }, }; @@ -815,7 +815,7 @@ describe( 'selectors', () => { describe( 'getSelectedBlockClientId', () => { it( 'should return null if no block is selected', () => { const state = { - blockSelection: { start: null, end: null }, + blockSelection: { start: {}, end: {} }, }; expect( getSelectedBlockClientId( state ) ).toBe( null ); @@ -823,7 +823,7 @@ describe( 'selectors', () => { it( 'should return null if there is multi selection', () => { const state = { - blockSelection: { start: 23, end: 123 }, + blockSelection: { start: { clientId: 23 }, end: { clientId: 123 } }, }; expect( getSelectedBlockClientId( state ) ).toBe( null ); @@ -838,7 +838,7 @@ describe( 'selectors', () => { }, }, }, - blockSelection: { start: 23, end: 23 }, + blockSelection: { start: { clientId: 23 }, end: { clientId: 23 } }, }; expect( getSelectedBlockClientId( state ) ).toEqual( 23 ); @@ -863,7 +863,7 @@ describe( 'selectors', () => { 123: [], }, }, - blockSelection: { start: null, end: null }, + blockSelection: { start: {}, end: {} }, }; expect( getSelectedBlock( state ) ).toBe( null ); @@ -886,7 +886,7 @@ describe( 'selectors', () => { 123: [], }, }, - blockSelection: { start: 23, end: 123 }, + blockSelection: { start: { clientId: 23 }, end: { clientId: 123 } }, }; expect( getSelectedBlock( state ) ).toBe( null ); @@ -909,7 +909,7 @@ describe( 'selectors', () => { 123: [], }, }, - blockSelection: { start: 23, end: 23 }, + blockSelection: { start: { clientId: 23 }, end: { clientId: 23 } }, }; expect( getSelectedBlock( state ) ).toEqual( { @@ -993,7 +993,7 @@ describe( 'selectors', () => { '': [ 123, 23 ], }, }, - blockSelection: { start: null, end: null }, + blockSelection: { start: {}, end: {} }, }; expect( getSelectedBlockClientIds( state ) ).toEqual( [] ); @@ -1006,7 +1006,7 @@ describe( 'selectors', () => { '': [ 5, 4, 3, 2, 1 ], }, }, - blockSelection: { start: 2, end: 2 }, + blockSelection: { start: { clientId: 2 }, end: { clientId: 2 } }, }; expect( getSelectedBlockClientIds( state ) ).toEqual( [ 2 ] ); @@ -1019,7 +1019,7 @@ describe( 'selectors', () => { '': [ 5, 4, 3, 2, 1 ], }, }, - blockSelection: { start: 2, end: 4 }, + blockSelection: { start: { clientId: 2 }, end: { clientId: 4 } }, }; expect( getSelectedBlockClientIds( state ) ).toEqual( [ 4, 3, 2 ] ); @@ -1033,7 +1033,7 @@ describe( 'selectors', () => { 4: [ 9, 8, 7, 6 ], }, }, - blockSelection: { start: 7, end: 9 }, + blockSelection: { start: { clientId: 7 }, end: { clientId: 9 } }, }; expect( getSelectedBlockClientIds( state ) ).toEqual( [ 9, 8, 7 ] ); @@ -1048,7 +1048,7 @@ describe( 'selectors', () => { '': [ 123, 23 ], }, }, - blockSelection: { start: null, end: null }, + blockSelection: { start: {}, end: {} }, }; expect( getMultiSelectedBlockClientIds( state ) ).toEqual( [] ); @@ -1061,7 +1061,7 @@ describe( 'selectors', () => { '': [ 5, 4, 3, 2, 1 ], }, }, - blockSelection: { start: 2, end: 4 }, + blockSelection: { start: { clientId: 2 }, end: { clientId: 4 } }, }; expect( getMultiSelectedBlockClientIds( state ) ).toEqual( [ 4, 3, 2 ] ); @@ -1075,7 +1075,7 @@ describe( 'selectors', () => { 4: [ 9, 8, 7, 6 ], }, }, - blockSelection: { start: 7, end: 9 }, + blockSelection: { start: { clientId: 7 }, end: { clientId: 9 } }, }; expect( getMultiSelectedBlockClientIds( state ) ).toEqual( [ 9, 8, 7 ] ); @@ -1090,7 +1090,7 @@ describe( 'selectors', () => { attributes: {}, order: {}, }, - blockSelection: { start: null, end: null }, + blockSelection: { start: {}, end: {} }, }; expect( @@ -1102,7 +1102,7 @@ describe( 'selectors', () => { describe( 'getMultiSelectedBlocksStartClientId', () => { it( 'returns null if there is no multi selection', () => { const state = { - blockSelection: { start: null, end: null }, + blockSelection: { start: {}, end: {} }, }; expect( getMultiSelectedBlocksStartClientId( state ) ).toBeNull(); @@ -1110,7 +1110,7 @@ describe( 'selectors', () => { it( 'returns multi selection start', () => { const state = { - blockSelection: { start: 2, end: 4 }, + blockSelection: { start: { clientId: 2 }, end: { clientId: 4 } }, }; expect( getMultiSelectedBlocksStartClientId( state ) ).toBe( 2 ); @@ -1120,7 +1120,7 @@ describe( 'selectors', () => { describe( 'getMultiSelectedBlocksEndClientId', () => { it( 'returns null if there is no multi selection', () => { const state = { - blockSelection: { start: null, end: null }, + blockSelection: { start: {}, end: {} }, }; expect( getMultiSelectedBlocksEndClientId( state ) ).toBeNull(); @@ -1128,7 +1128,7 @@ describe( 'selectors', () => { it( 'returns multi selection end', () => { const state = { - blockSelection: { start: 2, end: 4 }, + blockSelection: { start: { clientId: 2 }, end: { clientId: 4 } }, }; expect( getMultiSelectedBlocksEndClientId( state ) ).toBe( 4 ); @@ -1296,7 +1296,7 @@ describe( 'selectors', () => { describe( 'isBlockSelected', () => { it( 'should return true if the block is selected', () => { const state = { - blockSelection: { start: 123, end: 123 }, + blockSelection: { start: { clientId: 123 }, end: { clientId: 123 } }, }; expect( isBlockSelected( state, 123 ) ).toBe( true ); @@ -1304,7 +1304,7 @@ describe( 'selectors', () => { it( 'should return false if a multi-selection range exists', () => { const state = { - blockSelection: { start: 123, end: 124 }, + blockSelection: { start: { clientId: 123 }, end: { clientId: 124 } }, }; expect( isBlockSelected( state, 123 ) ).toBe( false ); @@ -1312,7 +1312,7 @@ describe( 'selectors', () => { it( 'should return false if the block is not selected', () => { const state = { - blockSelection: { start: null, end: null }, + blockSelection: { start: {}, end: {} }, }; expect( isBlockSelected( state, 23 ) ).toBe( false ); @@ -1322,7 +1322,7 @@ describe( 'selectors', () => { describe( 'hasSelectedInnerBlock', () => { it( 'should return false if the selected block is a child of the given ClientId', () => { const state = { - blockSelection: { start: 5, end: 5 }, + blockSelection: { start: { clientId: 5 }, end: { clientId: 5 } }, blocks: { order: { 4: [ 3, 2, 1 ], @@ -1335,7 +1335,7 @@ describe( 'selectors', () => { it( 'should return true if the selected block is a child of the given ClientId', () => { const state = { - blockSelection: { start: 3, end: 3 }, + blockSelection: { start: { clientId: 3 }, end: { clientId: 3 } }, blocks: { order: { 4: [ 3, 2, 1 ], @@ -1353,7 +1353,7 @@ describe( 'selectors', () => { 6: [ 5, 4, 3, 2, 1 ], }, }, - blockSelection: { start: 2, end: 4 }, + blockSelection: { start: { clientId: 2 }, end: { clientId: 4 } }, }; expect( hasSelectedInnerBlock( state, 6 ) ).toBe( true ); } ); @@ -1366,7 +1366,7 @@ describe( 'selectors', () => { 6: [ 5, 4 ], }, }, - blockSelection: { start: 5, end: 4 }, + blockSelection: { start: { clientId: 5 }, end: { clientId: 4 } }, }; expect( hasSelectedInnerBlock( state, 3 ) ).toBe( false ); } ); @@ -1375,7 +1375,7 @@ describe( 'selectors', () => { describe( 'isBlockWithinSelection', () => { it( 'should return true if the block is selected but not the last', () => { const state = { - blockSelection: { start: 5, end: 3 }, + blockSelection: { start: { clientId: 5 }, end: { clientId: 3 } }, blocks: { order: { '': [ 5, 4, 3, 2, 1 ], @@ -1388,7 +1388,7 @@ describe( 'selectors', () => { it( 'should return false if the block is the last selected', () => { const state = { - blockSelection: { start: 5, end: 3 }, + blockSelection: { start: { clientId: 5 }, end: { clientId: 3 } }, blocks: { order: { '': [ 5, 4, 3, 2, 1 ], @@ -1401,7 +1401,7 @@ describe( 'selectors', () => { it( 'should return false if the block is not selected', () => { const state = { - blockSelection: { start: 5, end: 3 }, + blockSelection: { start: { clientId: 5 }, end: { clientId: 3 } }, blocks: { order: { '': [ 5, 4, 3, 2, 1 ], @@ -1414,7 +1414,7 @@ describe( 'selectors', () => { it( 'should return false if there is no selection', () => { const state = { - blockSelection: {}, + blockSelection: { start: {}, end: {} }, blocks: { order: { '': [ 5, 4, 3, 2, 1 ], @@ -1430,8 +1430,8 @@ describe( 'selectors', () => { it( 'should return false if no selection', () => { const state = { blockSelection: { - start: null, - end: null, + start: {}, + end: {}, }, }; @@ -1441,8 +1441,8 @@ describe( 'selectors', () => { it( 'should return false if singular selection', () => { const state = { blockSelection: { - start: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1', - end: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1', + start: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, + end: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, }, }; @@ -1452,8 +1452,8 @@ describe( 'selectors', () => { it( 'should return true if multi-selection', () => { const state = { blockSelection: { - start: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1', - end: '9db792c6-a25a-495d-adbd-97d56a4c4189', + start: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, + end: { clientId: '9db792c6-a25a-495d-adbd-97d56a4c4189' }, }, }; @@ -1468,7 +1468,7 @@ describe( 'selectors', () => { '': [ 5, 4, 3, 2, 1 ], }, }, - blockSelection: { start: 2, end: 4 }, + blockSelection: { start: { clientId: 2 }, end: { clientId: 4 } }, }; it( 'should return true if the block is multi selected', () => { @@ -1487,7 +1487,7 @@ describe( 'selectors', () => { '': [ 5, 4, 3, 2, 1 ], }, }, - blockSelection: { start: 2, end: 4 }, + blockSelection: { start: { clientId: 2 }, end: { clientId: 4 } }, }; it( 'should return true if the block is first in multi selection', () => { @@ -1581,8 +1581,8 @@ describe( 'selectors', () => { it( 'should return the explicitly assigned insertion point', () => { const state = { blockSelection: { - start: 'clientId2', - end: 'clientId2', + start: { clientId: 'clientId2' }, + end: { clientId: 'clientId2' }, }, blocks: { byClientId: { @@ -1614,8 +1614,8 @@ describe( 'selectors', () => { it( 'should return an object for the selected block', () => { const state = { blockSelection: { - start: 'clientId1', - end: 'clientId1', + start: { clientId: 'clientId1' }, + end: { clientId: 'clientId1' }, }, blocks: { byClientId: { @@ -1641,8 +1641,8 @@ describe( 'selectors', () => { it( 'should return an object for the nested selected block', () => { const state = { blockSelection: { - start: 'clientId2', - end: 'clientId2', + start: { clientId: 'clientId2' }, + end: { clientId: 'clientId2' }, }, blocks: { byClientId: { @@ -1671,8 +1671,8 @@ describe( 'selectors', () => { it( 'should return an object for the last multi selected clientId', () => { const state = { blockSelection: { - start: 'clientId1', - end: 'clientId2', + start: { clientId: 'clientId1' }, + end: { clientId: 'clientId2' }, }, blocks: { byClientId: { @@ -1701,8 +1701,8 @@ describe( 'selectors', () => { it( 'should return an object for the last block if no selection', () => { const state = { blockSelection: { - start: null, - end: null, + start: {}, + end: {}, }, blocks: { byClientId: { diff --git a/packages/e2e-tests/specs/blocks/__snapshots__/list.test.js.snap b/packages/e2e-tests/specs/blocks/__snapshots__/list.test.js.snap index 92fb70a48a60cc..a7212afc153e6f 100644 --- a/packages/e2e-tests/specs/blocks/__snapshots__/list.test.js.snap +++ b/packages/e2e-tests/specs/blocks/__snapshots__/list.test.js.snap @@ -254,7 +254,7 @@ exports[`List should split into two with paragraph and merge lists 2`] = ` exports[`List should split into two with paragraph and merge lists 3`] = ` " -
  • one
+
  • one
diff --git a/packages/e2e-tests/specs/keyboard-navigable-blocks.test.js b/packages/e2e-tests/specs/keyboard-navigable-blocks.test.js index 0945d3525f8961..5253528a615cd0 100644 --- a/packages/e2e-tests/specs/keyboard-navigable-blocks.test.js +++ b/packages/e2e-tests/specs/keyboard-navigable-blocks.test.js @@ -97,34 +97,6 @@ const tabThroughBlockToolbar = async () => { ); await expect( isFocusedRightAlignmentControl ).toBe( true ); - // Tab to focus on the 'Bold' formatting button - await page.keyboard.press( 'Tab' ); - const isFocusedBoldFormattingButton = await page.evaluate( () => - document.activeElement.classList.contains( 'components-toolbar__control' ) - ); - await expect( isFocusedBoldFormattingButton ).toBe( true ); - - // Tab to focus on the 'Italic' formatting button - await page.keyboard.press( 'Tab' ); - const isFocusedItalicFormattingButton = await page.evaluate( () => - document.activeElement.classList.contains( 'components-toolbar__control' ) - ); - await expect( isFocusedItalicFormattingButton ).toBe( true ); - - // Tab to focus on the 'Hyperlink' formatting button - await page.keyboard.press( 'Tab' ); - const isFocusedHyperlinkFormattingButton = await page.evaluate( () => - document.activeElement.classList.contains( 'components-toolbar__control' ) - ); - await expect( isFocusedHyperlinkFormattingButton ).toBe( true ); - - // Tab to focus on the 'Strikethrough' formatting button - await page.keyboard.press( 'Tab' ); - const isFocusedMoreFormattingDropdown = await page.evaluate( () => - document.activeElement.classList.contains( 'components-dropdown-menu__toggle' ) - ); - await expect( isFocusedMoreFormattingDropdown ).toBe( true ); - // Tab to focus on the 'More formatting' dropdown toggle await page.keyboard.press( 'Tab' ); const isFocusedBlockSettingsDropdown = await page.evaluate( () => diff --git a/packages/rich-text/src/to-dom.js b/packages/rich-text/src/to-dom.js index 33cc08b27e47df..e56711fe13a4e5 100644 --- a/packages/rich-text/src/to-dom.js +++ b/packages/rich-text/src/to-dom.js @@ -270,15 +270,15 @@ export function applySelection( { startPath, endPath }, current ) { range.setStart( startContainer, startOffset ); range.setEnd( endContainer, endOffset ); + // Set back focus if focus is lost. + if ( ownerDocument.activeElement !== current ) { + current.focus(); + } + if ( selection.rangeCount > 0 ) { // If the to be added range and the live range are the same, there's no // need to remove the live range and add the equivalent range. if ( isRangeEqual( range, selection.getRangeAt( 0 ) ) ) { - // Set back focus if focus is lost. - if ( ownerDocument.activeElement !== current ) { - current.focus(); - } - return; }