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 multi select #896

Merged
merged 11 commits into from
May 30, 2017
53 changes: 37 additions & 16 deletions components/button/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,43 @@
import './style.scss';
import classnames from 'classnames';

function Button( { href, target, isPrimary, isLarge, isToggled, className, disabled, ...additionalProps } ) {
const classes = classnames( 'components-button', className, {
button: ( isPrimary || isLarge ),
'button-primary': isPrimary,
'button-large': isLarge,
'is-toggled': isToggled,
} );

const tag = href !== undefined && ! disabled ? 'a' : 'button';
const tagProps = tag === 'a' ? { href, target } : { type: 'button', disabled };

return wp.element.createElement( tag, {
...tagProps,
...additionalProps,
className: classes,
} );
class Button extends wp.element.Component {
constructor( props ) {
super( props );
this.setRef = this.setRef.bind( this );
}

componentDidMount() {
if ( this.props.focus ) {
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain the issues with focus you were seeing? Do we need to monitor this prop if it changes during the lifetime of the component?

Copy link
Member

Choose a reason for hiding this comment

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

Could you explain the issues with focus you were seeing? Do we need to monitor this prop if it changes during the lifetime of the component?

Circling back to this. I encountered the Toolbar focus prop when working on some unrelated refactoring and had no idea what its purpose is. I still don't, and could use some clarification as to this unanswered question.

this.ref.focus();
}
}

setRef( ref ) {
this.ref = ref;
}

render() {
const { href, target, isPrimary, isLarge, isToggled, className, disabled, ...additionalProps } = this.props;
const classes = classnames( 'components-button', className, {
button: ( isPrimary || isLarge ),
'button-primary': isPrimary,
'button-large': isLarge,
'is-toggled': isToggled,
} );

const tag = href !== undefined && ! disabled ? 'a' : 'button';
const tagProps = tag === 'a' ? { href, target } : { type: 'button', disabled };

delete additionalProps.focus;

return wp.element.createElement( tag, {
...tagProps,
...additionalProps,
className: classes,
ref: this.setRef,
} );
}
}

export default Button;
4 changes: 2 additions & 2 deletions components/icon-button/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ import './style.scss';
import Button from '../button';
import Dashicon from '../dashicon';

function IconButton( { icon, children, label, className, ...additionalProps } ) {
function IconButton( { icon, children, label, className, focus, ...additionalProps } ) {
Copy link
Member

Choose a reason for hiding this comment

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

focus would be passed to the underlying Button without being explicitly handled here, based on the behavior of additionalProps spread.

const classes = classnames( 'components-icon-button', className );

return (
<Button { ...additionalProps } aria-label={ label } className={ classes }>
<Button { ...additionalProps } aria-label={ label } className={ classes } focus={ focus }>
<Dashicon icon={ icon } />
{ children }
</Button>
Expand Down
1 change: 1 addition & 0 deletions components/toolbar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ function Toolbar( { controls } ) {
'is-active': control.isActive,
} ) }
aria-pressed={ control.isActive }
focus={ focus && ! index }
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is causing some trouble for me in tests. I am unsure as to what is happening here. Where is focus coming from? Is this supposed to be passed into the component as a prop? If that is the case, we will need to add focus to the function signature at the top. Toolbar( { controls, focus } ).

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right. This was causing a bug too. Thanks for catching it!

/>
) ) }
</ul>
Expand Down
13 changes: 7 additions & 6 deletions editor/block-mover/index.js
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
114 changes: 114 additions & 0 deletions editor/modes/visual-editor/block-list.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/**
* External dependencies
*/
import { connect } from 'react-redux';
import clickOutside from 'react-click-outside';

/**
* Internal dependencies
*/
import VisualEditorBlock from './block';
import {
getBlockUids,
getBlockInsertionPoint,
getBlockSelectionStart,
getBlockSelectionEnd,
} from '../../selectors';

const INSERTION_POINT_PLACEHOLDER = '[[insertion-point]]';

class VisualEditorBlockList extends wp.element.Component {
constructor( props ) {
super( props );

this.onSelectionStart = this.onSelectionStart.bind( this );
this.onSelectionChange = this.onSelectionChange.bind( this );
this.onSelectionEnd = this.onSelectionEnd.bind( this );

this.state = {
selectionAtStart: null,
};
}

onSelectionStart( uid ) {
this.setState( { selectionAtStart: uid } );
}

onSelectionChange( uid ) {
const { onMultiSelect, selectionStart, selectionEnd } = this.props;
const { selectionAtStart } = this.state;
const isAtStart = selectionAtStart === uid;

if ( ! selectionAtStart ) {
return;
}

if ( isAtStart && selectionStart ) {
onMultiSelect( { start: null, end: null } );
}

if ( ! isAtStart && selectionEnd !== uid ) {
onMultiSelect( { start: selectionAtStart, end: uid } );
}
}

onSelectionEnd() {
this.setState( { selectionAtStart: null } );
}

handleClickOutside() {
this.props.clearSelectedBlock();
}

render() {
const { blocks, insertionPoint } = this.props;
const insertionPointIndex = blocks.indexOf( insertionPoint );
const blocksWithInsertionPoint = insertionPoint
? [
...blocks.slice( 0, insertionPointIndex + 1 ),
INSERTION_POINT_PLACEHOLDER,
...blocks.slice( insertionPointIndex + 1 ),
]
: blocks;

return (
<div>
{ blocksWithInsertionPoint.map( ( uid ) => {
if ( uid === INSERTION_POINT_PLACEHOLDER ) {
return (
<div
key={ INSERTION_POINT_PLACEHOLDER }
className="editor-visual-editor__insertion-point"
/>
);
}

return (
<VisualEditorBlock
key={ uid }
uid={ uid }
onSelectionStart={ () => this.onSelectionStart( uid ) }
onSelectionChange={ () => this.onSelectionChange( uid ) }
onSelectionEnd={ this.onSelectionEnd }
/>
);
} ) }
</div>
);
}
}

export default connect(
( state ) => ( {
blocks: getBlockUids( state ),
insertionPoint: getBlockInsertionPoint( state ),
selectionStart: getBlockSelectionStart( state ),
selectionEnd: getBlockSelectionEnd( state ),
} ),
( dispatch ) => ( {
clearSelectedBlock: () => dispatch( { type: 'CLEAR_SELECTED_BLOCK' } ),
onMultiSelect( { start, end } ) {
dispatch( { type: 'MULTI_SELECT', start, end } );
},
} )
)( clickOutside( VisualEditorBlockList ) );
Loading