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

List block: Undo/redo history breaks when undoing list block creation #44196

Closed
fluiddot opened this issue Sep 15, 2022 · 4 comments
Closed

List block: Undo/redo history breaks when undoing list block creation #44196

fluiddot opened this issue Sep 15, 2022 · 4 comments
Assignees
Labels
[Block] List Affects the List Block [Type] Bug An existing feature does not function as intended

Comments

@fluiddot
Copy link
Contributor

Description

After inserting a List block and adding list items, undoing all actions until the List block is removed breaks the undo/redo history.

Step-by-step reproduction instructions

  1. Add a List block.
  2. Add multiple items to the list.
  3. Add another block (e.g. add a Paragraph block with some text).
  4. Click on the "Undo" button multiple times until all list items and the List block is removed.
  5. Click on the "Redo" button multiple times and observe that the List block is created but the list items are not restored.

Expected: The "Redo" button should restore all the content that has been undone.

Screenshots, screen recording, code snippet

list-block-undo-redo-history.mp4

Environment info

  • Gutenberg: v14.0.2
  • Editing Toolkit: 3.42557
  • WP AMP: 2.0.5
  • CoBlocks: 2.18.1-simple-rev.2

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@fluiddot fluiddot added [Type] Bug An existing feature does not function as intended [Block] List Affects the List Block labels Sep 15, 2022
@fluiddot fluiddot changed the title List block: Undo/redo history breaks when undo list block creation List block: Undo/redo history breaks when undoing list block creation Sep 15, 2022
@fluiddot
Copy link
Contributor Author

fluiddot commented Sep 15, 2022

I tried to debug the issue and found out that it might be caused by how the inner blocks are updated via the useInnerBlockTemplateSync hook, specifically when executing the redo action that restores the List block. This hook updates the inner blocks using the action replaceInnerBlocks, which as far as I tested, updates the undo/redo history similarly as if the change was done by the user, ultimately leading to replacing part of the undo/redo history and losing the previous performed editions.

NOTE 1: I debugged this issue using the native version, but as mentioned in this comment, this issue is common on both platforms so we should expect the same results in the web version.

NOTE 2: I tested the undo/redo behavior on other blocks that use inner blocks (e.g. Group block) and I encountered similar issues.

In order to provide a better context, let me share some results about the debugging session I did:

Steps done before debugging:

  1. Add a List block from the Inserter.
  2. Add a first item to the list with text 1.
  3. Add a second item to the list with text 2.
  4. Click on the "Undo" button two times to remove the second item.
  5. Click again on the "Undo" button two times to remove the first item.
  6. Click again on the "Undo" button to remove the empty List block.

After these steps, the state of the core-data store, which reflects the undo/redo history, is the following:

Screenshot 2022-09-15 at 18 28 04

At this point, clicking on the "Redo" button leads to the problem described in the issue's description. Doing this produces the execution of two EDIT_ENTITY_RECORD actions that are outlined in the next sections.

The first one that restores the List block:

{
    "type": "EDIT_ENTITY_RECORD",
    "kind": "postType",
    "name": "post",
    "recordId": 1,
    "edits": {
        "blocks": [
            {
                "clientId": "23ca9214-7b16-4d52-a573-a59c11d91f5f",
                "name": "core/list",
                "isValid": true,
                "attributes": {
                    "ordered": false,
                    "values": ""
                },
                "innerBlocks": []
            }
        ],
        "selection": {
            "selectionStart": {
                "clientId": "23ca9214-7b16-4d52-a573-a59c11d91f5f"
            },
            "selectionEnd": {
                "clientId": "23ca9214-7b16-4d52-a573-a59c11d91f5f"
            },
            "initialPosition": 0
        }
    },
    "meta": {
        "isRedo": true
    }
}

However, this action is detected as a change by the useInnerBlockTemplateSync hook, which in order to set inner blocks content, it executes replaceInnerBlocks which leads to another EDIT_ENTITY_RECORD (this action is the one that breaks the undo/redo history):

{
    "type": "EDIT_ENTITY_RECORD",
    "kind": "postType",
    "name": "post",
    "recordId": 1,
    "edits": {
        "blocks": [
            {
                "clientId": "23ca9214-7b16-4d52-a573-a59c11d91f5f",
                "name": "core/list",
                "isValid": true,
                "attributes": {
                    "ordered": false,
                    "values": ""
                },
                "innerBlocks": [
                    {
                        "clientId": "1bb8e9eb-99ec-4087-913e-5cdd19b861b1",
                        "name": "core/list-item",
                        "isValid": true,
                        "attributes": {
                            "content": ""
                        },
                        "innerBlocks": []
                    }
                ]
            }
        ],
        "selection": {
            "selectionStart": {
                "clientId": "1bb8e9eb-99ec-4087-913e-5cdd19b861b1"
            },
            "selectionEnd": {
                "clientId": "1bb8e9eb-99ec-4087-913e-5cdd19b861b1"
            },
            "initialPosition": 0
        }
    },
    "transientEdits": {
        "blocks": true,
        "selection": true
    },
    "meta": {
        "undo": {
            "kind": "postType",
            "name": "post",
            "recordId": 1,
            "edits": {
                "blocks": [
                    {
                        "clientId": "23ca9214-7b16-4d52-a573-a59c11d91f5f",
                        "name": "core/list",
                        "isValid": true,
                        "attributes": {
                            "ordered": false,
                            "values": ""
                        },
                        "innerBlocks": []
                    }
                ],
                "selection": {
                    "selectionStart": {
                        "clientId": "23ca9214-7b16-4d52-a573-a59c11d91f5f"
                    },
                    "selectionEnd": {
                        "clientId": "23ca9214-7b16-4d52-a573-a59c11d91f5f"
                    },
                    "initialPosition": 0
                }
            },
            "transientEdits": {
                "blocks": true,
                "selection": true
            }
        }
    }
}

Potential solution

Not sure if it's possible, but we should detect somehow when updating the inner block's content if the change was generated by the user or a redo action. If the latter, the EDIT_ENTITY_RECORD should use the undoIgnore parameter to avoid modifying the undo/redo history.

@paulopmt1
Copy link
Contributor

Hey, I'm gonna work on this :)

@geriux
Copy link
Member

geriux commented Jun 12, 2023

I've just checked this on the latest Gutenberg version and it is working as expected, I'm guessing some of the improvements added to the block fixed the problem. What do you think if we close this ticket @fluiddot ?

@fluiddot
Copy link
Contributor Author

I've just checked this on the latest Gutenberg version and it is working as expected, I'm guessing some of the improvements added to the block fixed the problem. What do you think if we close this ticket @fluiddot ?

Sure, I've just tested the app (version 22.4) and web version and couldn't manage to reproduce the issue. That said, I'll proceed to close the issue. Thanks for the heads up @geriux 🙇 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] List Affects the List Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

3 participants