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

Multi block selection: more responsive UI #18915

Merged
merged 1 commit into from
Dec 6, 2019
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
5 changes: 3 additions & 2 deletions packages/block-editor/src/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,11 @@ function BlockListBlock( {
// Focus the selected block's wrapper or inner input on mount and update
const isMounting = useRef( true );
useEffect( () => {
if ( isSelected ) {
if ( isSelected && ! isMultiSelecting ) {
focusTabbable( ! isMounting.current );
}
isMounting.current = false;
}, [ isSelected ] );
}, [ isSelected, isMultiSelecting ] );

// Focus the first multi selected block
useEffect( () => {
Expand Down Expand Up @@ -445,6 +445,7 @@ function BlockListBlock( {
! hasFixedToolbar &&
isLargeViewport &&
! showEmptyBlockSideInserter &&
! isMultiSelecting &&
(
( isSelected && ( ! isTypingWithinBlock || isCaretWithinFormattedText ) ) ||
isFirstMultiSelected
Expand Down
38 changes: 21 additions & 17 deletions packages/block-editor/src/components/block-list/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class BlockList extends Component {

this.onSelectionStart = this.onSelectionStart.bind( this );
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd love if we can somehow isolate/decouple the multi-selection code form the BlockList rendering code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good. I feel like the PR converting to hooks is a good opportunity to do this.

this.onSelectionEnd = this.onSelectionEnd.bind( this );
this.setSelection = this.setSelection.bind( this );
this.onSelectionChange = this.onSelectionChange.bind( this );
this.updateNativeSelection = this.updateNativeSelection.bind( this );

this.ref = createRef();
Expand All @@ -80,6 +80,7 @@ class BlockList extends Component {

componentWillUnmount() {
window.removeEventListener( 'mouseup', this.onSelectionEnd );
document.removeEventListener( 'selectionchange', this.onSelectionChange );
window.cancelAnimationFrame( this.rafId );
}

Expand All @@ -93,9 +94,10 @@ class BlockList extends Component {
blockClientIds,
// These must be in the right DOM order.
multiSelectedBlockClientIds,
isMultiSelecting,
} = this.props;

if ( ! hasMultiSelection ) {
if ( ! hasMultiSelection || isMultiSelecting ) {
return;
}

Expand Down Expand Up @@ -149,6 +151,7 @@ class BlockList extends Component {
// (from a block). The selection ends when `mouseup` happens anywhere
// in the window.
window.addEventListener( 'mouseup', this.onSelectionEnd );
document.addEventListener( 'selectionchange', this.onSelectionChange );

// Removing the contenteditable attributes within the block editor is
// essential for selection to work across editable areas. The edible
Expand All @@ -171,27 +174,30 @@ class BlockList extends Component {
onSelectionEnd() {
// Equivalent to attaching the listener once.
window.removeEventListener( 'mouseup', this.onSelectionEnd );
document.removeEventListener( 'selectionchange', this.onSelectionChange );

if ( ! this.props.isMultiSelecting ) {
return;
}

// The browser selection won't have updated yet at this point, so wait
// until the next animation frame to get the browser selection.
this.rafId = window.requestAnimationFrame( this.setSelection );
this.rafId = window.requestAnimationFrame( () => {
this.onSelectionChange();
this.props.onStopMultiSelect();
} );
}

setSelection() {
onSelectionChange() {
const selection = window.getSelection();
const {
onStopMultiSelect,
onMultiSelect,
getBlockParents,
onSelectBlock,
} = this.props;

// If no selection is found, end multi selection.
if ( ! selection.rangeCount || selection.isCollapsed ) {
this.props.onStopMultiSelect();
return;
}

Expand All @@ -206,19 +212,15 @@ class BlockList extends Component {
! ( clientId = focusNode.getAttribute( 'data-block' ) )
);

// If the final selection doesn't leave the block, there is no multi
// selection.
if ( this.startClientId === clientId ) {
onStopMultiSelect();
return;
}
onSelectBlock( clientId );
} else {
const startPath = [ ...getBlockParents( this.startClientId ), this.startClientId ];
const endPath = [ ...getBlockParents( clientId ), clientId ];
const depth = Math.min( startPath.length, endPath.length ) - 1;

const startPath = [ ...getBlockParents( this.startClientId ), this.startClientId ];
const endPath = [ ...getBlockParents( clientId ), clientId ];
const depth = Math.min( startPath.length, endPath.length ) - 1;

onMultiSelect( startPath[ depth ], endPath[ depth ] );
onStopMultiSelect();
onMultiSelect( startPath[ depth ], endPath[ depth ] );
}
}

render() {
Expand Down Expand Up @@ -326,12 +328,14 @@ export default compose( [
startMultiSelect,
stopMultiSelect,
multiSelect,
selectBlock,
} = dispatch( 'core/block-editor' );

return {
onStartMultiSelect: startMultiSelect,
onStopMultiSelect: stopMultiSelect,
onMultiSelect: multiSelect,
onSelectBlock: selectBlock,
};
} ),
] )( BlockList );
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ class MultiSelectScrollIntoView extends Component {
* visible within the nearest scrollable container.
*/
scrollIntoView() {
const { extentClientId } = this.props;
if ( ! extentClientId ) {
const { extentClientId, isMultiSelecting } = this.props;
if ( ! extentClientId || isMultiSelecting ) {
return;
}

Expand Down Expand Up @@ -56,9 +56,13 @@ class MultiSelectScrollIntoView extends Component {
}

export default withSelect( ( select ) => {
const { getLastMultiSelectedBlockClientId } = select( 'core/block-editor' );
const {
getLastMultiSelectedBlockClientId,
isMultiSelecting,
} = select( 'core/block-editor' );

return {
extentClientId: getLastMultiSelectedBlockClientId(),
isMultiSelecting: isMultiSelecting(),
};
} )( MultiSelectScrollIntoView );