From 2fb8d84ddad357e78de93e15436f8050d84cab63 Mon Sep 17 00:00:00 2001 From: iseulde Date: Tue, 17 Sep 2019 22:45:12 +0300 Subject: [PATCH 1/3] Block Editor: Simplify Selection Reducer --- .../developers/data/data-core-block-editor.md | 2 +- packages/block-editor/src/store/reducer.js | 230 ++++++----- packages/block-editor/src/store/selectors.js | 116 +++--- .../block-editor/src/store/test/effects.js | 30 +- .../block-editor/src/store/test/reducer.js | 358 ++++++------------ .../block-editor/src/store/test/selectors.js | 167 ++++---- 6 files changed, 405 insertions(+), 498 deletions(-) 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 f6f80cdfec535..eb9201165dafd 100644 --- a/docs/designers-developers/developers/data/data-core-block-editor.md +++ b/docs/designers-developers/developers/data/data-core-block-editor.md @@ -703,7 +703,7 @@ _Returns_ Returns true if the block corresponding to the specified client ID is currently selected but isn't the last of the selected blocks. Here "last" refers to the block sequence in the document, _not_ the sequence of -multi-selection, which is why `state.blockSelection.end` isn't used. +multi-selection, which is why `state.selectionEnd` isn't used. _Parameters_ diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index d35ea11e10129..ff760ff660f77 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -922,108 +922,49 @@ 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. + * General reducer returning the block selection's start and end. * * @param {Object} state Current state. * @param {Object} action Dispatched action. * * @return {Object} Updated state. */ -export function blockSelection( state = BLOCK_SELECTION_INITIAL_STATE, action ) { +export function selection( state = {}, action ) { switch ( action.type ) { - case 'CLEAR_SELECTED_BLOCK': - if ( ! state.start || ! state.start.clientId ) { - return state; - } - - return { - ...state, - start: BLOCK_SELECTION_EMPTY_OBJECT, - end: BLOCK_SELECTION_EMPTY_OBJECT, - isMultiSelecting: false, - initialPosition: null, - }; - case 'START_MULTI_SELECT': - if ( state.isMultiSelecting ) { - return state; - } - - return { - ...state, - isMultiSelecting: true, - initialPosition: null, - }; - case 'STOP_MULTI_SELECT': - if ( ! state.isMultiSelecting ) { - return state; + case 'CLEAR_SELECTED_BLOCK': { + if ( state.clientId ) { + return {}; } - return { - ...state, - isMultiSelecting: false, - initialPosition: null, - }; - case 'MULTI_SELECT': - return { - ...state, - isMultiSelecting: state.isMultiSelecting, - start: { clientId: action.start }, - end: { clientId: action.end }, - }; + return state; + } case 'SELECT_BLOCK': - if ( - action.clientId === state.start.clientId && - action.clientId === state.end.clientId - ) { + if ( action.clientId === state.clientId ) { return state; } - return { - ...state, - initialPosition: action.initialPosition, - start: { clientId: action.clientId }, - end: { clientId: action.clientId }, - }; + return { 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: { clientId: action.blocks[ 0 ].clientId }, - end: { clientId: action.blocks[ 0 ].clientId }, - }; + if ( ! action.updateSelection ) { + return state; } - return state; + return { clientId: action.blocks[ 0 ].clientId }; } case 'REMOVE_BLOCKS': if ( ! action.clientIds || ! action.clientIds.length || - action.clientIds.indexOf( state.start.clientId ) === -1 + action.clientIds.indexOf( state.clientId ) === -1 ) { return state; } - return { - ...state, - start: BLOCK_SELECTION_EMPTY_OBJECT, - end: BLOCK_SELECTION_EMPTY_OBJECT, - isMultiSelecting: false, - initialPosition: null, - }; + return {}; case 'REPLACE_BLOCKS': { - if ( action.clientIds.indexOf( state.start.clientId ) === -1 ) { + if ( action.clientIds.indexOf( state.clientId ) === -1 ) { return state; } @@ -1031,52 +972,127 @@ export function blockSelection( state = BLOCK_SELECTION_INITIAL_STATE, action ) const blockToSelect = action.blocks[ indexToSelect ]; if ( ! blockToSelect ) { - return { - ...state, - start: BLOCK_SELECTION_EMPTY_OBJECT, - end: BLOCK_SELECTION_EMPTY_OBJECT, - isMultiSelecting: false, - initialPosition: null, - }; + return {}; } - if ( - blockToSelect.clientId === state.start.clientId && - blockToSelect.clientId === state.end.clientId - ) { + if ( blockToSelect.clientId === state.clientId ) { return state; } - return { - ...state, - start: { clientId: blockToSelect.clientId }, - end: { clientId: blockToSelect.clientId }, - }; + return { clientId: blockToSelect.clientId }; } - case 'TOGGLE_SELECTION': + } + + return state; +} + +/** + * Reducer returning the block selection's start. + * + * @param {Object} state Current state. + * @param {Object} action Dispatched action. + * + * @return {Object} Updated state. + */ +export function selectionStart( state = {}, action ) { + switch ( action.type ) { + case 'SELECTION_CHANGE': return { - ...state, - isEnabled: action.isSelectionEnabled, + clientId: action.clientId, + attributeKey: action.attributeKey, + offset: action.startOffset, }; + case 'RESET_SELECTION': + return action.selectionStart; + case 'MULTI_SELECT': + return { clientId: action.start }; + } + + return selection( state, action ); +} + +/** + * Reducer returning the block selection's end. + * + * @param {Object} state Current state. + * @param {Object} action Dispatched action. + * + * @return {Object} Updated state. + */ +export function selectionEnd( state = {}, action ) { + switch ( action.type ) { case 'SELECTION_CHANGE': return { - ...state, - start: { - clientId: action.clientId, - attributeKey: action.attributeKey, - offset: action.startOffset, - }, - end: { - clientId: action.clientId, - attributeKey: action.attributeKey, - offset: action.endOffset, - }, + clientId: action.clientId, + attributeKey: action.attributeKey, + offset: action.endOffset, }; + case 'RESET_SELECTION': + return action.selectionEnd; + case 'MULTI_SELECT': + return { clientId: action.end }; + } + + return selection( state, action ); +} + +/** + * Reducer returning whether the user is multi-selecting. + * + * @param {boolean} state Current state. + * @param {Object} action Dispatched action. + * + * @return {boolean} Updated state. + */ +export function isMultiSelecting( state = false, action ) { + switch ( action.type ) { + case 'START_MULTI_SELECT': + return true; + + case 'STOP_MULTI_SELECT': + return false; + } + + return state; +} + +/** + * Reducer returning whether selection is enabled. + * + * @param {boolean} state Current state. + * @param {Object} action Dispatched action. + * + * @return {boolean} Updated state. + */ +export function isSelectionEnabled( state = true, action ) { + switch ( action.type ) { + case 'TOGGLE_SELECTION': + return action.isSelectionEnabled; } return state; } +/** + * Reducer returning the intial block selection. + * + * Currently this in only used to restore the selection after block deletion. + * This reducer should eventually be removed in favour of setting selection + * directly. + * + * @param {boolean} state Current state. + * @param {Object} action Dispatched action. + * + * @return {?number} Initial position: -1 or undefined. + */ +export function initialPosition( state, action ) { + if ( action.type === 'SELECT_BLOCK' ) { + return action.initialPosition; + } else if ( action.type === 'REMOVE_BLOCKS' ) { + return state; + } +} + export function blocksMode( state = {}, action ) { if ( action.type === 'TOGGLE_BLOCK_MODE' ) { const { clientId } = action; @@ -1303,7 +1319,11 @@ export default combineReducers( { blocks, isTyping, isCaretWithinFormattedText, - blockSelection, + selectionStart, + selectionEnd, + isMultiSelecting, + isSelectionEnabled, + initialPosition, blocksMode, blockListSettings, insertionPoint, diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js index 058a69599bb95..d035c3f610c05 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -284,9 +284,9 @@ export function getBlockCount( state, rootClientId ) { /** * @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. + * @property {string} clientId A block client ID. + * @property {string} attributeKey A block attribute key. + * @property {number} offset A block attribute offset. */ /** @@ -298,7 +298,7 @@ export function getBlockCount( state, rootClientId ) { * @return {WPBlockSelection} Selection start information. */ export function getSelectionStart( state ) { - return state.blockSelection.start; + return state.selectionStart; } /** @@ -310,7 +310,7 @@ export function getSelectionStart( state ) { * @return {WPBlockSelection} Selection end information. */ export function getSelectionEnd( state ) { - return state.blockSelection.end; + return state.selectionEnd; } /** @@ -323,7 +323,7 @@ export function getSelectionEnd( state ) { * @return {?string} Client ID of block selection start. */ export function getBlockSelectionStart( state ) { - return state.blockSelection.start.clientId; + return state.selectionStart.clientId; } /** @@ -336,7 +336,7 @@ export function getBlockSelectionStart( state ) { * @return {?string} Client ID of block selection end. */ export function getBlockSelectionEnd( state ) { - return state.blockSelection.end.clientId; + return state.selectionEnd.clientId; } /** @@ -353,7 +353,7 @@ export function getSelectedBlockCount( state ) { return multiSelectedBlockCount; } - return state.blockSelection.start.clientId ? 1 : 0; + return state.selectionStart.clientId ? 1 : 0; } /** @@ -364,8 +364,11 @@ export function getSelectedBlockCount( state ) { * @return {boolean} Whether a single block is selected. */ export function hasSelectedBlock( state ) { - const { start, end } = state.blockSelection; - return !! start.clientId && start.clientId === end.clientId; + const { selectionStart, selectionEnd } = state; + return ( + !! selectionStart.clientId && + selectionStart.clientId === selectionEnd.clientId + ); } /** @@ -377,11 +380,14 @@ export function hasSelectedBlock( state ) { * @return {?string} Selected block client ID. */ export function getSelectedBlockClientId( state ) { - const { start, end } = state.blockSelection; - // 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.clientId && start.clientId === end.clientId && !! state.blocks.byClientId[ start.clientId ] ? start.clientId : null; + const { selectionStart, selectionEnd } = state; + const { clientId } = selectionStart; + + if ( ! clientId || clientId !== selectionEnd.clientId ) { + return null; + } + + return clientId; } /** @@ -529,13 +535,7 @@ export function getNextBlockClientId( state, startClientId ) { * @return {?Object} Selected block. */ export function getSelectedBlocksInitialCaretPosition( state ) { - const { start, end } = state.blockSelection; - - if ( start.clientId !== end.clientId || ! start.clientId ) { - return null; - } - - return state.blockSelection.initialPosition; + return state.initialPosition; } /** @@ -547,27 +547,30 @@ export function getSelectedBlocksInitialCaretPosition( state ) { */ export const getSelectedBlockClientIds = createSelector( ( state ) => { - const { start, end } = state.blockSelection; + const { selectionStart, selectionEnd } = state; - if ( start.clientId === undefined || end.clientId === undefined ) { + if ( + selectionStart.clientId === undefined || + selectionEnd.clientId === undefined + ) { return EMPTY_ARRAY; } - if ( start.clientId === end.clientId ) { - return [ start.clientId ]; + if ( selectionStart.clientId === selectionEnd.clientId ) { + return [ selectionStart.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.clientId ); + const rootClientId = getBlockRootClientId( state, selectionStart.clientId ); if ( rootClientId === null ) { return EMPTY_ARRAY; } const blockOrder = getBlockOrder( state, rootClientId ); - const startIndex = blockOrder.indexOf( start.clientId ); - const endIndex = blockOrder.indexOf( end.clientId ); + const startIndex = blockOrder.indexOf( selectionStart.clientId ); + const endIndex = blockOrder.indexOf( selectionEnd.clientId ); if ( startIndex > endIndex ) { return blockOrder.slice( endIndex, startIndex + 1 ); @@ -577,8 +580,8 @@ export const getSelectedBlockClientIds = createSelector( }, ( state ) => [ state.blocks.order, - state.blockSelection.start.clientId, - state.blockSelection.end.clientId, + state.selectionStart.clientId, + state.selectionEnd.clientId, ], ); @@ -591,9 +594,9 @@ export const getSelectedBlockClientIds = createSelector( * @return {Array} Multi-selected block client IDs. */ export function getMultiSelectedBlockClientIds( state ) { - const { start, end } = state.blockSelection; + const { selectionStart, selectionEnd } = state; - if ( start.clientId === end.clientId ) { + if ( selectionStart.clientId === selectionEnd.clientId ) { return EMPTY_ARRAY; } @@ -698,8 +701,8 @@ export const isAncestorMultiSelected = createSelector( }, ( state ) => [ state.blocks.order, - state.blockSelection.start.clientId, - state.blockSelection.end.clientId, + state.selectionStart.clientId, + state.selectionEnd.clientId, ], ); /** @@ -715,11 +718,13 @@ export const isAncestorMultiSelected = createSelector( * @return {?string} Client ID of block beginning multi-selection. */ export function getMultiSelectedBlocksStartClientId( state ) { - const { start, end } = state.blockSelection; - if ( start.clientId === end.clientId ) { + const { selectionStart, selectionEnd } = state; + + if ( selectionStart.clientId === selectionEnd.clientId ) { return null; } - return start.clientId || null; + + return selectionStart.clientId || null; } /** @@ -735,11 +740,13 @@ export function getMultiSelectedBlocksStartClientId( state ) { * @return {?string} Client ID of block ending multi-selection. */ export function getMultiSelectedBlocksEndClientId( state ) { - const { start, end } = state.blockSelection; - if ( start.clientId === end.clientId ) { + const { selectionStart, selectionEnd } = state; + + if ( selectionStart.clientId === selectionEnd.clientId ) { return null; } - return end.clientId || null; + + return selectionEnd.clientId || null; } /** @@ -780,13 +787,13 @@ export function getBlockIndex( state, clientId, rootClientId ) { * @return {boolean} Whether block is selected and multi-selection exists. */ export function isBlockSelected( state, clientId ) { - const { start, end } = state.blockSelection; + const { selectionStart, selectionEnd } = state; - if ( start.clientId !== end.clientId ) { + if ( selectionStart.clientId !== selectionEnd.clientId ) { return false; } - return start.clientId === clientId; + return selectionStart.clientId === clientId; } /** @@ -813,7 +820,7 @@ export function hasSelectedInnerBlock( state, clientId, deep = false ) { * Returns true if the block corresponding to the specified client ID is * currently selected but isn't the last of the selected blocks. Here "last" * refers to the block sequence in the document, _not_ the sequence of - * multi-selection, which is why `state.blockSelection.end` isn't used. + * multi-selection, which is why `state.selectionEnd` isn't used. * * @param {Object} state Editor state. * @param {string} clientId Block client ID. @@ -839,8 +846,8 @@ export function isBlockWithinSelection( state, clientId ) { * @return {boolean} Whether multi-selection has been made. */ export function hasMultiSelection( state ) { - const { start, end } = state.blockSelection; - return start.clientId !== end.clientId; + const { selectionStart, selectionEnd } = state; + return selectionStart.clientId !== selectionEnd.clientId; } /** @@ -855,7 +862,7 @@ export function hasMultiSelection( state ) { * @return {boolean} True if multi-selecting, false if not. */ export function isMultiSelecting( state ) { - return state.blockSelection.isMultiSelecting; + return state.isMultiSelecting; } /** @@ -866,7 +873,7 @@ export function isMultiSelecting( state ) { * @return {boolean} True if it should be possible to multi-select blocks, false if multi-selection is disabled. */ export function isSelectionEnabled( state ) { - return state.blockSelection.isEnabled; + return state.isSelectionEnabled; } /** @@ -915,15 +922,16 @@ export function isCaretWithinFormattedText( state ) { export function getBlockInsertionPoint( state ) { let rootClientId, index; - const { insertionPoint, blockSelection } = state; + const { insertionPoint, selectionEnd } = state; if ( insertionPoint !== null ) { return insertionPoint; } - const { end } = blockSelection; - if ( end.clientId ) { - rootClientId = getBlockRootClientId( state, end.clientId ) || undefined; - index = getBlockIndex( state, end.clientId, rootClientId ) + 1; + const { clientId } = selectionEnd; + + if ( clientId ) { + rootClientId = getBlockRootClientId( state, clientId ) || undefined; + index = getBlockIndex( state, selectionEnd.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 827dde0d39b5c..fbb93ddfd7f13 100644 --- a/packages/block-editor/src/store/test/effects.js +++ b/packages/block-editor/src/store/test/effects.js @@ -107,12 +107,10 @@ describe( 'effects', () => { }; const dispatch = jest.fn(); const getState = () => ( { - blockSelection: { - start: { - clientId: blockB.clientId, - attributeKey: 'content', - offset: 0, - }, + selectionStart: { + clientId: blockB.clientId, + attributeKey: 'content', + offset: 0, }, } ); handler( mergeBlocks( blockA.clientId, blockB.clientId ), { dispatch, getState } ); @@ -171,12 +169,10 @@ describe( 'effects', () => { }; const dispatch = jest.fn(); const getState = () => ( { - blockSelection: { - start: { - clientId: blockB.clientId, - attributeKey: 'content', - offset: 0, - }, + selectionStart: { + clientId: blockB.clientId, + attributeKey: 'content', + offset: 0, }, } ); handler( mergeBlocks( blockA.clientId, blockB.clientId ), { dispatch, getState } ); @@ -238,12 +234,10 @@ describe( 'effects', () => { }; const dispatch = jest.fn(); const getState = () => ( { - blockSelection: { - start: { - clientId: blockB.clientId, - attributeKey: 'content2', - offset: 0, - }, + selectionStart: { + clientId: blockB.clientId, + attributeKey: 'content2', + offset: 0, }, } ); handler( mergeBlocks( blockA.clientId, blockB.clientId ), { dispatch, getState } ); diff --git a/packages/block-editor/src/store/test/reducer.js b/packages/block-editor/src/store/test/reducer.js index 6eb56576f71b2..2f3688e6ca748 100644 --- a/packages/block-editor/src/store/test/reducer.js +++ b/packages/block-editor/src/store/test/reducer.js @@ -22,7 +22,11 @@ import { blocks, isTyping, isCaretWithinFormattedText, - blockSelection, + selection, + selectionStart, + selectionEnd, + initialPosition, + isMultiSelecting, preferences, blocksMode, insertionPoint, @@ -1786,196 +1790,150 @@ describe( 'state', () => { } ); } ); - describe( 'blockSelection()', () => { + describe( 'initialPosition()', () => { it( 'should return with block clientId as selected', () => { - const state = blockSelection( undefined, { + const state = initialPosition( undefined, { type: 'SELECT_BLOCK', - clientId: 'kumquat', initialPosition: -1, } ); - expect( state ).toEqual( { - start: { clientId: 'kumquat' }, - end: { clientId: 'kumquat' }, - initialPosition: -1, - isMultiSelecting: false, - isEnabled: true, - } ); + expect( state ).toBe( -1 ); } ); + } ); - it( 'should set multi selection', () => { - const original = deepFreeze( { isMultiSelecting: false, initialPosition: null, isEnabled: true } ); - const state = blockSelection( original, { - type: 'MULTI_SELECT', - start: 'ribs', - end: 'chicken', + describe( 'isMultiSelecting()', () => { + it( 'should start multi selection', () => { + const state = isMultiSelecting( false, { + type: 'START_MULTI_SELECT', } ); - expect( state ).toEqual( { - start: { clientId: 'ribs' }, - end: { clientId: 'chicken' }, - initialPosition: null, - isMultiSelecting: false, - isEnabled: true, - } ); + expect( state ).toBe( true ); } ); - it( 'should set continuous multi selection', () => { - const original = deepFreeze( { isMultiSelecting: true, initialPosition: null, isEnabled: true } ); - const state = blockSelection( original, { - type: 'MULTI_SELECT', - start: 'ribs', - end: 'chicken', + it( 'should end multi selection with selection', () => { + const state = isMultiSelecting( true, { + type: 'STOP_MULTI_SELECT', } ); - expect( state ).toEqual( { - start: { clientId: 'ribs' }, - end: { clientId: 'chicken' }, - initialPosition: null, - isMultiSelecting: true, - isEnabled: true, - } ); + expect( state ).toBe( false ); } ); + } ); - it( 'should start multi selection', () => { - const original = deepFreeze( { start: { clientId: 'ribs' }, end: { clientId: 'ribs' }, isMultiSelecting: false } ); - const state = blockSelection( original, { - type: 'START_MULTI_SELECT', - } ); + describe( 'selectionStart() and selectionEnd()', () => { + it( 'should set multi selection', () => { + const action = { + type: 'MULTI_SELECT', + start: 'ribs', + end: 'chicken', + }; + const state1 = selectionStart( undefined, action ); + const state2 = selectionEnd( undefined, action ); - expect( state ).toEqual( { - start: { clientId: 'ribs' }, - end: { clientId: 'ribs' }, - initialPosition: null, - isMultiSelecting: true, - } ); + expect( state1 ).toEqual( { clientId: 'ribs' } ); + expect( state2 ).toEqual( { clientId: 'chicken' } ); } ); - it( 'should return same reference if already multi-selecting', () => { - const original = deepFreeze( { start: { clientId: 'ribs' }, end: { clientId: 'ribs' }, isMultiSelecting: true } ); - const state = blockSelection( original, { - type: 'START_MULTI_SELECT', - } ); + it( 'should reset selection', () => { + const start = { clientId: 'a' }; + const end = { clientId: 'b' }; + const action = { + type: 'RESET_SELECTION', + selectionStart: start, + selectionEnd: end, + }; + const state1 = selectionStart( undefined, action ); + const state2 = selectionEnd( undefined, action ); - expect( state ).toBe( original ); + expect( state1 ).toEqual( start ); + expect( state2 ).toEqual( end ); } ); - it( 'should end multi selection with selection', () => { - const original = deepFreeze( { start: { clientId: 'ribs' }, end: { clientId: 'chicken' }, isMultiSelecting: true } ); - const state = blockSelection( original, { - type: 'STOP_MULTI_SELECT', - } ); + it( 'should change selection', () => { + const action = { + type: 'SELECTION_CHANGE', + clientId: 'a', + attributeKey: 'content', + startOffset: 1, + endOffset: 2, + }; + const state1 = selectionStart( undefined, action ); + const state2 = selectionEnd( undefined, action ); - expect( state ).toEqual( { - start: { clientId: 'ribs' }, - end: { clientId: 'chicken' }, - initialPosition: null, - isMultiSelecting: false, + expect( state1 ).toEqual( { + clientId: 'a', + attributeKey: 'content', + offset: 1, } ); - } ); - - it( 'should return same reference if already ended multi-selecting', () => { - const original = deepFreeze( { start: { clientId: 'ribs' }, end: { clientId: 'chicken' }, isMultiSelecting: false } ); - const state = blockSelection( original, { - type: 'STOP_MULTI_SELECT', + expect( state2 ).toEqual( { + clientId: 'a', + attributeKey: 'content', + offset: 2, } ); - - expect( state ).toBe( original ); } ); + } ); - it( 'should end multi selection without selection', () => { - const original = deepFreeze( { start: { clientId: 'ribs' }, end: { clientId: 'ribs' }, isMultiSelecting: true } ); - const state = blockSelection( original, { - type: 'STOP_MULTI_SELECT', + describe( 'selection()', () => { + it( 'should return with block clientId as selected', () => { + const state = selection( undefined, { + type: 'SELECT_BLOCK', + clientId: 'kumquat', } ); - expect( state ).toEqual( { - start: { clientId: 'ribs' }, - end: { clientId: 'ribs' }, - initialPosition: null, - isMultiSelecting: false, - } ); + expect( state ).toEqual( { clientId: 'kumquat' } ); } ); it( 'should not update the state if the block is already selected', () => { - const original = deepFreeze( { start: { clientId: 'ribs' }, end: { clientId: 'ribs' } } ); - - const state1 = blockSelection( original, { + const original = deepFreeze( { clientId: 'ribs' } ); + const state = selection( original, { type: 'SELECT_BLOCK', clientId: 'ribs', } ); - expect( state1 ).toBe( original ); + expect( state ).toBe( original ); } ); - it( 'should unset multi selection', () => { - const original = deepFreeze( { start: { clientId: 'ribs' }, end: { clientId: 'chicken' }, isEnabled: true } ); - - const state1 = blockSelection( original, { + it( 'should unset selection', () => { + const original = deepFreeze( { clientId: 'ribs' } ); + const state = selection( original, { type: 'CLEAR_SELECTED_BLOCK', } ); - expect( state1 ).toEqual( { - start: {}, - end: {}, - initialPosition: null, - isMultiSelecting: false, - isEnabled: true, - } ); + expect( state ).toEqual( {} ); } ); it( 'should return same reference if clearing selection but no selection', () => { - const original = blockSelection( undefined, {} ); - - const state1 = blockSelection( original, { + const original = selection( undefined, {} ); + const state = selection( original, { type: 'CLEAR_SELECTED_BLOCK', } ); - expect( state1 ).toBe( original ); + expect( state ).toBe( original ); } ); it( 'should select inserted block', () => { - const original = deepFreeze( { start: 'ribs', end: 'chicken', initialPosition: null, isEnabled: true, isMultiSelecting: false } ); - - const state3 = blockSelection( original, { + const state = selection( undefined, { type: 'INSERT_BLOCKS', - blocks: [ { - clientId: 'ribs', - name: 'core/freeform', - } ], + blocks: [ { clientId: 'ribs' } ], updateSelection: true, } ); - expect( state3 ).toEqual( { - start: { clientId: 'ribs' }, - end: { clientId: 'ribs' }, - initialPosition: null, - isMultiSelecting: false, - isEnabled: true, - } ); + expect( state ).toEqual( { clientId: 'ribs' } ); } ); it( 'should not select inserted block if updateSelection flag is false', () => { - const original = deepFreeze( { start: { clientId: 'a' }, end: { clientId: 'b' } } ); - - const state3 = blockSelection( original, { + const original = deepFreeze( { clientId: 'a' } ); + const state = selection( original, { type: 'INSERT_BLOCKS', - blocks: [ { - clientId: 'ribs', - name: 'core/freeform', - } ], + blocks: [ { clientId: 'ribs' } ], updateSelection: false, } ); - expect( state3 ).toEqual( { - start: { clientId: 'a' }, - end: { clientId: 'b' }, - } ); + expect( state ).toEqual( original ); } ); it( 'should not update the state if the block moved is already selected', () => { - const original = deepFreeze( { start: { clientId: 'ribs' }, end: { clientId: 'ribs' } } ); - const state = blockSelection( original, { + const original = deepFreeze( { clientId: 'ribs' } ); + const state = selection( original, { type: 'MOVE_BLOCKS_UP', clientIds: [ 'ribs' ], } ); @@ -1984,130 +1942,79 @@ describe( 'state', () => { } ); it( 'should replace the selected block', () => { - const original = deepFreeze( { start: { clientId: 'chicken' }, end: { clientId: 'chicken' }, initialPosition: null, isEnabled: true, isMultiSelecting: false } ); - const state = blockSelection( original, { + const original = deepFreeze( { clientId: 'chicken' } ); + const state = selection( original, { type: 'REPLACE_BLOCKS', clientIds: [ 'chicken' ], - blocks: [ { - clientId: 'wings', - name: 'core/freeform', - } ], + blocks: [ { clientId: 'wings' } ], } ); - expect( state ).toEqual( { - start: { clientId: 'wings' }, - end: { clientId: 'wings' }, - initialPosition: null, - isEnabled: true, - isMultiSelecting: false, - } ); + expect( state ).toEqual( { clientId: 'wings' } ); } ); it( 'should not replace the selected block if we keep it at the end when replacing blocks', () => { - const original = deepFreeze( { start: { clientId: 'wings' }, end: { clientId: 'wings' } } ); - const state = blockSelection( original, { + const original = deepFreeze( { clientId: 'wings' } ); + const state = selection( original, { type: 'REPLACE_BLOCKS', clientIds: [ 'wings' ], blocks: [ - { - clientId: 'chicken', - name: 'core/freeform', - }, - { - clientId: 'wings', - name: 'core/freeform', - } ], + { clientId: 'chicken' }, + { clientId: 'wings' }, + ], } ); expect( state ).toBe( original ); } ); it( 'should replace the selected block if we keep it not at the end when replacing blocks', () => { - const original = deepFreeze( { start: { clientId: 'chicken' }, end: { clientId: 'chicken' }, initialPosition: null, isEnabled: true, isMultiSelecting: false } ); - const state = blockSelection( original, { + const original = deepFreeze( { clientId: 'chicken' } ); + const state = selection( original, { type: 'REPLACE_BLOCKS', clientIds: [ 'chicken' ], blocks: [ - { - clientId: 'chicken', - name: 'core/freeform', - }, - { - clientId: 'wings', - name: 'core/freeform', - } ], + { clientId: 'chicken' }, + { clientId: 'wings' }, + ], } ); - expect( state ).toEqual( { - start: { clientId: 'wings' }, - end: { clientId: 'wings' }, - initialPosition: null, - isMultiSelecting: false, - isEnabled: true, - } ); + expect( state ).toEqual( { clientId: 'wings' } ); } ); it( 'should reset if replacing with empty set', () => { - const original = deepFreeze( { start: { clientId: 'chicken' }, end: { clientId: 'chicken' }, isEnabled: true } ); - const state = blockSelection( original, { + const original = deepFreeze( { clientId: 'chicken' } ); + const state = selection( original, { type: 'REPLACE_BLOCKS', clientIds: [ 'chicken' ], blocks: [], } ); - expect( state ).toEqual( { - start: {}, - end: {}, - initialPosition: null, - isMultiSelecting: false, - isEnabled: true, - } ); + expect( state ).toEqual( {} ); } ); it( 'should keep the selected block', () => { - const original = deepFreeze( { start: { clientId: 'chicken' }, end: { clientId: 'chicken' } } ); - const state = blockSelection( original, { + const original = deepFreeze( { clientId: 'chicken' } ); + const state = selection( original, { type: 'REPLACE_BLOCKS', clientIds: [ 'ribs' ], - blocks: [ { - clientId: 'wings', - name: 'core/freeform', - } ], + blocks: [ { clientId: 'wings' } ], } ); expect( state ).toBe( original ); } ); it( 'should remove the selection if we are removing the selected block', () => { - const original = deepFreeze( { - start: { clientId: 'chicken' }, - end: { clientId: 'chicken' }, - initialPosition: null, - isMultiSelecting: false, - isEnabled: true, - } ); - const state = blockSelection( original, { + const original = deepFreeze( { clientId: 'chicken' } ); + const state = selection( original, { type: 'REMOVE_BLOCKS', clientIds: [ 'chicken' ], } ); - expect( state ).toEqual( { - start: {}, - end: {}, - initialPosition: null, - isMultiSelecting: false, - isEnabled: true, - } ); + expect( state ).toEqual( {} ); } ); it( 'should keep the selection if we are not removing the selected block', () => { - const original = deepFreeze( { - start: { clientId: 'chicken' }, - end: { clientId: 'chicken' }, - initialPosition: null, - isMultiSelecting: false, - } ); - const state = blockSelection( original, { + const original = deepFreeze( { clientId: 'chicken' } ); + const state = selection( original, { type: 'REMOVE_BLOCKS', clientIds: [ 'ribs' ], } ); @@ -2116,49 +2023,22 @@ describe( 'state', () => { } ); it( 'should update the selection on inner blocks replace if updateSelection is true', () => { - const original = deepFreeze( { - start: { clientId: 'chicken' }, - end: { clientId: 'chicken' }, - initialPosition: null, - isMultiSelecting: false, - isEnabled: true, - } ); - const newBlock = { - name: 'core/test-block', - clientId: 'another-block', - }; - - const state = blockSelection( original, { + const original = deepFreeze( { clientId: 'chicken' } ); + const state = selection( original, { type: 'REPLACE_INNER_BLOCKS', - blocks: [ newBlock ], + blocks: [ { clientId: 'another-block' } ], rootClientId: 'parent', updateSelection: true, } ); - expect( state ).toEqual( { - start: { clientId: 'another-block' }, - end: { clientId: 'another-block' }, - initialPosition: null, - isMultiSelecting: false, - isEnabled: true, - } ); + expect( state ).toEqual( { clientId: 'another-block' } ); } ); it( 'should not update the selection on inner blocks replace if updateSelection is false', () => { - const original = deepFreeze( { - start: { clientId: 'chicken' }, - end: { clientId: 'chicken' }, - initialPosition: null, - isMultiSelecting: false, - } ); - const newBlock = { - name: 'core/test-block', - clientId: 'another-block', - }; - - const state = blockSelection( original, { + const original = deepFreeze( { clientId: 'chicken' } ); + const state = selection( original, { type: 'REPLACE_INNER_BLOCKS', - blocks: [ newBlock ], + blocks: [ { clientId: 'another-block' } ], rootClientId: 'parent', updateSelection: false, } ); diff --git a/packages/block-editor/src/store/test/selectors.js b/packages/block-editor/src/store/test/selectors.js index 22ee0ed1adef3..6000c76b824f0 100644 --- a/packages/block-editor/src/store/test/selectors.js +++ b/packages/block-editor/src/store/test/selectors.js @@ -524,10 +524,8 @@ describe( 'selectors', () => { describe( 'hasSelectedBlock', () => { it( 'should return false if no selection', () => { const state = { - blockSelection: { - start: {}, - end: {}, - }, + selectionStart: {}, + selectionEnd: {}, }; expect( hasSelectedBlock( state ) ).toBe( false ); @@ -535,10 +533,8 @@ describe( 'selectors', () => { it( 'should return false if multi-selection', () => { const state = { - blockSelection: { - start: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, - end: { clientId: '9db792c6-a25a-495d-adbd-97d56a4c4189' }, - }, + selectionStart: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, + selectionEnd: { clientId: '9db792c6-a25a-495d-adbd-97d56a4c4189' }, }; expect( hasSelectedBlock( state ) ).toBe( false ); @@ -546,10 +542,8 @@ describe( 'selectors', () => { it( 'should return true if singular selection', () => { const state = { - blockSelection: { - start: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, - end: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, - }, + selectionStart: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, + selectionEnd: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, }; expect( hasSelectedBlock( state ) ).toBe( true ); @@ -604,7 +598,8 @@ describe( 'selectors', () => { describe( 'getSelectedBlockClientId', () => { it( 'should return null if no block is selected', () => { const state = { - blockSelection: { start: {}, end: {} }, + selectionStart: {}, + selectionEnd: {}, }; expect( getSelectedBlockClientId( state ) ).toBe( null ); @@ -612,7 +607,8 @@ describe( 'selectors', () => { it( 'should return null if there is multi selection', () => { const state = { - blockSelection: { start: { clientId: 23 }, end: { clientId: 123 } }, + selectionStart: { clientId: 23 }, + selectionEnd: { clientId: 123 }, }; expect( getSelectedBlockClientId( state ) ).toBe( null ); @@ -627,7 +623,8 @@ describe( 'selectors', () => { }, }, }, - blockSelection: { start: { clientId: 23 }, end: { clientId: 23 } }, + selectionStart: { clientId: 23 }, + selectionEnd: { clientId: 23 }, }; expect( getSelectedBlockClientId( state ) ).toEqual( 23 ); @@ -656,7 +653,8 @@ describe( 'selectors', () => { 123: '', }, }, - blockSelection: { start: {}, end: {} }, + selectionStart: {}, + selectionEnd: {}, }; expect( getSelectedBlock( state ) ).toBe( null ); @@ -683,7 +681,8 @@ describe( 'selectors', () => { 23: '', }, }, - blockSelection: { start: { clientId: 23 }, end: { clientId: 123 } }, + selectionStart: { clientId: 23 }, + selectionEnd: { clientId: 123 }, }; expect( getSelectedBlock( state ) ).toBe( null ); @@ -713,7 +712,8 @@ describe( 'selectors', () => { 23: {}, }, }, - blockSelection: { start: { clientId: 23 }, end: { clientId: 23 } }, + selectionStart: { clientId: 23 }, + selectionEnd: { clientId: 23 }, }; expect( getSelectedBlock( state ) ).toEqual( { @@ -822,7 +822,8 @@ describe( 'selectors', () => { 23: '', }, }, - blockSelection: { start: {}, end: {} }, + selectionStart: {}, + selectionEnd: {}, }; expect( getSelectedBlockClientIds( state ) ).toEqual( [] ); @@ -842,7 +843,8 @@ describe( 'selectors', () => { 5: '', }, }, - blockSelection: { start: { clientId: 2 }, end: { clientId: 2 } }, + selectionStart: { clientId: 2 }, + selectionEnd: { clientId: 2 }, }; expect( getSelectedBlockClientIds( state ) ).toEqual( [ 2 ] ); @@ -862,7 +864,8 @@ describe( 'selectors', () => { 5: '', }, }, - blockSelection: { start: { clientId: 2 }, end: { clientId: 4 } }, + selectionStart: { clientId: 2 }, + selectionEnd: { clientId: 4 }, }; expect( getSelectedBlockClientIds( state ) ).toEqual( [ 4, 3, 2 ] ); @@ -887,7 +890,8 @@ describe( 'selectors', () => { 9: 4, }, }, - blockSelection: { start: { clientId: 7 }, end: { clientId: 9 } }, + selectionStart: { clientId: 7 }, + selectionEnd: { clientId: 9 }, }; expect( getSelectedBlockClientIds( state ) ).toEqual( [ 9, 8, 7 ] ); @@ -906,7 +910,8 @@ describe( 'selectors', () => { 123: '', }, }, - blockSelection: { start: {}, end: {} }, + selectionStart: {}, + selectionEnd: {}, }; expect( getMultiSelectedBlockClientIds( state ) ).toEqual( [] ); @@ -926,7 +931,8 @@ describe( 'selectors', () => { 5: '', }, }, - blockSelection: { start: { clientId: 2 }, end: { clientId: 4 } }, + selectionStart: { clientId: 2 }, + selectionEnd: { clientId: 4 }, }; expect( getMultiSelectedBlockClientIds( state ) ).toEqual( [ 4, 3, 2 ] ); @@ -951,7 +957,8 @@ describe( 'selectors', () => { 9: 4, }, }, - blockSelection: { start: { clientId: 7 }, end: { clientId: 9 } }, + selectionStart: { clientId: 7 }, + selectionEnd: { clientId: 9 }, }; expect( getMultiSelectedBlockClientIds( state ) ).toEqual( [ 9, 8, 7 ] ); @@ -967,7 +974,8 @@ describe( 'selectors', () => { order: {}, parents: {}, }, - blockSelection: { start: {}, end: {} }, + selectionStart: {}, + selectionEnd: {}, }; expect( @@ -979,7 +987,8 @@ describe( 'selectors', () => { describe( 'getMultiSelectedBlocksStartClientId', () => { it( 'returns null if there is no multi selection', () => { const state = { - blockSelection: { start: {}, end: {} }, + selectionStart: {}, + selectionEnd: {}, }; expect( getMultiSelectedBlocksStartClientId( state ) ).toBeNull(); @@ -987,7 +996,8 @@ describe( 'selectors', () => { it( 'returns multi selection start', () => { const state = { - blockSelection: { start: { clientId: 2 }, end: { clientId: 4 } }, + selectionStart: { clientId: 2 }, + selectionEnd: { clientId: 4 }, }; expect( getMultiSelectedBlocksStartClientId( state ) ).toBe( 2 ); @@ -997,7 +1007,8 @@ describe( 'selectors', () => { describe( 'getMultiSelectedBlocksEndClientId', () => { it( 'returns null if there is no multi selection', () => { const state = { - blockSelection: { start: {}, end: {} }, + selectionStart: {}, + selectionEnd: {}, }; expect( getMultiSelectedBlocksEndClientId( state ) ).toBeNull(); @@ -1005,7 +1016,8 @@ describe( 'selectors', () => { it( 'returns multi selection end', () => { const state = { - blockSelection: { start: { clientId: 2 }, end: { clientId: 4 } }, + selectionStart: { clientId: 2 }, + selectionEnd: { clientId: 4 }, }; expect( getMultiSelectedBlocksEndClientId( state ) ).toBe( 4 ); @@ -1232,7 +1244,8 @@ describe( 'selectors', () => { describe( 'isBlockSelected', () => { it( 'should return true if the block is selected', () => { const state = { - blockSelection: { start: { clientId: 123 }, end: { clientId: 123 } }, + selectionStart: { clientId: 123 }, + selectionEnd: { clientId: 123 }, }; expect( isBlockSelected( state, 123 ) ).toBe( true ); @@ -1240,7 +1253,8 @@ describe( 'selectors', () => { it( 'should return false if a multi-selection range exists', () => { const state = { - blockSelection: { start: { clientId: 123 }, end: { clientId: 124 } }, + selectionStart: { clientId: 123 }, + selectionEnd: { clientId: 124 }, }; expect( isBlockSelected( state, 123 ) ).toBe( false ); @@ -1248,7 +1262,8 @@ describe( 'selectors', () => { it( 'should return false if the block is not selected', () => { const state = { - blockSelection: { start: {}, end: {} }, + selectionStart: {}, + selectionEnd: {}, }; expect( isBlockSelected( state, 23 ) ).toBe( false ); @@ -1258,7 +1273,8 @@ describe( 'selectors', () => { describe( 'hasSelectedInnerBlock', () => { it( 'should return false if the selected block is a child of the given ClientId', () => { const state = { - blockSelection: { start: { clientId: 5 }, end: { clientId: 5 } }, + selectionStart: { clientId: 5 }, + selectionEnd: { clientId: 5 }, blocks: { order: { 4: [ 3, 2, 1 ], @@ -1276,7 +1292,8 @@ describe( 'selectors', () => { it( 'should return true if the selected block is a child of the given ClientId', () => { const state = { - blockSelection: { start: { clientId: 3 }, end: { clientId: 3 } }, + selectionStart: { clientId: 3 }, + selectionEnd: { clientId: 3 }, blocks: { order: { 4: [ 3, 2, 1 ], @@ -1306,7 +1323,8 @@ describe( 'selectors', () => { 5: 6, }, }, - blockSelection: { start: { clientId: 2 }, end: { clientId: 4 } }, + selectionStart: { clientId: 2 }, + selectionEnd: { clientId: 4 }, }; expect( hasSelectedInnerBlock( state, 6 ) ).toBe( true ); } ); @@ -1325,7 +1343,8 @@ describe( 'selectors', () => { 5: 6, }, }, - blockSelection: { start: { clientId: 5 }, end: { clientId: 4 } }, + selectionStart: { clientId: 5 }, + selectionEnd: { clientId: 4 }, }; expect( hasSelectedInnerBlock( state, 3 ) ).toBe( false ); } ); @@ -1334,7 +1353,8 @@ describe( 'selectors', () => { describe( 'isBlockWithinSelection', () => { it( 'should return true if the block is selected but not the last', () => { const state = { - blockSelection: { start: { clientId: 5 }, end: { clientId: 3 } }, + selectionStart: { clientId: 5 }, + selectionEnd: { clientId: 3 }, blocks: { order: { '': [ 5, 4, 3, 2, 1 ], @@ -1354,7 +1374,8 @@ describe( 'selectors', () => { it( 'should return false if the block is the last selected', () => { const state = { - blockSelection: { start: { clientId: 5 }, end: { clientId: 3 } }, + selectionStart: { clientId: 5 }, + selectionEnd: { clientId: 3 }, blocks: { order: { '': [ 5, 4, 3, 2, 1 ], @@ -1374,7 +1395,8 @@ describe( 'selectors', () => { it( 'should return false if the block is not selected', () => { const state = { - blockSelection: { start: { clientId: 5 }, end: { clientId: 3 } }, + selectionStart: { clientId: 5 }, + selectionEnd: { clientId: 3 }, blocks: { order: { '': [ 5, 4, 3, 2, 1 ], @@ -1394,7 +1416,8 @@ describe( 'selectors', () => { it( 'should return false if there is no selection', () => { const state = { - blockSelection: { start: {}, end: {} }, + selectionStart: {}, + selectionEnd: {}, blocks: { order: { '': [ 5, 4, 3, 2, 1 ], @@ -1416,10 +1439,8 @@ describe( 'selectors', () => { describe( 'hasMultiSelection', () => { it( 'should return false if no selection', () => { const state = { - blockSelection: { - start: {}, - end: {}, - }, + selectionStart: {}, + selectionEnd: {}, }; expect( hasMultiSelection( state ) ).toBe( false ); @@ -1427,10 +1448,8 @@ describe( 'selectors', () => { it( 'should return false if singular selection', () => { const state = { - blockSelection: { - start: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, - end: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, - }, + selectionStart: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, + selectionEnd: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, }; expect( hasMultiSelection( state ) ).toBe( false ); @@ -1438,10 +1457,8 @@ describe( 'selectors', () => { it( 'should return true if multi-selection', () => { const state = { - blockSelection: { - start: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, - end: { clientId: '9db792c6-a25a-495d-adbd-97d56a4c4189' }, - }, + selectionStart: { clientId: 'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1' }, + selectionEnd: { clientId: '9db792c6-a25a-495d-adbd-97d56a4c4189' }, }; expect( hasMultiSelection( state ) ).toBe( true ); @@ -1462,7 +1479,8 @@ describe( 'selectors', () => { 5: '', }, }, - blockSelection: { start: { clientId: 2 }, end: { clientId: 4 } }, + selectionStart: { clientId: 2 }, + selectionEnd: { clientId: 4 }, }; it( 'should return true if the block is multi selected', () => { @@ -1488,7 +1506,8 @@ describe( 'selectors', () => { 5: '', }, }, - blockSelection: { start: { clientId: 2 }, end: { clientId: 4 } }, + selectionStart: { clientId: 2 }, + selectionEnd: { clientId: 4 }, }; it( 'should return true if the block is first in multi selection', () => { @@ -1559,9 +1578,7 @@ describe( 'selectors', () => { describe( 'isSelectionEnabled', () => { it( 'should return true if selection is enable', () => { const state = { - blockSelection: { - isEnabled: true, - }, + isSelectionEnabled: true, }; expect( isSelectionEnabled( state ) ).toBe( true ); @@ -1569,9 +1586,7 @@ describe( 'selectors', () => { it( 'should return false if selection is disabled', () => { const state = { - blockSelection: { - isEnabled: false, - }, + isSelectionEnabled: false, }; expect( isSelectionEnabled( state ) ).toBe( false ); @@ -1581,10 +1596,8 @@ describe( 'selectors', () => { describe( 'getBlockInsertionPoint', () => { it( 'should return the explicitly assigned insertion point', () => { const state = { - blockSelection: { - start: { clientId: 'clientId2' }, - end: { clientId: 'clientId2' }, - }, + selectionStart: { clientId: 'clientId2' }, + selectionEnd: { clientId: 'clientId2' }, blocks: { byClientId: { clientId1: { clientId: 'clientId1' }, @@ -1618,10 +1631,8 @@ describe( 'selectors', () => { it( 'should return an object for the selected block', () => { const state = { - blockSelection: { - start: { clientId: 'clientId1' }, - end: { clientId: 'clientId1' }, - }, + selectionStart: { clientId: 'clientId1' }, + selectionEnd: { clientId: 'clientId1' }, blocks: { byClientId: { clientId1: { clientId: 'clientId1' }, @@ -1648,10 +1659,8 @@ describe( 'selectors', () => { it( 'should return an object for the nested selected block', () => { const state = { - blockSelection: { - start: { clientId: 'clientId2' }, - end: { clientId: 'clientId2' }, - }, + selectionStart: { clientId: 'clientId2' }, + selectionEnd: { clientId: 'clientId2' }, blocks: { byClientId: { clientId1: { clientId: 'clientId1' }, @@ -1682,10 +1691,8 @@ describe( 'selectors', () => { it( 'should return an object for the last multi selected clientId', () => { const state = { - blockSelection: { - start: { clientId: 'clientId1' }, - end: { clientId: 'clientId2' }, - }, + selectionStart: { clientId: 'clientId1' }, + selectionEnd: { clientId: 'clientId2' }, blocks: { byClientId: { clientId1: { clientId: 'clientId1' }, @@ -1716,10 +1723,8 @@ describe( 'selectors', () => { it( 'should return an object for the last block if no selection', () => { const state = { - blockSelection: { - start: {}, - end: {}, - }, + selectionStart: {}, + selectionEnd: {}, blocks: { byClientId: { clientId1: { clientId: 'clientId1' }, From 56c281c30d5ba8c4eab1dbbd1cab7fa772b55316 Mon Sep 17 00:00:00 2001 From: iseulde Date: Wed, 25 Sep 2019 13:42:15 +0300 Subject: [PATCH 2/3] Test selectionStart and selectionEnd reducers instead of internal selection reducer --- packages/block-editor/src/store/reducer.js | 5 +- .../block-editor/src/store/test/reducer.js | 160 ++++++++++++------ 2 files changed, 111 insertions(+), 54 deletions(-) diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index ff760ff660f77..69bdbdb749bcd 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -923,14 +923,15 @@ export function isCaretWithinFormattedText( state = false, action ) { } /** - * General reducer returning the block selection's start and end. + * Internal helper reducer for selectionStart and selectionEnd. Can hold a block + * selection, represented by an object with property clientId. * * @param {Object} state Current state. * @param {Object} action Dispatched action. * * @return {Object} Updated state. */ -export function selection( state = {}, action ) { +function selection( state = {}, action ) { switch ( action.type ) { case 'CLEAR_SELECTED_BLOCK': { if ( state.clientId ) { diff --git a/packages/block-editor/src/store/test/reducer.js b/packages/block-editor/src/store/test/reducer.js index 2f3688e6ca748..606b54510e9bd 100644 --- a/packages/block-editor/src/store/test/reducer.js +++ b/packages/block-editor/src/store/test/reducer.js @@ -22,7 +22,6 @@ import { blocks, isTyping, isCaretWithinFormattedText, - selection, selectionStart, selectionEnd, initialPosition, @@ -1870,180 +1869,237 @@ describe( 'state', () => { offset: 2, } ); } ); - } ); - describe( 'selection()', () => { it( 'should return with block clientId as selected', () => { - const state = selection( undefined, { + const action = { type: 'SELECT_BLOCK', clientId: 'kumquat', - } ); + }; + const state1 = selectionStart( undefined, action ); + const state2 = selectionEnd( undefined, action ); + const expected = { clientId: 'kumquat' }; - expect( state ).toEqual( { clientId: 'kumquat' } ); + expect( state1 ).toEqual( expected ); + expect( state2 ).toEqual( expected ); } ); it( 'should not update the state if the block is already selected', () => { const original = deepFreeze( { clientId: 'ribs' } ); - const state = selection( original, { + const action = { type: 'SELECT_BLOCK', clientId: 'ribs', - } ); + }; + const state1 = selectionStart( original, action ); + const state2 = selectionEnd( original, action ); - expect( state ).toBe( original ); + expect( state1 ).toBe( original ); + expect( state2 ).toBe( original ); } ); it( 'should unset selection', () => { const original = deepFreeze( { clientId: 'ribs' } ); - const state = selection( original, { + const action = { type: 'CLEAR_SELECTED_BLOCK', - } ); + }; - expect( state ).toEqual( {} ); + const state1 = selectionStart( original, action ); + const state2 = selectionEnd( original, action ); + + expect( state1 ).toEqual( {} ); + expect( state2 ).toEqual( {} ); } ); it( 'should return same reference if clearing selection but no selection', () => { - const original = selection( undefined, {} ); - const state = selection( original, { + const original = deepFreeze( {} ); + const action = { type: 'CLEAR_SELECTED_BLOCK', - } ); + }; - expect( state ).toBe( original ); + const state1 = selectionStart( original, action ); + const state2 = selectionEnd( original, action ); + + expect( state1 ).toBe( original ); + expect( state2 ).toBe( original ); } ); it( 'should select inserted block', () => { - const state = selection( undefined, { + const action = { type: 'INSERT_BLOCKS', blocks: [ { clientId: 'ribs' } ], updateSelection: true, - } ); + }; + const state1 = selectionStart( undefined, action ); + const state2 = selectionEnd( undefined, action ); + const expected = { clientId: 'ribs' }; - expect( state ).toEqual( { clientId: 'ribs' } ); + expect( state1 ).toEqual( expected ); + expect( state2 ).toEqual( expected ); } ); it( 'should not select inserted block if updateSelection flag is false', () => { const original = deepFreeze( { clientId: 'a' } ); - const state = selection( original, { + const action = { type: 'INSERT_BLOCKS', blocks: [ { clientId: 'ribs' } ], updateSelection: false, - } ); + }; + const state1 = selectionStart( original, action ); + const state2 = selectionEnd( original, action ); - expect( state ).toEqual( original ); + expect( state1 ).toBe( original ); + expect( state2 ).toBe( original ); } ); it( 'should not update the state if the block moved is already selected', () => { const original = deepFreeze( { clientId: 'ribs' } ); - const state = selection( original, { + const action = { type: 'MOVE_BLOCKS_UP', clientIds: [ 'ribs' ], - } ); + }; - expect( state ).toBe( original ); + const state1 = selectionStart( original, action ); + const state2 = selectionEnd( original, action ); + + expect( state1 ).toBe( original ); + expect( state2 ).toBe( original ); } ); it( 'should replace the selected block', () => { const original = deepFreeze( { clientId: 'chicken' } ); - const state = selection( original, { + const action = { type: 'REPLACE_BLOCKS', clientIds: [ 'chicken' ], blocks: [ { clientId: 'wings' } ], - } ); + }; + const state1 = selectionStart( original, action ); + const state2 = selectionEnd( original, action ); + const expected = { clientId: 'wings' }; - expect( state ).toEqual( { clientId: 'wings' } ); + expect( state1 ).toEqual( expected ); + expect( state2 ).toEqual( expected ); } ); it( 'should not replace the selected block if we keep it at the end when replacing blocks', () => { const original = deepFreeze( { clientId: 'wings' } ); - const state = selection( original, { + const action = { type: 'REPLACE_BLOCKS', clientIds: [ 'wings' ], blocks: [ { clientId: 'chicken' }, { clientId: 'wings' }, ], - } ); + }; + const state1 = selectionStart( original, action ); + const state2 = selectionEnd( original, action ); - expect( state ).toBe( original ); + expect( state1 ).toBe( original ); + expect( state2 ).toBe( original ); } ); it( 'should replace the selected block if we keep it not at the end when replacing blocks', () => { const original = deepFreeze( { clientId: 'chicken' } ); - const state = selection( original, { + const action = { type: 'REPLACE_BLOCKS', clientIds: [ 'chicken' ], blocks: [ { clientId: 'chicken' }, { clientId: 'wings' }, ], - } ); + }; + const state1 = selectionStart( original, action ); + const state2 = selectionEnd( original, action ); + const expected = { clientId: 'wings' }; - expect( state ).toEqual( { clientId: 'wings' } ); + expect( state1 ).toEqual( expected ); + expect( state2 ).toEqual( expected ); } ); it( 'should reset if replacing with empty set', () => { const original = deepFreeze( { clientId: 'chicken' } ); - const state = selection( original, { + const action = { type: 'REPLACE_BLOCKS', clientIds: [ 'chicken' ], blocks: [], - } ); + }; - expect( state ).toEqual( {} ); + const state1 = selectionStart( original, action ); + const state2 = selectionEnd( original, action ); + + expect( state1 ).toEqual( {} ); + expect( state2 ).toEqual( {} ); } ); it( 'should keep the selected block', () => { const original = deepFreeze( { clientId: 'chicken' } ); - const state = selection( original, { + const action = { type: 'REPLACE_BLOCKS', clientIds: [ 'ribs' ], blocks: [ { clientId: 'wings' } ], - } ); + }; + const state1 = selectionStart( original, action ); + const state2 = selectionEnd( original, action ); - expect( state ).toBe( original ); + expect( state1 ).toBe( original ); + expect( state2 ).toBe( original ); } ); it( 'should remove the selection if we are removing the selected block', () => { const original = deepFreeze( { clientId: 'chicken' } ); - const state = selection( original, { + const action = { type: 'REMOVE_BLOCKS', clientIds: [ 'chicken' ], - } ); + }; - expect( state ).toEqual( {} ); + const state1 = selectionStart( original, action ); + const state2 = selectionEnd( original, action ); + + expect( state1 ).toEqual( {} ); + expect( state2 ).toEqual( {} ); } ); it( 'should keep the selection if we are not removing the selected block', () => { const original = deepFreeze( { clientId: 'chicken' } ); - const state = selection( original, { + const action = { type: 'REMOVE_BLOCKS', clientIds: [ 'ribs' ], - } ); + }; - expect( state ).toBe( original ); + const state1 = selectionStart( original, action ); + const state2 = selectionEnd( original, action ); + + expect( state1 ).toEqual( original ); + expect( state2 ).toEqual( original ); } ); it( 'should update the selection on inner blocks replace if updateSelection is true', () => { const original = deepFreeze( { clientId: 'chicken' } ); - const state = selection( original, { + const action = { type: 'REPLACE_INNER_BLOCKS', blocks: [ { clientId: 'another-block' } ], rootClientId: 'parent', updateSelection: true, - } ); + }; + const state1 = selectionStart( original, action ); + const state2 = selectionEnd( original, action ); + const expected = { clientId: 'another-block' }; - expect( state ).toEqual( { clientId: 'another-block' } ); + expect( state1 ).toEqual( expected ); + expect( state2 ).toEqual( expected ); } ); it( 'should not update the selection on inner blocks replace if updateSelection is false', () => { const original = deepFreeze( { clientId: 'chicken' } ); - const state = selection( original, { + const action = { type: 'REPLACE_INNER_BLOCKS', blocks: [ { clientId: 'another-block' } ], rootClientId: 'parent', updateSelection: false, - } ); + }; + const state1 = selectionStart( original, action ); + const state2 = selectionEnd( original, action ); - expect( state ).toBe( original ); + expect( state1 ).toEqual( original ); + expect( state2 ).toEqual( original ); } ); } ); From 243e24532e8d81527429f5ae12e81afc64966819 Mon Sep 17 00:00:00 2001 From: iseulde Date: Wed, 25 Sep 2019 19:17:50 +0300 Subject: [PATCH 3/3] Add comment when reducer resets state by default --- packages/block-editor/src/store/reducer.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index 69bdbdb749bcd..7cafd627e9cec 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -1092,6 +1092,8 @@ export function initialPosition( state, action ) { } else if ( action.type === 'REMOVE_BLOCKS' ) { return state; } + + // Reset the state by default (for any action not handled). } export function blocksMode( state = {}, action ) {