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

✨Enable undo and redo #110

Merged
merged 9 commits into from
Mar 1, 2022
Merged

✨Enable undo and redo #110

merged 9 commits into from
Mar 1, 2022

Conversation

AdrySky
Copy link
Contributor

@AdrySky AdrySky commented Feb 28, 2022

Description

This will enable undo and redo xircuits. Either via context menu or shortcut. Also add shortcut for save.

Shortcut:

  1. Ctrl + z = Undo
  2. Ctrl + y = Redo
  3. Ctrl + s = Save

In addition, change the all the icons to LabIcon and sync it with Jupyterlab theme.

Before in dark theme(Component lib icon):
Before dark

After in dark theme(Component lib icon):
After dark

References

Currently, it detect changes mentioned in #75

Pull Request Type

  • Xircuits Core (Jupyterlab Related changes)
  • Xircuits Canvas (Custom RD Related changes)
  • Xircuits Component Library
  • Documentation
  • Others (Please Specify)

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Tests

Based on detected changes mentioned in #75,

  1. Check on whether the undo/redo works via shortcuts key.
  2. Test also via context menu.
  3. Also, test the icons when changing theme(white -> dark). Previously, the color only works in white theme.

Tested on?

  • Windows
  • Linux Ubuntu
  • Centos
  • Mac
  • Others (State here -> xxx )

Notes

Probably need to add a listener when nodes position was changed and connected links deleted.

@AdrySky AdrySky added the enhancement New feature or request label Feb 28, 2022
@AdrySky AdrySky requested a review from MFA-X-AI February 28, 2022 02:09
@AdrySky AdrySky self-assigned this Feb 28, 2022
Copy link
Member

@MFA-X-AI MFA-X-AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pull request! Looks like another good QOL feature to have.
As discussed in our chat, the undo-redo function doesn't work possibly because the detect changes / dirty state isn't functioning properly. Hope ya can fix it. Thanks.

@AdrySky AdrySky requested a review from MFA-X-AI February 28, 2022 03:04
@AdrySky
Copy link
Contributor Author

AdrySky commented Feb 28, 2022

Thanks for the reviewed. I fixed the issue. It related to detecting changes when saving it. I removed it previously because the undo/redo manager detect saving xircuits as changes. Right now, it should be working fine.

Copy link
Member

@MFA-X-AI MFA-X-AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, the undo and redo is now working!
Was pleasantly surprised when you mentioned that since you've used Juypyter's doc undo and redo there's no limit to the steps, awesome.

One thing that was odd was that although the undo and redo works perfectly from the right click menu, pressing ctrl z / ctrl y moves backwards / forwards 2 steps. It doesn't break anything else though, so I guess we can merge it if you want and improve it in the future ones.

The changes in the icons are also noted and appreciated (but as always, would prefer if you could make a new pull request for other changes so I can track them easily).

@MFA-X-AI MFA-X-AI merged commit 890ae33 into master Mar 1, 2022
@MFA-X-AI MFA-X-AI deleted the adry/undo-redo branch March 1, 2022 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants