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

Changes in field validators aren't grouped with field changes. #8588

Closed
1 task done
johnnesky opened this issue Sep 24, 2024 · 0 comments · Fixed by #8589
Closed
1 task done

Changes in field validators aren't grouped with field changes. #8588

johnnesky opened this issue Sep 24, 2024 · 0 comments · Fixed by #8589
Assignees
Labels
issue: bug Describes why the code or behaviour is wrong issue: triage Issues awaiting triage by a Blockly team member

Comments

@johnnesky
Copy link
Member

johnnesky commented Sep 24, 2024

Check for duplicates

  • I have searched for similar issues before opening a new one.

Description

In #8575 I noticed that the undo history isn't recorded correctly when a block's inputs are created or destroyed in response to a field validator. If a child block was attached to an input when the input is being destroyed, the block will become detached, which is recorded as a BlockMove event, but this event is not grouped with the BlockChange that caused it. This can cause problems when undoing, because the creating the input separately from moving the detached child block is likely to "bump" the child otherwise. If the child block gets bumped while undoing, that records a BlockMove event that can unintentionally erase any remaining the redo history.

The following block types are known to be affected:

  • lists_getIndex
  • lists_setIndex
  • lists_getSublist
  • text_getSubstring

Whether or not we want people to be making changes to the workspace in field validators is an interesting question, but the fact remains that it's already happening, in the blockly core library no less.

See also: #6456

Reproduction steps

  1. Drag a lists_getIndex and a math_number block into the workspace.
  2. Attach the number block to the input connection.
  3. Edit the dropdown field that says "#", changing it to "first" or "last". The number block will become detached, but remain in place overlapping the lists_getIndex block. A BlockMove event with a group id will have been fired, followed by a BlockChange event without any group id.
  4. Undo once. This will recreate the input connection, but won't attach the child block to it. Instead, the child block will get bumped.

Screenshots

Screen.Recording.2024-09-24.at.12.04.43.PM.mov
@johnnesky johnnesky added issue: bug Describes why the code or behaviour is wrong issue: triage Issues awaiting triage by a Blockly team member labels Sep 24, 2024
@johnnesky johnnesky self-assigned this Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: bug Describes why the code or behaviour is wrong issue: triage Issues awaiting triage by a Blockly team member
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant