Implement Undo/Redo Functionality In Designer

[barreeeiroo]

General items:

• I have updated the relevant documentation files under docs/
• My code follows the:
  • Google Java style guide (for .java files)
  • Google JavaScript style guide (for .js files)
• ant tests passes on my machine

If your code changes how something works on the device (i.e., it affects the companion):

• I branched from ucr
• My pull request has ucr as the base

Further, if you've changed the blocks language or another user-facing designer/blocks API (added a SimpleProperty, etc.):

• I have updated the corresponding version number in appinventor/components/src/.../common/YaVersion.java
• I have updated the corresponding upgrader in appinventor/appengine/src/.../client/youngandroid/YoungAndroidFormUpgrader.java (components only)
• I have updated the corresponding entries in appinventor/blocklyeditor/src/versioning.js

For all other changes:

• I branched from master
• My pull request has master as the base

What does this PR accomplish?

Description

Adds Undo/Redo functionality to the Designer. It implements a diff-based stack, rather than full screen snapshots.

Functionality Details

• Stacks are limited to 100 edits backwards ("undo") and 50 edits forwards ("redo").
• Stacks are per-screen: switching screens maintains the old stack. So, navigating back to the previous screen allows to "restore" the history.
• When a component is deleted either by Ctrl+Z or Delete button directly, this is propagated to blocks editor to delete all those blocks. Blocks editor now has that in their own Undo stack, so it's possible to restore as well there (see the last demo video). As of now, restoring a component from Undo/Redo Designer doesn't restore the blocks, but I think this is the intended behaviour to make the Designer be able to "create" blocks automatically...
• There's a small bug/feature: in the Table Arrangement, there are 3 actions happening when a component is moved: move component, and x2 set property in the arrangement. These 2 correspond go the row/columns. In the demo, you'll notice that I need to Ctrl+Z 3 times to actually undo that action.

Demo Recordings

Classic Theme Demo

keyviz_DEOZctgGCq.mp4

New Theme Demo

Spotify_1OKqEMyL4F.mp4

Layouts Demo

keyviz_lvKhrmsI6f.mp4

Components' Blocks Destroy/Restore Demo

keyviz_ZllXFWmmLV.mp4

[ewpatton]

@mark-friedman Given you worked on undo/redo before, would you mind taking a look and share your thoughts?

[mark-friedman]

Sure.

[mark-friedman]

There's a small bug/feature: in the Table Arrangement, there are 3 actions happening when a component is moved: move component, and x2 set property in the arrangement. These 2 correspond go the row/columns. In the demo, you'll notice that I need to Ctrl+Z 3 times to actually undo that action.

I also notice that if you do a move in the Table Arrangement and then undo the move, redo won't redo the move.

[mark-friedman]

When a component is deleted either by Ctrl+Z or Delete button directly, this is propagated to blocks editor to delete all those blocks. Blocks editor now has that in their own Undo stack, so it's possible to restore as well there (see the last demo video). As of now, restoring a component from Undo/Redo Designer doesn't restore the blocks, but I think this is the intended behaviour to make the Designer be able to "create" blocks automatically...

Is this really the desired behavior? I would want to have the blocks restored when you undo the component deletion.

Related to this, I notice that if you delete a component and then go to the blocks editor you can restore the blocks via undo, which you point out. However, you probably shouldn't be able to, since the component no longer exists. To be fair, AI2 has similar behavior (although it shows an error for the restored component blocks, which this implementation does not). But now we have undo in the designer, so we can do better. I think that the deletion of the component and the blocks should be considered a single unit and should be undone and redone together.

[barreeeiroo]

Is this really the desired behavior? I would want to have the blocks restored when you undo the component deletion.

To fix this, it means that we have to store in the "Designer" stack, also the blocks. We also probably need to merge the Undo/Redo of Blockly and Designer together. For example, it's possible the user undoes or redoes separate things:

1. User deletes a component from Designer (blocks are gone, existing behaviour)
2. Then the user adds some blocks for other component in the blocks area
3. User Ctrl+Z the component, restoring it

We can't apply a Ctrl+Z to the Blocks editor, as it has diverged since the deletion. It is also possible that some more complex blocks (nested setters or getters inside ifs no longer existing, for example) make this restore impossible.

Unless we merge both undo/redo stacks into a unified one, I don't think we can restore the blocks as well.

However, you probably shouldn't be able to, since the component no longer exists.

This is the existing behaviour unfortunately, even without undo in designer. If you delete a component, the blocks deletion are recorded in Blockly's undo stack, so you can undo and get back that corrupted block:

To be fair, AI2 has similar behavior (although it shows an error for the restored component blocks, which this implementation does not).

It behaves the same in AI2. You can see it has an error in the "restored block of the deleted component". It's just that the error is shown as "missing value" because it takes precedence over the "missing component" one.

I think that the deletion of the component and the blocks should be considered a single unit and should be undone and redone together.

I'd say this is a different discussion. Right now, users can edit Designer and Blocks fully separate. Their "history trees" will deviate, and can reach some states where restoring is not possible, as they are not syncronized. My way of thinking is:

• We have Designer and Blocks, as separate editors (users can work in Designer for a while, then move to Blocks editor, work there, and move back to Designer).
• As of now, Ctrl+Z in Blocks just operates in Blocks: we don't care if a property in Designer was changed in between, even if a component was deleted earlier and now we introduce a corrupted block.
• If we merge both "undo/redo" stacks, that means that users will only have a single history, and I think this will be painful based on the current usage pattern:
  • Let's say someone spends most of the time in Blocks editor. They first created a component, deleted it, and then did 100 different actions in Blocks editor for other components.
  • They realized they need to undo the action. If it's a single history tree, that means "undoing" 100 different blocks which they may indeed want to retain.
  • As soon as they modify the branch after the 100 undos, there's no way to redo and recover the original blocks operations.
• I don't think the idea of "storing the blocks" in the Designer stack to restore when a component is restored is feasible. Because of the other history tree (if we go with this approach), we have no guarantee we will be able to restore the procedures, setters or getters properly.
  • For example, someone has a Screen1.Initialize -> Set Label1.Text action.
  • Now they delete Label1
  • They now operate in the blocks, and delete Screen1.Initialize.
  • If they want to restore Label1, we cannot restore Set Label1.Text.
• In this last approach, we could potentially only restore event handlers and nested blocks. However, it's still risky:
  • For example, someone has a Button1.Click-> Set Global Variable X action
  • Now they delete Button1
  • They now operate in the blocks, and delete Global Variable X
  • If they want to restore Button1 with the event handlers, the Set Global Variable X block will be corrupted/impossible to restore.

With those considerations, that's why I think it's safer to keep them as separate stacks, and the users can undo/redo them independently. There's already validation in place for missing components regarding blocks, so we don't need to introduce a breaking behaviour.