-
Notifications
You must be signed in to change notification settings - Fork 160
Add support for LocalizedStringResource
#315
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
base: main
Are you sure you want to change the base?
Add support for LocalizedStringResource
#315
Conversation
Hi @connor-ricks, thanks for looking into this, but I think it'd be better to add a case to But to do that you would need to hold onto an |
I can give this a stab at some point this week! I'm traveling today. |
Updates LocalizedStringResource usage to use an Any box rather than using the string verbatim. This allows us to use the actual "correct" Text exposed by SwiftUI. - The Any box is required given the minimum version os swift-navigation. Initializing anything else besides a resource would be a programmer error, so encapsulating it in a box allows us to fail in one place rather than having pre-conditions everywhere. - Renames `.localized` to `.localizedStringKey` for clarity.
@mbrandonw let me know your thoughts on these changes. I wrapped things in a "box" so that all the "impossible" places were handled in one location rather than having a couple scattered across the file. I also renamed |
// MARK: - LocalizedStringResourceBox | ||
|
||
fileprivate struct LocalizedStringResourceBox: Equatable, Hashable, @unchecked Sendable { | ||
let value: Any |
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.
Can you use any Sendable
here to get rid of the @unchecked Sendable
?
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.
Turns out I can't actually do this as LocalizedStringResource isn't sendable until iOS 18.
A small PR to add an initializers for
LocalizedStringResource
for convenience.Apple is pushing developers away from
LocalizedStringKey
and instead recommending they useLocalizedStringResource
going forward.There are a variety of benefits of resources over keys mentioned in Apple's docs.
SwiftUI views like
Text
andButton
takeLocalizedStringResource
. While it is easy enough to callString(localized:)
ourselves when usingTextState
, adding this initializer feels like a small amount of effort for a sizable convenience for consumers.