Implements undo/redo for draw, pin, and actuation mode

[skuiry]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
ewod-gui-2-0	❌ Failed (Inspect)			Feb 2, 2024 10:42am

[leofuturer]

Looks like the undo and redo is only implemented for "drawing", "pin mapping", and the "actuation" (not from this PR) mode. Did we decide to not include the "select and move" mode?

Besides, since the undo and redo might be coupled with other issues, let's try to merge PR #265 into the stage first before we work on this issue.

[skuiry]

Looks like the undo and redo is only implemented for "drawing", "pin mapping", and the "actuation" (not from this PR) mode. Did we decide to not include the "select and move" mode?

Besides, since the undo and redo might be coupled with other issues, let's try to merge PR #265 into the stage first before we work on this issue.

The undo should be implemented within the select and move mode as well. I forgot to include that in the PR title. And yeah, I'll look into closing #265 soon.

[leofuturer]

I tested again and the undo and redo for "select and move" mode is still missing. The icons for undo and redo disappear if I switch to the "select and move" mode.

Besides, please fix the conflicts here.

[skuiry]

I tested again and the undo and redo for "select and move" mode is still missing. The icons for undo and redo disappear if I switch to the "select and move" mode.

That's odd, let me look into this right now.

[leofuturer]

Thanks, Shounak!
And the undo for mapping mode seems to have some issues:

20230209_154859.mp4

1. If I map one electrode to a pin number and then click undo (with the current electrode highlighted), the undo erases the previous mapping.
2. If I map one electrode to a pin number -> click another electrode to highlight it -> click undo, the undo seems to stop working

[skuiry]

Thanks, Shounak! And the undo for mapping mode seems to have some issues:

20230209_154859.mp4

1. If I map one electrode to a pin number and then click undo (with the current electrode highlighted), the undo erases the previous mapping.
2. If I map one electrode to a pin number -> click another electrode to highlight it -> click undo, the undo seems to stop working

Ok, the undo buttons will show up in "select and move" mode now. Let me check what's wrong with the mapping mode. Sorry about that!

[skuiry]

Ok, I believe I fixed the pin mapping bug. I reproduced it like in the video and the undo function didn't break. It undoes the latest pin as expected. Please let me know if it still doesn't work!

[leofuturer]

Looks good! Thank you, Shounak.
When I was testing the undo and redo functions for the "select and move" mode, I found there was still some unexpected behavior of the function. For example, when I cut and pasted some electrodes -> click undo, it only removes the pasted electrodes instead of returning those electrodes to the original position.

I think, for now, we should make a document for a comprehensive list of the expected behavior of the undo and redo (for the 4 modes). This will help us think and test more systematically, and it also helps when you implement Cypress tests for the undo and redo functions. Could you make one? I will comment on it after seeing your draft.
For the "select and move" mode (which is probably the most complicated mode for undo and redo), you can refer to this one: https://ewod-gui-2-0-158jzqwr8-leofuturer.vercel.app/

[skuiry]

Looks good! Thank you, Shounak. When I was testing the undo and redo functions for the "select and move" mode, I found there was still some unexpected behavior of the function. For example, when I cut and pasted some electrodes -> click undo, it only removes the pasted electrodes instead of returning those electrodes to the original position.

I think, for now, we should make a document for a comprehensive list of the expected behavior of the undo and redo (for the 4 modes). This will help us think and test more systematically, and it also helps when you implement Cypress tests for the undo and redo functions. Could you make one? I will comment on it after seeing your draft. For the "select and move" mode (which is probably the most complicated mode for undo and redo), you can refer to this one: https://ewod-gui-2-0-158jzqwr8-leofuturer.vercel.app/

Yeah, that sounds like a great idea! I'll make a doc in the shared drive and fill it in. Also, by 4 modes, do you mean pin, actuation, draw, and move?

[leofuturer]

Yeah, that sounds like a great idea! I'll make a doc in the shared drive and fill it in. Also, by 4 modes, do you mean pin, actuation, draw, and move?

Yes!

[skuiry]

Looks good! Thank you, Shounak. When I was testing the undo and redo functions for the "select and move" mode, I found there was still some unexpected behavior of the function. For example, when I cut and pasted some electrodes -> click undo, it only removes the pasted electrodes instead of returning those electrodes to the original position.

Also, I just tested cutting and pasting some electrodes on the GUI. It seemed to correctly return the pasted electrodes to the original position. Could you try again and see if you run into the issue again?

[leofuturer]

I tested on the Vercel deployment for your latest commit: https://ewod-gui-2-0-158jzqwr8-leofuturer.vercel.app/

20230209_175553.mp4

[leofuturer]

Please resolve the conflict here as well @skuiry

[leofuturer]

It seems if we select some electrodes -> "move" or "cut and paste" them -> undo, those electrodes returned back to original position can not be selected anymore. After refresh the page, they can be selected again.

20230218_235608.mp4