Fix multiselecting blocks using shift + arrow #11237
Conversation
youknowriad
added
[Type] Bug
|
The failure is something in master and it was actually fixed already. |
| // When triggering a multi-selection, | ||
| // move the focus to the wrapper of the first selected block. | ||
| if ( this.props.isFirstMultiSelected && ! prevProps.isFirstMultiSelected ) { | ||
| this.wrapperNode.focus(); |
There was a problem hiding this comment.
Is this something which is meant to occur instead of focusTabbable? If so, why do we allow focusTabbable to occur at all? I wonder also if these redundant focus changes are announced loudly in assistive technologies.
There was a problem hiding this comment.
Yes, it's meant to occur instead of focusTabbable and yes focusTabbable won't occur here because the isSelected is false when the block is multiselected.
I'm not sure about the assertive technologies though, it's very tied with multi-selection, not sure how multi-selection is announced.
| @@ -77,4 +77,44 @@ describe( 'Multi-block selection', () => { | |||
| await pressWithModifier( META_KEY, 'a' ); | |||
| await expectMultiSelected( blocks, true ); | |||
| } ); | |||
|
|
|||
| it( 'Should select/unselect multiple blocks using Shift + Arrows', async () => { | |||
| const firstBlockSelector = '[data-type="core/paragraph"]'; | |||
There was a problem hiding this comment.
The actual issue was for 3 blocks of the same type. We should add another test :)
There was a problem hiding this comment.
The issue for me was happening no matter the type of the blocks.
| @@ -314,7 +320,11 @@ export class BlockListBlock extends Component { | |||
| deleteOrInsertAfterWrapper( event ) { | |||
| const { keyCode, target } = event; | |||
|
|
|||
| if ( target !== this.wrapperNode || this.props.isLocked ) { | |||
| if ( | |||
| ! this.props.isSelected || | |||
| @@ -93,6 +93,12 @@ export class BlockListBlock extends Component { | |||
| if ( this.props.isSelected && ! prevProps.isSelected ) { | |||
| this.focusTabbable( true ); | |||
| } | |||
|
|
|||
| // When triggering a multi-selection, | |||
| // move the focus to the wrapper of the first selected block. | |||
There was a problem hiding this comment.
These comments explain what we're doing (which is pretty plainly obvious in the implementation anyways) but not why. I don't know why we're doing this.
There was a problem hiding this comment.
We move the focus to ensure that it is not possible to continue editing the initially selected block. This happens for all active elements that allow editing like RichText, PlainText and all form fields. This behavior ensures that you no longer can modify the content of the selected blocks.
closes #9848
This PR fixes multiselection (of more than one block) using
shift + arrowbut it also keeps the focus ono the wrapper of the first multi-selected block which allows easy access to the toolbar usingTaborAlt+F10Testing instructions