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

Fix LocText translation handling #15

Merged
merged 2 commits into from
Jul 8, 2024

Conversation

jabou
Copy link
Member

@jabou jabou commented Jun 12, 2024

Intro

This PR is a quick hotfix for handling LocText translation in case where we have to modify the underlying Text by calling the public var text: Text property.
In the future, we can see if we can remove that property from the lib, but in that case, we will have to introduce support for setting some properties on underlying Text.

Issue

In case we use public var text: Text from the LocText directly (use-case example later), we will get the error:
Screenshot 2024-06-12 at 16 19 19

This will result in a not translated LocText element (only the default language will be set; a language change will not take effect).

Fix

To fix this behavior, I replaced @Environment(\.language) base handling with completion base one. I can even remove LanguageModifier if we don't need it (not sure if someone is using it?), and we can call .onReceive directly on LocText text inside the body.
That way, when a language change occurs and a notification is fired, @State will be updated, and we will have a correct translation on the next body render.

Use Case

In case when we want to set kerning and lineSpacing on font, we have to access Text directly as those modifiers are not available on Content (e.g., creating the ViewModifier) or View (i.e., on LocText). In that case, we can use the existing public var text: Text property on which we can then apply kerning and lineSpacing. Example:

extension LocText {

    func appFont(_ fontStyle: AppFontStyle) -> some View {
        return text // returns `Text`
            .font(fontStyle.font)
            .kerning(fontStyle.style.kern) // available only on `Text`
            .lineSpacing(fontStyle.lineSpacing)
            .padding(.vertical, fontStyle.lineSpacing / 2)
    }
}

@jabou jabou self-assigned this Jun 12, 2024
@jabou
Copy link
Member Author

jabou commented Jun 13, 2024

We will test this with the Alps Alpine regression test. If everything goes well, I'll merge and publish it.

@jabou jabou merged commit d18e84a into master Jul 8, 2024
1 check passed
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.

3 participants