-
Notifications
You must be signed in to change notification settings - Fork 150
Replace gridicons with SFSymbols on the Example app #1319
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
Replace gridicons with SFSymbols on the Example app #1319
Conversation
loremattei
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me and I like that we can simplify the build system removing Carthage.
✅ Check if the project builds correctly locally.
✅ Check if the Example app runs and you can use the toolbar to edit
✅ Check the overlays on the image using the Image Overlay sample content.
I noticed that there's a Warning when building the Example app because of the switch to iOS 13:
AztecEditor-iOS/Example/Example/EditorDemoController.swift:455:22: 'init(input:modifierFlags:action:discoverabilityTitle:)' was deprecated in iOS 13.0.
Since it's a deprecation notice and it's on the sample app, I'm approving the PR anyway, but I'm reporting it as we may want to fix it in this PR.
Good catch @loremattei , I updated the code to remove that warning and removed some missing references to Carthage. |
|
@ceyhun do you mind giving it a look? |
ceyhun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with Xcode 12.3 and Example app built fine without Carthage, icons in toolbar and image overlays looking good. The project builds fine as well ![]()
Refs: #1224
This PR replaces the use of Gridicons on the example app with SFSymbols.
This will remove the only third party dependency of this project and will also allow us to remove the use of Carthage on the build system.
|
|
|
Notes:
To test: