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

Feature/swiftui support #43

Merged
merged 33 commits into from
Jan 17, 2025
Merged

Feature/swiftui support #43

merged 33 commits into from
Jan 17, 2025

Conversation

ZvonimirMedak
Copy link
Collaborator

Summary

This PR adds SwiftUI support to the Sentinel library, with the refactor of the currently available tools to SwiftUI.
There are a lot of files changed, but some of them are minor changes, and some of them have been deleted

Related issue: None

Changes

Type

  • [-] Feature: This pull request introduces a new feature.
  • [-] Bug fix: This pull request fixes a bug.
  • Refactor: This pull request refactors existing code.
  • [-] Documentation: This pull request updates documentation.
  • [-] Other: This pull request makes other changes.

Additional information

  • This pull request introduces a breaking change.

Description

This PR contains three major updates to the current state of the library:

  1. The Tool protocol now contains a ViewBuilder for the SwiftUI view representing the Tool. Removing the UIKit coupling with the presentPreview
  2. The ToolTableItem is an Enum now which contains four primary cases (Navigation, Toggle, CustomInfo, and Performance) which can be added to the List view to be easily shown in Sentinel. Also, includes a new .custom case which adds support for a custom View, and behavior
  3. The currently available tools have been refactored to use SwiftUI. Although, EmailSender, and Custom location have been wrapped in a UIViewRepresentable since there's no support for MessagesUI in SwiftUI, and CustomLocation would lose some of its functionalities due to lack of MapReader on iOS 14

Checklist

  • I have performed a self-review of my own code.
  • I have tested my changes, including edge cases.
  • I have added necessary tests for the changes introduced (if applicable).
  • [-] I have updated the documentation to reflect my changes (if applicable).

Additional notes

The code documentation will be added in a separate PR

@ZvonimirMedak ZvonimirMedak added the enhancement New feature or request label Dec 13, 2024
@ZvonimirMedak ZvonimirMedak self-assigned this Dec 13, 2024
Copy link
Member

@nikolamajcen nikolamajcen left a comment

Choose a reason for hiding this comment

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

Hey @ZvonimirMedak, amazing job! You did a heavy-lifting regarding the project update and its support for SwiftUI.

Still, I've added some comments regarding the implementation - please check them out. Alongside that, please check the usage of Objective-C and remove it from the project (this will also affect some classes which are subclasses of NSObject).

Note: I was unable to build the project due to multiple files (NavigationToolTableItem duplication and location issues).

CODE_OF_CONDUCT.md Outdated Show resolved Hide resolved
CustomToolTableItem.swift Outdated Show resolved Hide resolved
Example/Sentinel/AppDelegate.swift Outdated Show resolved Hide resolved
Example/Sentinel/AppDelegate.swift Outdated Show resolved Hide resolved
Example/Sentinel/ColorChangeToolTableItem.swift Outdated Show resolved Hide resolved
Sentinel/Classes/Core/ToolTable.swift Show resolved Hide resolved
Sentinel/Classes/EmailSender/EmailSenderTool.swift Outdated Show resolved Hide resolved
Sentinel/Classes/TextEditing/TextEditingTool.swift Outdated Show resolved Hide resolved
Sentinel/Classes/TextEditing/TextEditingTool.swift Outdated Show resolved Hide resolved
@nikolamajcen nikolamajcen added this to the 2.0.0 milestone Dec 24, 2024
@ZvonimirMedak
Copy link
Collaborator Author

Hi @nikolamajcen,
I've fixed the comments which weren't fixed in the #44 PR. The objc comments, and the warnings are fixed in the next PR. I'm not sure how I didn't catch the models which weren't properly added to the package. Hopefully, now you can test it out properly 😄

I left a question regarding the Extension MARKs, and we can sync later on regarding the fonts. If you want me to merge the #44 before you review this one again just let me know 😄

@nikolamajcen
Copy link
Member

Hey @ZvonimirMedak, I'm fine with merging #44 before this one so that I can check the final implementation with removal and updates for the mentioned parts of the code (e.g., obj-c support, etc.).

Regarding the unused MARKs - you can remove those that are empty (e.g. extension marks in multiple cases).

@ZvonimirMedak
Copy link
Collaborator Author

Hi @nikolamajcen, please take a look again 🤞

@nikolamajcen
Copy link
Member

Additionally, check the Bitrise and fix tests issue - in short, change the current minimum version from iOS 18 to iOS 14.0 on the Sentinel_Tests target.

Everything else looks really nice! Great work 👏

Copy link
Member

@nikolamajcen nikolamajcen left a comment

Choose a reason for hiding this comment

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

Ready to go 🚀

@ZvonimirMedak ZvonimirMedak merged commit 64c7bdf into master Jan 17, 2025
1 check passed
@ZvonimirMedak ZvonimirMedak deleted the feature/swiftui-support branch January 17, 2025 07:09
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