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

Add objective C support to SwiftGen assets helpers #5533

Merged
merged 2 commits into from
Feb 10, 2022

Conversation

aringenbach
Copy link
Contributor

This adds Objective C annotations and converts SwiftGen assets objects from enum/structs to NSObject classes.
I made the update for most [UIImage imageNamed:] in Objective C classes.

@github-actions
Copy link

github-actions bot commented Feb 9, 2022

📱 Scan the QR code below to install the build for this PR.
🔒 This build is for internal testing purpose. Only devices listed in the ad-hoc provisioning profile can install Element Alpha.

QR code

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/2DtQ4i

Copy link
Member

@stefanceriu stefanceriu left a comment

Choose a reason for hiding this comment

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

Looks great, well done! 👍

The only nitpicks I would comment on are that I would've used the dot notation everywhere and that there's a couple imageNamed: calls left in the RecentRoomTableViewCell and UnifiedSearchViewController

Copy link
Member

@stefanceriu stefanceriu left a comment

Choose a reason for hiding this comment

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

Looks great! 👍

@aringenbach
Copy link
Contributor Author

Made a few changes

  • on the stencil (rename a param and add a missing access identifier)
  • dot notation (my bad on that one, I kinda forgot that ObjC is sometimes convenient)
  • actually just added the Images.swift to the RiotShareExtension target (the same way it's already done for NSE)

One way to improve from there would probably be to have a module/package dedicated to shared contents (in order to exclude properly other targets from snooping around Riot assets), but I guess that might be some work and not a huge impact.

@aringenbach aringenbach merged commit 94dab7a into develop Feb 10, 2022
@aringenbach aringenbach deleted the aringenbach/assets_stencil_objective_c branch February 10, 2022 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants