Fix getSelectedBlockClientId selector #12214
Conversation
At the moment, we don't have a way to clear or update block selection for UNDO / REDO actions. By inverting the dependencies between getSelectedBlockClientId and getSelectedBlock we make sure we always return a valid clientId in getSelectedBlockClientId. We still have to fix the UNDO/REDO problem internally, but, in the meantime, this patch fixes the API.
This makes me thing the fix should be done in the reducer ideally, whenever the block is removed and it's the currently selected block, reset the selection in the reducer. |
|
There is handling already there for https://github.com/WordPress/gutenberg/blob/master/packages/editor/src/store/reducer.js#L788-L798 |
|
I'm pretty sure we're missing a case there as it's the only way |
|
Or I guess it's because this reducer is not inside the "history" :) mmm interesting issue. While we can fix it by tweaking the selector to check for existence first. I wonder if the selection reducer should be part of the history somehow. cc @aduth |
|
@gziolo @youknowriad for reference, take a look at this comment #12002 (comment) I agree this should be fixed in the reducer, but I've hit some deadends in every approach I've tried so far. |
|
This bandaid fix could be something like |
|
Pushed a more performant variant of this. And less code! |
Is there an issue for this? |
| // We need to check the block exists because the current state.blockSelection reducer | ||
| // doesn't take into account the UNDO / REDO actions to update selection. | ||
| // To be removed when that's fixed. | ||
| return start && start === end && !! state.editor.present.blocks.byClientId[ start ] ? start : null; |
There was a problem hiding this comment.
Minor / more of a recommendation for the future: I get uneasy when I see chains of mixed infix operators, and would prefer this written as ( a && a === b && !! c ) ? a : null — i.e. with the brackets around the ternary's condition.
* Fix getSelectedBlockClientId selector At the moment, we don't have a way to clear or update block selection for UNDO / REDO actions. By inverting the dependencies between getSelectedBlockClientId and getSelectedBlock we make sure we always return a valid clientId in getSelectedBlockClientId. We still have to fix the UNDO/REDO problem internally, but, in the meantime, this patch fixes the API. * Fix tests * Revert "Fix getSelectedBlockClientId selector" This reverts commit cfa3066. * Revert "Fix tests" This reverts commit 60abeab. * Use a more performant variant
Addresses #12002
Superseedes #12209 and #12052 that tried to fix this only for the
BlockSettingsMenucomponent.At the moment, we don't have a way to clear or update block selection for
UNDO/REDOactions. This is problematic for every client that usesgetSelectedBlockClientIdbecause they may end up with a block that no longer exist in the editor.By inverting the dependencies between
getSelectedBlockClientIdandgetSelectedBlockwe make sure we always return a valid clientId ingetSelectedBlockClientId.We still have to fix the
UNDO/REDOproblem, but this patch gives us time to think while the external API works as expected.