Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block Editor: All references to blocks should be accounted for in history #12327

Closed
youknowriad opened this issue Nov 26, 2018 · 5 comments
Closed
Assignees
Labels
[Feature] History History, undo, redo, revisions, autosave. [Type] Bug An existing feature does not function as intended [Type] Code Quality Issues or PRs that relate to code quality

Comments

@youknowriad
Copy link
Contributor

Right now, if we edit blocks, the history (undo/redo) is kept in the redux store using the withHistory higher-order reducer. But only the blocks substate is kept in the history while the block selection is not. Which means if we trigger undo/redo we can potentially end up with a state tree where the blockSelection points to a non-existing block. (For example a block that was removed by undoing a change).

We should explore whether moving the block selection reducer inside the withHistory Higher-order reducer could solve these issues and which side-effects it would have on the undo/redo behavior.

Related #12214

@youknowriad youknowriad added [Type] Enhancement A suggestion for improvement. [Type] Code Quality Issues or PRs that relate to code quality labels Nov 26, 2018
@aduth
Copy link
Member

aduth commented Nov 26, 2018

Am I correct in thinking that this could apply to any state which holds a reference to a block's clientId?

This would include:

@youknowriad
Copy link
Contributor Author

Am I correct in thinking that this could apply to any state which holds a reference to a block's clientId?

Yes, 👍

@aduth
Copy link
Member

aduth commented May 21, 2019

I think it's more accurate to consider this a bug. The issue has manifested itself in a few forms:

#15543 (comment)

click the "More" button and the editor crashes

I think the issue here is that wp.data.select('core/block-editor').getSelectedBlockClientIds() still returns the previously selected block, even though it has been deleted.

#12002

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

@aduth aduth changed the title Consider including the selection reducer in the history Higher-order reducer Block Editor: All references to blocks should be accounted for in history May 21, 2019
@aduth aduth added [Type] Bug An existing feature does not function as intended and removed [Type] Enhancement A suggestion for improvement. labels May 21, 2019
@ellatrix ellatrix self-assigned this Jul 5, 2019
@ellatrix
Copy link
Member

@youknowriad Is this still relevant after #17824?

@youknowriad
Copy link
Contributor Author

Maybe not, I was only aware of the selection. I think we can close and reopen specific issues if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] History History, undo, redo, revisions, autosave. [Type] Bug An existing feature does not function as intended [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

No branches or pull requests

3 participants