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.

Sign up for GitHub

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

Jump to bottom

Undo/Redo Feature for Mantis #379

Merged
merged 42 commits into from
Mar 24, 2024
Merged

Undo/Redo Feature for Mantis #379

merged 42 commits into from
Mar 24, 2024

Conversation

rickshane
Copy link
Contributor

This is the Undo/Redo feature for Mantis.

@guoyingtao
Copy link
Owner

This feature is for Mantis, I think we need to move more code related to undo/redo from Example project into Mantis to make undo/redo easier to use.

@guoyingtao
Copy link
Owner

We may need to add some properties in Config enable/disable undo/redo
Mantis only provides basic features by default but provide more features by turning on switches in Config.

guoyingtao
guoyingtao requested changes Mar 21, 2024
View reviewed changes
Copy link
Owner

@guoyingtao guoyingtao 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 PR!
I added some comments. Please take a look.

@rickshane
Addressing comments in PR
819f732
@rickshane
Add MantisExampleTests target back to project - somehow got deleted.
01533eb
@rickshane
Adressing Comments in Chante List:
8a5e2d6 
- added "enableUndo" flag in Config
- undo/redo/reset logic checks the "enableUndo" flag in cropViewController to enable functionality
- abstracted the TransformRecord logic into the CropViewController class
- removed UIButtons from TransformDelegate
- moved TransformDelegate in to new file in the Protocols folder
@rickshane
Copy link
Contributor Author

All comments/issues have been addressed.

guoyingtao
guoyingtao requested changes Mar 21, 2024
View reviewed changes
Copy link
Owner

@guoyingtao guoyingtao 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 quick update.
I added more comments. Please take a look.

@rickshane
Address latest round of comments
ee8c1d6 
- More private variables.
- Embedded controller uses proxy functions to communicate with CropViewController state.
- Defaults function implementations in delegate methods.
@rickshane
Copy link
Contributor Author

latest comments addressed

@guoyingtao
Copy link
Owner

Thanks for the quick fixes!
I will go back to continue the reviews when I get more time.

@rickshane
Add new files to Mantis.xcodeproj:
f141294 
- TransformStack.swift
- TransformRecord.swift
- TransformDelegate.swift
@rickshane
Copy link
Contributor Author

Added new files to Mantis.xcodeproj

@guoyingtao
Copy link
Owner

Added new files to Mantis.xcodeproj

Thanks for doing that!
Can you also fix the building errors for Unit Tests? Some test codes are failed to build due to the recent code changes.

image

@rickshane
Rename delegate methods in CropViewControllerDelegate:
9f6497e 
- cropViewControllerDidUpdateEnableStateForUndo(_ enable: Bool) -> cropViewController(_ cropViewController: CropViewController, didUpdateEnableStateForUndo enable: Bool)

- cropViewControllerDidUpdateEnableStateForRedo(_ enable: Bool) -> cropViewController(_ cropViewController: CropViewController, didUpdateEnableStateForRedo enable: Bool)

- cropViewControllerDidUpdateEnableStateForReset(_ enable: Bool) -> cropViewController(_ cropViewController: CropViewController, didUpdateEnableStateForReset enable: Bool)
@rickshane
Remove blanl line
eaf77c7
@rickshane
Simplify an optional value boolean test
787ba3f
@rickshane
Remove three unused variables from TransformStack
49b3c1c
@rickshane
Address a SwiftLint warning in TransformRecord
06d4b10
@rickshane
Address a SwiftLint warning in TransformRecord
c0a8d3f
@rickshane
Fixed Unit Tests in Mantis.xcodeproj related to new API for undo/redo…
458a2fb 
… feature.
@rickshane
Copy link
Contributor Author

Fixed unit test in Mantis.xcodeproj related to new API for undo/redo feature.

guoyingtao
guoyingtao approved these changes Mar 24, 2024
View reviewed changes
Copy link
Owner

@guoyingtao guoyingtao left a comment

Choose a reason for hiding this comment

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

Looks good.
Thanks for your work!

@guoyingtao guoyingtao merged commit 9a1e67a into guoyingtao:master Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guoyingtao guoyingtao guoyingtao approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@rickshane @guoyingtao