Undo action doesn't update block selection

[oandregal]

Note: this is easier to spot if you don't use "Unified Toolbar" mode, as the block toolbar is positioned on top of the selected block, but the same behavior applies.

Related #11996 (reverted) #12052

Steps to reproduce the behavior:

For duplication:

• Open a post
• Select a block and duplicate it using the block settings
• Observe how the focus is changed to the new block
• Click the undo button
• Observe how the focus is lost

The expected result is that the focus was returned to the previous block.

For deletion:

• Open a post
• Select a block and duplicate it using the block settings
• Observe how the focus is changed to the previous block
• Click the undo button
• Observe how the focus is maintained by the previous block

[oandregal]

More context. It looks like the select( 'core/editor' ).getSelectedBlockClientId() isn't updated after the undo operation. In the first case (Duplication + Undo) what ends happening is that the selected block no longer exists.

[ellatrix]

Just noting that this has been an issue for a while. We don't save any selection state to in history, neither block selection, nor rich text selection. In fact, rich text selection state is local in the RichText component. This will need some rework... I thought there was an older issue about this.

[ellatrix]

In my opinion, this seems like something that should be looked at soon. At least make an explicit decision whether this should be part of 5.0 or not.

[mcsf]

I see a PR for this that then got closed, and I see the issue is in the 5.0 milestone. What's the status of this? From here it looks this belongs in post-5.0.

[oandregal]

This is what I've considered so far:

1. Clear selection on UNDO / REDO actions. Discarded as this would cause problems when the user is editing a block.
2. Clear selection on UNDO / REDO actions if the block doesn't exist. This seemed promising. The way we would do this is by catching the UNDO/REDO actions in the blockSelection reducer and do the proper checks. The problem is that we need other state subtrees to check if a block exists in the current post (like the editor.present subtree). Normally, a reducer gets this kind of data from the dispatched action, but in this case UNDO / REDO don't attach any.
3. Maybe update block selection on UNDO/REDO actions #12188 Could we use effects to dispatch the clear selection action after UNDO / REDO? No. The component would be updated first, causing this error.
4. Make undo action update blockSelection #12133 Making the blockSelection history aware by wrapping it with withHistory. The issue with this approach is that selection could contain the same block several times and UNDO to get past selections wouldn't ensure the block selected is a valid block, it could be actually the same block that has been deleted. We'd still need the ability to check whether the block still exists in the post (same problem than option 2).

I'm sharing this in the hope I've missed some obvious approach that we can implement easily. cc @WordPress/gutenberg-core for feedback.

[oandregal]

@mcsf I was just writing above comment when closing #12133 At the moment I think we should merge #12052 for 5.0 and revisit this post-5.0.

[oandregal]

I actually found a way that fixes the external API (the getSelectedBlockClientId selector) #12214

[aduth]

Is this closed now by #12214 ?

Tip: You can auto-close issues by pull requests using specific keywords.

[aduth]

Related: #12327 (accounting for selection in undo history)

[oandregal]

Is this closed now by #12214 ?

Not from my point of view, that's why I haven't marked it as closed. 12214 is only a workaround while this issue is not fixed.

[youknowriad]

Which means this is a duplicate of #12327 (we should close one of them).

[oandregal]

Oh, I see. I assumed we were using this to track the bug, but I've now seen 12327 and it looks more implementation focused than this one. Whatever you feel is useful for not losing sight of the problem would work for me.

[aduth]

Let's consolidate to #12327. I've back-referenced to this issue and reclassified it as a bug to ensure appropriate urgency.