Fix multiselecting blocks using shift + arrow#11237
Conversation
youknowriad
added
[Type] Bug
jasmussen
left a comment
jasmussen
left a comment
There was a problem hiding this comment.
YAAAAY THIS fixes it!
I also dig the amount of red.
Code looks very healthy to me, but if you feel you need a sanity check might want to verify with some of the better coders out there.
Thank you!
|
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.
| } ); | ||
|
|
||
| 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.
|
|
||
| if ( target !== this.wrapperNode || this.props.isLocked ) { | ||
| if ( | ||
| ! this.props.isSelected || |
| } | ||
|
|
||
| // 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.
4 participants
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