Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add state for storing dragged block client ids to block-editor store #24782

Merged
merged 3 commits into from
Aug 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,20 @@ _Returns_

- `Array`: ids of top-level and descendant blocks.

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

Returns the client ids of any blocks being directly dragged.

This does not include children of a parent being dragged.

_Parameters_

- _state_ `Object`: Global application state.

_Returns_

- `Array<string>`: Array of dragged block client ids.

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

Returns the client ID of the first block in the multi-selection set, or null
Expand Down Expand Up @@ -744,6 +758,19 @@ _Returns_

- `boolean`: Whether the block as an inner block selected

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

Returns whether a parent/ancestor of the block is being dragged.

_Parameters_

- _state_ `Object`: Global application state.
- _clientId_ `string`: Client id for block to check.

_Returns_

- `boolean`: Whether the block's ancestor is being dragged.

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

Returns true if an ancestor of the block is multi-selected, or false
Expand All @@ -758,6 +785,23 @@ _Returns_

- `boolean`: Whether an ancestor of the block is in multi-selection set.

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

Returns whether the block is being dragged.

Only returns true if the block is being directly dragged,
not if the block is a child of a parent being dragged.
See `isAncestorBeingDragged` for child blocks.

_Parameters_

- _state_ `Object`: Global application state.
- _clientId_ `string`: Client id for block to check.

_Returns_

- `boolean`: Whether the block is being dragged.

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

Returns true if the current highlighted block matches the block clientId.
Expand Down Expand Up @@ -1348,6 +1392,10 @@ _Returns_

Returns an action object used in signalling that the user has begun to drag blocks.

_Parameters_

- _clientIds_ `Array<string>`: An array of client ids being dragged

_Returns_

- `Object`: Action object.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ const BlockDraggable = ( {
elementId={ `block-${ clientIds[ 0 ] }` }
transferData={ transferData }
onDragStart={ ( event ) => {
startDraggingBlocks();
startDraggingBlocks( clientIds );
isDragging.current = true;

startScrolling( event );
Expand Down
7 changes: 3 additions & 4 deletions packages/block-editor/src/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,11 @@ function BlockListBlock( {
// (editor.BlockListBlock filter)
const { isDragging, isHighlighted } = useSelect(
( select ) => {
const { isDraggingBlocks, isBlockHighlighted } = select(
const { isBlockBeingDragged, isBlockHighlighted } = select(
'core/block-editor'
);
return {
isDragging: isDraggingBlocks(),
isDragging: isBlockBeingDragged( clientId ),
isHighlighted: isBlockHighlighted( clientId ),
};
},
Expand Down Expand Up @@ -154,8 +154,7 @@ function BlockListBlock( {
'is-highlighted': isHighlighted,
'is-multi-selected': isMultiSelected,
'is-reusable': isReusableBlock( blockType ),
'is-dragging':
isDragging && ( isSelected || isPartOfMultiSelection ),
draganescu marked this conversation as resolved.
Show resolved Hide resolved
'is-dragging': isDragging,
'is-typing': isTypingWithinBlock,
'is-focused':
isFocusMode && ( isSelected || isAncestorOfSelectedBlock ),
Expand Down
5 changes: 4 additions & 1 deletion packages/block-editor/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -718,11 +718,14 @@ export function stopTyping() {
/**
* Returns an action object used in signalling that the user has begun to drag blocks.
*
* @param {string[]} clientIds An array of client ids being dragged
*
* @return {Object} Action object.
*/
export function startDraggingBlocks() {
export function startDraggingBlocks( clientIds = [] ) {
return {
type: 'START_DRAGGING_BLOCKS',
clientIds,
};
}

Expand Down
14 changes: 7 additions & 7 deletions packages/block-editor/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1131,20 +1131,20 @@ export function isTyping( state = false, action ) {
}

/**
* Reducer returning dragging state.
* Reducer returning dragged block client id.
*
* @param {boolean} state Current state.
* @param {string[]} state Current state.
* @param {Object} action Dispatched action.
*
* @return {boolean} Updated state.
* @return {string[]} Updated state.
*/
export function isDraggingBlocks( state = false, action ) {
export function draggedBlocks( state = [], action ) {
switch ( action.type ) {
case 'START_DRAGGING_BLOCKS':
return true;
return action.clientIds;

case 'STOP_DRAGGING_BLOCKS':
return false;
return [];
}

return state;
Expand Down Expand Up @@ -1653,7 +1653,7 @@ export function highlightedBlock( state, action ) {
export default combineReducers( {
blocks,
isTyping,
isDraggingBlocks,
draggedBlocks,
isCaretWithinFormattedText,
selectionStart,
selectionEnd,
Expand Down
52 changes: 51 additions & 1 deletion packages/block-editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1036,7 +1036,57 @@ export function isTyping( state ) {
* @return {boolean} Whether user is dragging blocks.
*/
export function isDraggingBlocks( state ) {
return state.isDraggingBlocks;
return !! state.draggedBlocks.length;
}

/**
* Returns the client ids of any blocks being directly dragged.
*
* This does not include children of a parent being dragged.
*
* @param {Object} state Global application state.
*
* @return {string[]} Array of dragged block client ids.
Copy link
Member

Choose a reason for hiding this comment

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

It could return null if not dragging blocks. Should we return [] or null in this case? :🤔. I feel like [] would be simpler to indicate that there's no blocks being dragged, and unify the type. We just have to check the length attribute in isDraggingBlocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, makes sense, fixed in 680714b 👍

*/
export function getDraggedBlockClientIds( state ) {
return state.draggedBlocks;
}

/**
* Returns whether the block is being dragged.
*
* Only returns true if the block is being directly dragged,
* not if the block is a child of a parent being dragged.
* See `isAncestorBeingDragged` for child blocks.
*
* @param {Object} state Global application state.
* @param {string} clientId Client id for block to check.
*
* @return {boolean} Whether the block is being dragged.
*/
export function isBlockBeingDragged( state, clientId ) {
return state.draggedBlocks.includes( clientId );
}

/**
* Returns whether a parent/ancestor of the block is being dragged.
*
* @param {Object} state Global application state.
* @param {string} clientId Client id for block to check.
*
* @return {boolean} Whether the block's ancestor is being dragged.
*/
export function isAncestorBeingDragged( state, clientId ) {
// Return early if no blocks are being dragged rather than
// the more expensive check for parents.
if ( ! isDraggingBlocks( state ) ) {
return false;
}

const parents = getBlockParents( state, clientId );
return some( parents, ( parentClientId ) =>
kevin940726 marked this conversation as resolved.
Show resolved Hide resolved
isBlockBeingDragged( state, parentClientId )
);
}

/**
Expand Down
20 changes: 20 additions & 0 deletions packages/block-editor/src/store/test/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import {
startTyping,
stopMultiSelect,
stopTyping,
startDraggingBlocks,
stopDraggingBlocks,
toggleBlockMode,
toggleSelection,
updateBlock,
Expand Down Expand Up @@ -847,6 +849,24 @@ describe( 'actions', () => {
} );
} );

describe( 'startDraggingBlocks', () => {
it( 'should return the START_DRAGGING_BLOCKS action with the list of clientIds passed', () => {
const clientIds = [ 'block-1', 'block-2', 'block-3' ];
expect( startDraggingBlocks( clientIds ) ).toEqual( {
type: 'START_DRAGGING_BLOCKS',
clientIds,
} );
} );
} );

describe( 'stopDraggingBlocks', () => {
it( 'should return the STOP_DRAGGING_BLOCKS action', () => {
expect( stopDraggingBlocks() ).toEqual( {
type: 'STOP_DRAGGING_BLOCKS',
} );
} );
} );

describe( 'enterFormattedText', () => {
it( 'should return the ENTER_FORMATTED_TEXT action', () => {
expect( enterFormattedText() ).toEqual( {
Expand Down
22 changes: 22 additions & 0 deletions packages/block-editor/src/store/test/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
isUpdatingSameBlockAttribute,
blocks,
isTyping,
draggedBlocks,
isCaretWithinFormattedText,
selectionStart,
selectionEnd,
Expand Down Expand Up @@ -2064,6 +2065,27 @@ describe( 'state', () => {
} );
} );

describe( 'draggedBlocks', () => {
it( 'should store the dragged client ids when a user starts dragging blocks', () => {
const clientIds = [ 'block-1', 'block-2', 'block-3' ];
const state = draggedBlocks( [], {
type: 'START_DRAGGING_BLOCKS',
clientIds,
} );

expect( state ).toBe( clientIds );
} );

it( 'should set the state to an empty array when a user stops dragging blocks', () => {
const previousState = [ 'block-1', 'block-2', 'block-3' ];
const state = draggedBlocks( previousState, {
type: 'STOP_DRAGGING_BLOCKS',
} );

expect( state ).toEqual( [] );
} );
} );

describe( 'isCaretWithinFormattedText()', () => {
it( 'should set the flag to true', () => {
const state = isCaretWithinFormattedText( false, {
Expand Down
94 changes: 94 additions & 0 deletions packages/block-editor/src/store/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ const {
isFirstMultiSelectedBlock,
getBlockMode,
isTyping,
isDraggingBlocks,
getDraggedBlockClientIds,
isBlockBeingDragged,
isAncestorBeingDragged,
isCaretWithinFormattedText,
getBlockInsertionPoint,
isBlockInsertionPointVisible,
Expand Down Expand Up @@ -1849,6 +1853,96 @@ describe( 'selectors', () => {
} );
} );

describe( 'isDraggingBlocks', () => {
it( 'should return true if a block is being dragged', () => {
const state = {
draggedBlocks: [ 'block-client-id' ],
};
expect( isDraggingBlocks( state ) ).toBe( true );
} );

it( 'should return false if a block is not being dragged', () => {
const state = {
draggedBlocks: [],
};
expect( isDraggingBlocks( state ) ).toBe( false );
} );
} );

describe( 'getDraggedBlockClientIds', () => {
it( 'returns the draggedBlocks state', () => {
const draggedBlocks = [ 'block-client-id' ];
const state = {
draggedBlocks,
};
expect( getDraggedBlockClientIds( state ) ).toBe( draggedBlocks );
} );
} );

describe( 'isBlockBeingDragged', () => {
it( 'returns true if the given client id is one of the blocks being dragged', () => {
const state = {
draggedBlocks: [ 'block-1', 'block-2', 'block-3' ],
};
expect( isBlockBeingDragged( state, 'block-2' ) ).toBe( true );
} );

it( 'returns false if the given client id is not one of the blocks being dragged', () => {
const state = {
draggedBlocks: [ 'block-1', 'block-2', 'block-3' ],
};
expect( isBlockBeingDragged( state, 'block-4' ) ).toBe( false );
} );

it( 'returns false if no blocks are being dragged', () => {
const state = {
draggedBlocks: [],
};
expect( isBlockBeingDragged( state, 'block-1' ) ).toBe( false );
} );
} );

describe( 'isAncestorBeingDragged', () => {
it( 'returns true if the given client id is a child of one of the blocks being dragged', () => {
const state = {
draggedBlocks: [ 'block-1_grandparent' ],
blocks: {
parents: {
'block-1': 'block-1_parent',
'block-1_parent': 'block-1_grandparent',
},
},
};
expect( isAncestorBeingDragged( state, 'block-1' ) ).toBe( true );
} );

it( 'returns false if the given client id does not have an ancestor being dragged', () => {
const state = {
draggedBlocks: [ 'block-2_grandparent' ],
blocks: {
parents: {
'block-1': 'block-1_parent',
'block-1_parent': 'block-1_grandparent',
},
},
};
expect( isAncestorBeingDragged( state, 'block-1' ) ).toBe( false );
} );

it( 'returns false if no blocks are being dragged', () => {
const state = {
draggedBlocks: [],
blocks: {
parents: {
'block-1': 'block-1_parent',
'block-1_parent': 'block-1_grandparent',
},
},
};
expect( isAncestorBeingDragged( state, 'block-1' ) ).toBe( false );
} );
} );

describe( 'isCaretWithinFormattedText', () => {
it( 'returns true if the isCaretWithinFormattedText state is also true', () => {
const state = {
Expand Down