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

Adds Support for Swift Package Manager #208

Merged
merged 11 commits into from
Apr 12, 2024
Merged

Conversation

jaerod95
Copy link
Contributor

Adds support for SPM. See README.md for instructions on how to add it to your project.

Finally got around to fixing this up again. Now that Apple supports Resources I was able to import the Localizations. I added to the LTHPasscodeViewControllerStrings method to search for the SPM bundle if it didn't find a localized string since Apple moves those localizations into its own bundle.

Copy link
Owner

@rolandleth rolandleth left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution! Just a few changes to be made, please.

Also, could you please explain a bit the need for the LTHPasscodeViewControllerStrings change? Haven't worked with SPM yet, so no idea what the issue was and how it got solved.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
LTHPasscodeViewController/LTHPasscodeViewController.m Outdated Show resolved Hide resolved
Package.swift Outdated Show resolved Hide resolved
Package.swift Outdated Show resolved Hide resolved
@jaerod95
Copy link
Contributor Author

The reason I needed to change the LTHPasscodeViewControllerStrings was due to how the new SPM tooling processes resources. Basically it compiles them into a bundle and exposes that bundle to Swift and Objc. I gathered most of my information on what that looks like from this article: https://darjeelingsteve.com/articles/How-to-Use-Module-Resources-in-Objective-C-SPM-Packages.html

@rolandleth
Copy link
Owner

Thanks for sharing the article!

Hmm, is there any way to test the package before merging? Can I add it to a project by entering the URL of your dev branch? Not sure what the syntax would be, maybe https://github.com/SimpleNexus/LTHPasscodeViewController/tree/dev?

@jaerod95
Copy link
Contributor Author

jaerod95 commented Apr 8, 2021

Thanks for sharing the article!

Hmm, is there any way to test the package before merging? Can I add it to a project by entering the URL of your dev branch? Not sure what the syntax would be, maybe https://github.com/SimpleNexus/LTHPasscodeViewController/tree/dev?

Yes! You can simply add https://github.com/SimpleNexus/LTHPasscodeViewController as the git source, then on the rules page, just select branch and point it at dev as seen in the picture below:
Screen Shot 2021-04-08 at 10 16 45 AM

@rolandleth
Copy link
Owner

Cool, I'll give it a go today, thanks!

@rolandleth
Copy link
Owner

Hey! Very sorry for the long delay, but I managed to give it a go today. I added the package to a Swift project and everything works, except localization. Maybe I'm not doing it properly, but I set the project to use English and German, I added a Localizable.strings file, but the passcode view isn't localized.

Either I'm doing something wrong, or the macro isn't working properly 🤔🤔 Can you get localization to work when using the passcode as a package?

The demo project is properly localized.

@jaerod95
Copy link
Contributor Author

jaerod95 commented Dec 6, 2021

@rolandleth Sorry for the delay, got distracted for a quick....8 months haha. However, I updated the code to work with localization (nice catch by the way). There was an issue where I was looking in the wrong bundle. The last two commits fix that. Let me know if there are any other issues.

@jaerod95 jaerod95 requested a review from rolandleth December 6, 2021 16:58
Jason Rodriguez and others added 2 commits December 7, 2021 09:39
I was testing localization and set the Touch ID string as the value for the string to use. I have since removed this and use the key parameter instead
@rolandleth
Copy link
Owner

@jaerod95 This has to be the slowest moving PR in history 🙈 I tested it and it works, I think it's good to merge, thank you!

@rolandleth rolandleth merged commit 85f4f79 into rolandleth:master Apr 12, 2024
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