Input Interaction: set caret correctly on merge forward (Delete) #15599
Conversation
ellatrix
added
[Feature] Writing Flow
ellatrix
requested review from
ajitbohra,
gziolo,
nerrad,
noisysocks,
ntwb and
talldan
as code owners
|
So if I'm reading properly, this doesn't need to be backported to WP 5.2 as the culprit PR is not included in it right? |
|
@youknowriad I guess you're right, if the full Gutenberg release will not be included in WordPress 5.2.1. |
|
It should probably still be included in the Gutenberg release. |
| const blockBType = getBlockType( blockB.name ); | ||
| const { clientId, attributeKey, offset } = getSelectionStart( state ); | ||
| const hasSelection = clientId === clientIdA || clientId === clientIdB; |
There was a problem hiding this comment.
The problem is that we are just assuming that the caret is in block B when merging blocks, while it could be in either.
I guess more accurately: It could be either, or neither? Which to me sounds like a reasonable qualification to consider, since I should be able to call mergeBlocks on any two blocks, selected or otherwise.
There was a problem hiding this comment.
Yes, that's why I added the hasSelection check. If none of the blocks have selection, the caret replacement will be skipped.
| updatedAttributes[ newAttributeKey ] = newHtml; | ||
|
|
||
| dispatch( selectionChange( | ||
| blockA.clientId, |
There was a problem hiding this comment.
Am I correct in thinking: While the selection may be within one or the other (or neither) of the two blocks, the merge will result in the first block becoming selected (if a selection had existed previously).
There was a problem hiding this comment.
Should it? I didn't think much about it. If there was no selection previously, there's no selection to be merged?
gziolo
added
the
Backport to WP 6.7 Beta/RC
I added the label |
|
@gziolo Actually, we don't need this label here because this bug is not on WP 5.2 |
gziolo
removed
the
Backport to WP 6.7 Beta/RC
|
👋 , feel free to update or close #15619 if this PR fixes it, thanks! |
Description
Probably regressed in #14640. The problem is that we are just assuming that the caret is in block B when merging blocks, while it could be in either. I updated the code to look which block is selected.
How has this been tested?
Merge two paragraph blocks with the
Deletekey (orFn+Deletefor Mac). Caret should be at merge point.Screenshots
Types of changes
Checklist: