Skip to content

Add multi select #896

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.

Sign up for GitHub

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

Merged
ellatrix merged 11 commits into from
May 30, 2017
Merged

Add multi select #896

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a687b81
Add multi select
ellatrix May 24, 2017
1cb0e46
Add darker blue border on selection rectangles.
jasmussen May 25, 2017
4ef611a
Hide selection backgroud on multi selection
ellatrix May 27, 2017
01d124a
Separate reducer for multi selection: fixes setting state back for si…
ellatrix May 27, 2017
cd04a65
Deselect blocks on click outside
ellatrix May 27, 2017
ec368f5
Add buttons
ellatrix May 28, 2017
91b577c
Add tests
ellatrix May 29, 2017
d489fce
Code polish
ellatrix May 29, 2017
336eade
Add esc and backspace
ellatrix May 29, 2017
236bf6f
Fix focus issue on the buttons
ellatrix May 30, 2017
6ef9c81
Keep scroll position when moving the selected blocks
ellatrix May 30, 2017
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
Prev Previous commit
Next Next commit
Add buttons
  • Loading branch information
@ellatrix
ellatrix committed May 29, 2017
commit ec368f57cc9a9ce907cb55196fd3cf39722aacfc
13 changes: 7 additions & 6 deletions editor/block-mover/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* External dependencies
*/
import { connect } from 'react-redux';
import { first, last } from 'lodash';

/**
* WordPress dependencies
Expand Down Expand Up @@ -40,20 +41,20 @@ function BlockMover( { onMoveUp, onMoveDown, isFirst, isLast } ) {

export default connect(
( state, ownProps ) => ( {
isFirst: isFirstBlock( state, ownProps.uid ),
isLast: isLastBlock( state, ownProps.uid ),
isFirst: isFirstBlock( state, first( ownProps.uids ) ),
isLast: isLastBlock( state, last( ownProps.uids ) ),
} ),
( dispatch, ownProps ) => ( {
onMoveDown() {
dispatch( {
type: 'MOVE_BLOCK_DOWN',
uid: ownProps.uid,
type: 'MOVE_BLOCKS_DOWN',
uids: ownProps.uids,
} );
},
onMoveUp() {
dispatch( {
type: 'MOVE_BLOCK_UP',
uid: ownProps.uid,
type: 'MOVE_BLOCKS_UP',
uids: ownProps.uids,
} );
},
} )
Expand Down
6 changes: 4 additions & 2 deletions editor/modes/visual-editor/block-list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import VisualEditorBlock from './block';
import {
getBlockUids,
getBlockInsertionPoint,
getSelectedBlocks,
getBlockSelectionStart,
getBlockSelectionEnd,
} from '../../selectors';
Expand Down Expand Up @@ -61,7 +62,7 @@ class VisualEditorBlockList extends wp.element.Component {
}

render() {
const { blocks, insertionPoint } = this.props;
const { blocks, insertionPoint, selectedBlocks } = this.props;
const insertionPointIndex = blocks.indexOf( insertionPoint );
const blocksWithInsertionPoint = insertionPoint
? [
Expand All @@ -87,10 +88,10 @@ class VisualEditorBlockList extends wp.element.Component {
<VisualEditorBlock
key={ uid }
uid={ uid }
selectionStart={ this.state.selectionStart }
onSelectionStart={ () => this.onSelectionStart( uid ) }
onSelectionChange={ () => this.onSelectionChange( uid ) }
onSelectionEnd={ this.onSelectionEnd }
selectedBlocks={ selectedBlocks }
Copy link
Contributor

@youknowriad youknowriad May 29, 2017

Choose a reason for hiding this comment

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

Should we drop the selectedProps prop and move it to the connect of VisualEditorBlock instead?

Copy link
Member Author

@ellatrix ellatrix May 29, 2017

Choose a reason for hiding this comment

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

Yeah :)

/>
);
} ) }
Expand All @@ -103,6 +104,7 @@ export default connect(
( state ) => ( {
blocks: getBlockUids( state ),
insertionPoint: getBlockInsertionPoint( state ),
selectedBlocks: getSelectedBlocks( state ),
selectionStart: getBlockSelectionStart( state ),
selectionEnd: getBlockSelectionEnd( state ),
} ),
Expand Down
37 changes: 28 additions & 9 deletions editor/modes/visual-editor/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
isBlockHovered,
isBlockSelected,
isBlockMultiSelected,
isFirstSelected,
isTypingInBlock,
} from '../../selectors';

Expand Down Expand Up @@ -105,7 +106,7 @@ class VisualEditorBlock extends wp.element.Component {

// Remove block on backspace
if ( 8 /* Backspace */ === keyCode && target === this.node ) {
this.props.onRemove( this.props.uid );
this.props.onRemove( [ this.props.uid ] );
if ( this.props.previousBlock ) {
this.props.onFocus( this.props.previousBlock.uid, { offset: -1 } );
}
Expand Down Expand Up @@ -165,7 +166,7 @@ class VisualEditorBlock extends wp.element.Component {
}

render() {
const { block } = this.props;
const { block, selectedBlocks } = this.props;
const settings = wp.blocks.getBlockSettings( block.blockType );

let BlockEdit;
Expand Down Expand Up @@ -199,8 +200,6 @@ class VisualEditorBlock extends wp.element.Component {
return (
<div
ref={ this.bindBlockNode }
onClick={ this.selectAndStopPropagation }
onFocus={ onSelect }
onKeyDown={ this.removeOrDeselect }
onMouseDown={ this.props.onSelectionStart }
onTouchStart={ this.props.onSelectionStart }
Expand All @@ -218,7 +217,7 @@ class VisualEditorBlock extends wp.element.Component {
tabIndex="0"
{ ...wrapperProps }
>
{ ( showUI || isHovered ) && <BlockMover uid={ block.uid } /> }
{ ( showUI || isHovered ) && <BlockMover uids={ [ block.uid ] } /> }
{ showUI &&
<CSSTransitionGroup
transitionName={ { appear: 'is-appearing', appearActive: 'is-appearing-active' } }
Expand All @@ -242,7 +241,26 @@ class VisualEditorBlock extends wp.element.Component {
</div>
</CSSTransitionGroup>
}
<div onKeyPress={ this.maybeStartTyping }>
{ this.props.isFirstSelected && (
Copy link
Contributor

@youknowriad youknowriad May 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Minor: Could we destructure all the this.props.*?

Copy link
Member Author

@ellatrix ellatrix May 29, 2017

Choose a reason for hiding this comment

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

Yeah. I think I forgot to remove this after I saw it clashes with the selector... Any naming conventions here?

Copy link
Contributor

@youknowriad youknowriad May 29, 2017

Choose a reason for hiding this comment

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

I'm ok keeping it to avoid conflicting with the selector. but Maybe I'd rename the selector isFirstSelectedBlock

Copy link
Member Author

@ellatrix ellatrix May 29, 2017

Choose a reason for hiding this comment

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

Sounds good!

<BlockMover uids={ selectedBlocks } />
) }
{ this.props.isFirstSelected && (
<div className="editor-visual-editor__block-controls">
<Toolbar
controls={ [ {
icon: 'trash',
title: '',
onClick: () => this.props.onRemove( selectedBlocks ),
isActive: false,
} ] }
/>
</div>
) }
<div
onKeyPress={ this.maybeStartTyping }
onFocus={ onSelect }
onClick={ this.selectAndStopPropagation }
>
<BlockEdit
focus={ focus }
attributes={ block.attributes }
Expand All @@ -266,6 +284,7 @@ export default connect(
block: getBlock( state, ownProps.uid ),
isSelected: isBlockSelected( state, ownProps.uid ),
isMultiSelected: isBlockMultiSelected( state, ownProps.uid ),
isFirstSelected: isFirstSelected( state, ownProps.uid ),
isHovered: isBlockHovered( state, ownProps.uid ),
focus: getBlockFocus( state, ownProps.uid ),
isTyping: isTypingInBlock( state, ownProps.uid ),
Expand Down Expand Up @@ -319,10 +338,10 @@ export default connect(
dispatch( focusBlock( ...args ) );
},

onRemove( uid ) {
onRemove( uids ) {
dispatch( {
type: 'REMOVE_BLOCK',
uid,
type: 'REMOVE_BLOCKS',
uids,
} );
},

Expand Down
4 changes: 4 additions & 0 deletions editor/selectors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ export function isFirstBlock( state, uid ) {
return first( state.editor.blockOrder ) === uid;
}

export function isFirstSelected( state, uid ) {
return first( getSelectedBlocks( state ) ) === uid;
}

export function isLastBlock( state, uid ) {
return last( state.editor.blockOrder ) === uid;
}
Expand Down
75 changes: 45 additions & 30 deletions editor/state.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/
import { combineReducers, applyMiddleware, createStore } from 'redux';
import refx from 'refx';
import { keyBy, last, omit, without, flowRight } from 'lodash';
import { keyBy, first, last, omit, without, flowRight } from 'lodash';

/**
* Internal dependencies
Expand Down Expand Up @@ -47,10 +47,10 @@ export const editor = combineUndoableReducers( {

case 'UPDATE_BLOCK':
case 'INSERT_BLOCK':
case 'MOVE_BLOCK_DOWN':
case 'MOVE_BLOCK_UP':
case 'MOVE_BLOCKS_DOWN':
case 'MOVE_BLOCKS_UP':
case 'REPLACE_BLOCKS':
case 'REMOVE_BLOCK':
case 'REMOVE_BLOCKS':
case 'EDIT_POST':
return true;
}
Expand Down Expand Up @@ -89,59 +89,72 @@ export const editor = combineUndoableReducers( {
};
}, omit( state, action.uids ) );

case 'REMOVE_BLOCK':
return omit( state, action.uid );
case 'REMOVE_BLOCKS':
return omit( state, action.uids );
}

return state;
},

blockOrder( state = [], action ) {
let index;
let swappedUid;
switch ( action.type ) {
case 'RESET_BLOCKS':
return action.blocks.map( ( { uid } ) => uid );

case 'INSERT_BLOCK':
case 'INSERT_BLOCK': {
Copy link
Contributor

@youknowriad youknowriad May 29, 2017

Choose a reason for hiding this comment

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

Curious to know why do you wrap case between {}. Is this a good practice I'm missing?

Copy link
Member Author

@ellatrix ellatrix May 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah I'd rather scope the const declarations to one case instead of the whole reducer. You can use {} anywhere you want to create a new block scope AFAIK.

/* ... */

{
    let something = 1;
}

/* ... */

Copy link
Member Author

@ellatrix ellatrix May 29, 2017

Choose a reason for hiding this comment

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

https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Statements/block

Copy link
Contributor

@youknowriad youknowriad May 29, 2017

Choose a reason for hiding this comment

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

Gotcha, Thanks for the explanation

const position = action.after ? state.indexOf( action.after ) + 1 : state.length;
return [
...state.slice( 0, position ),
action.block.uid,
...state.slice( position ),
];
}

case 'MOVE_BLOCKS_UP': {
const firstUid = first( action.uids );
const lastUid = last( action.uids );

case 'MOVE_BLOCK_UP':
if ( action.uid === state[ 0 ] ) {
if ( ! state.length || firstUid === first( state ) ) {
return state;
}
index = state.indexOf( action.uid );
swappedUid = state[ index - 1 ];

const firstIndex = state.indexOf( firstUid );
const lastIndex = state.indexOf( lastUid );
const swappedUid = state[ firstIndex - 1 ];

return [
...state.slice( 0, index - 1 ),
action.uid,
...state.slice( 0, firstIndex - 1 ),
...action.uids,
swappedUid,
...state.slice( index + 1 ),
...state.slice( lastIndex + 1 ),
];
}

case 'MOVE_BLOCK_DOWN':
if ( action.uid === last( state ) ) {
case 'MOVE_BLOCKS_DOWN': {
const firstUid = first( action.uids );
const lastUid = last( action.uids );

if ( ! state.length || lastUid === last( state ) ) {
return state;
}
index = state.indexOf( action.uid );
swappedUid = state[ index + 1 ];

const firstIndex = state.indexOf( firstUid );
const lastIndex = state.indexOf( lastUid );
const swappedUid = state[ lastIndex + 1 ];

return [
...state.slice( 0, index ),
...state.slice( 0, firstIndex ),
swappedUid,
action.uid,
...state.slice( index + 2 ),
...action.uids,
...state.slice( lastIndex + 2 ),
];
}

case 'REPLACE_BLOCKS':
if ( ! action.blocks ) {
return state;
}
index = state.indexOf( action.uids[ 0 ] );

return state.reduce( ( memo, uid ) => {
if ( uid === action.uids[ 0 ] ) {
return memo.concat( action.blocks.map( ( block ) => block.uid ) );
Expand All @@ -152,8 +165,8 @@ export const editor = combineUndoableReducers( {
return memo;
}, [] );

case 'REMOVE_BLOCK':
return without( state, action.uid );
case 'REMOVE_BLOCKS':
return without( state, ...action.uids );
}

return state;
Expand Down Expand Up @@ -204,11 +217,13 @@ export function selectedBlock( state = {}, action ) {
case 'CLEAR_SELECTED_BLOCK':
return {};

case 'MOVE_BLOCK_UP':
case 'MOVE_BLOCK_DOWN':
return action.uid === state.uid
case 'MOVE_BLOCKS_UP':
case 'MOVE_BLOCKS_DOWN': {
const firstUid = first( action.uids );
return firstUid === state.uid
? state
: { uid: action.uid, typing: false, focus: {} };
: { uid: firstUid, typing: false, focus: {} };
}

case 'INSERT_BLOCK':
return {
Expand Down
Loading