Skip to content

Conversation

@SergioEstevao
Copy link
Contributor

@SergioEstevao SergioEstevao commented Jan 7, 2021

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.

| Simulator Screen Shot - iPhone 8 - 2021-01-07 at 14 44 20 | Simulator Screen Shot - iPhone 8 - 2021-01-07 at 14 44 57 |

Notes:

  • This not changes the minimum version supported by the library, Aztec and WordPressEditor are still supporting iOS 11 and above
  • We are only removing the gridicons on Aztec local Example project, the main app will continue to use Gridicons for the legacy editor

To test:

  • 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.
Copy link
Contributor

@loremattei loremattei left a 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.

@SergioEstevao
Copy link
Contributor Author

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.

Good catch @loremattei , I updated the code to remove that warning and removed some missing references to Carthage.

@SergioEstevao
Copy link
Contributor Author

@ceyhun do you mind giving it a look?

Copy link

@ceyhun ceyhun left a 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 :shipit:

@SergioEstevao SergioEstevao merged commit 9b93655 into develop Jan 11, 2021
@SergioEstevao SergioEstevao deleted the issue/replace_gridicons_with_SFSymbols branch January 11, 2021 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment