[Block Editor]: Fix selection by holding shift key for nested blocks #35668
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ntsekouras
added
[Type] Bug
ntsekouras
commented
Oct 15, 2021
| if ( | ||
| blockSelectionStart && | ||
| blockSelectionStart !== clientId | ||
| blockSelectionStart !== clientId && | ||
| ! startParents?.includes( clientId ) |
There was a problem hiding this comment.
@ellatrix I'm not sure if there is a more elegant way to handle this or in another place..
There was a problem hiding this comment.
This is for when we shift+click in a parent of the selected block, right? What should the outcome be for that case? Do we need a test here too?
There was a problem hiding this comment.
AFAIU, it's for this case:
- Pressing shift + click within a rich-text block nested inside a nested block like
Group
Currently, when you do that, you can't really edit the block anymore, see #33665. So this line is supposed to fix that behavior. In practice, for what I see, it highlights the text between the cursor location, and the location that's being shift-clicked.
There was a problem hiding this comment.
Do we need a test here too?
I think it's covered by this e2e test, isn't it?
There was a problem hiding this comment.
I think it's covered by this e2e test, isn't it?
Yes
|
Size Change: -5.22 kB (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
16 tasks
ockham
reviewed
Oct 21, 2021
Comment on lines
+180
to
+190
| const startPath = [ | ||
| ...startParents, | ||
| blockSelectionStart, | ||
| ]; | ||
| const endPath = [ | ||
| ...getBlockParents( clientId ), | ||
| clientId, | ||
| ]; | ||
| const depth = | ||
| Math.min( startPath.length, endPath.length ) - 1; | ||
| multiSelect( startPath[ depth ], endPath[ depth ] ); |
There was a problem hiding this comment.
It seems like this is pretty much the same logic as
Should we extract it into a small helper method, or share it otherwise?
There was a problem hiding this comment.
I thought about it but in the second case we use the parents for another check before, so a shared util for our 2 cases would end up passing the start/end paths and just having the Math.min in there, so I'm not really sure it's worth it..
I'm going to merge and happy to follow up on this if it needs to 😄
ockham
approved these changes
Oct 21, 2021
ellatrix
approved these changes
Oct 22, 2021
vcanales
pushed a commit
that referenced
this pull request
Oct 27, 2021
…35668) * [Block Editor]: Fix selection by holding shift key for nested blocks * add e2e tests
|
I'm not sure if this PR is related, but I get the sense that it may have tried to fix an issue that is similar to what I'm seeing at the moment, which is that after having edited content for a bit, the title block becomes inert:
I don't have specific steps to reproduce, other than to start editing some content, waiting for a bit, saving, then trying to edit the title. Sometimes it continues to work, sometimes it breaks. |
|
@jasmussen this PR had to do specifically with holding |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of: #35662
Fixes: #33665
There are currently two bugs in selection when holding
shiftkey that result in locking the user out of editing.GroupGroupblocks.Before
Screeny.Video.14.Oct.2021.at.18.41.39.mov
and
bug.mov
After
Screen.Recording.2021-10-15.at.3.05.24.PM.mov
In my videos the different
Groupblocks have different background colors.