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 #118: Fill in missing string translations with base translation #470

Merged
merged 7 commits into from
Jun 8, 2024

Conversation

colintheshots-meetup
Copy link
Contributor

If no translation is found, we should fallback to the base translation in the default locale for iOS and MacOS.

@colintheshots-meetup
Copy link
Contributor Author

Will anyone please review this PR? The existing code does not fallback to a base locale.

@pkulikowski
Copy link

@Alex009 Is there any plan to add this? We have this issue that on iOS the app does not return a default localization and we have string keys everywhere. @colintheshots-meetup Will this one fix the iOS issue?

@Alex009
Copy link
Member

Alex009 commented Oct 19, 2023

@Alex009 Is there any plan to add this? We have this issue that on iOS the app does not return a default localization and we have string keys everywhere. @colintheshots-meetup Will this one fix the iOS issue?

hi. this pr fix case when you have in base localization more keys than in other languages. with this fix when you try read some key, that not exist in language X but exist in base localization - you got value from base. at now you got just key, as not found.

i think i include this pr in next release (with kotlin 1.9 support)

@SubhrajyotiSen
Copy link

@Alex009 Is there any pending work on this PR that can push it to get included in the next release? I can work on it if needed

# Conflicts:
#	resources/src/appleMain/kotlin/dev/icerock/moko/resources/desc/PluralFormattedStringDesc.kt
@Alex009
Copy link
Member

Alex009 commented May 16, 2024

@ExNDY need to add new tests with cases when string/plural have base localization, but not have some language. in this case on language that not localized we should receive text from base localization

@SubhrajyotiSen
Copy link

SubhrajyotiSen commented Jun 2, 2024

@Alex009 @ExNDY I've added test cases for this PR at SubhrajyotiSen@5c453a1

Can you please review these? If they work, feel free to cherry-pick. Otherwise I can open a new PR

@ExNDY
Copy link
Contributor

ExNDY commented Jun 6, 2024

Hi, i'm check this changes on samples and... it doesn't work if you has string in base. I'm find solution of this problem, and my opinion: change your fallback on Base, and after add tests for check.

@SubhrajyotiSen
Copy link

SubhrajyotiSen commented Jun 6, 2024

@ExNDY Does the test at SubhrajyotiSen@5c453a1#diff-bcc9dabae360e0fe619920a8ccedbab18f97381771aa23cf20ce544f1abfb1e6R32 not cover the case you mentioned? The string is present in base but not in another language.

Or am I missing a case?

@ExNDY
Copy link
Contributor

ExNDY commented Jun 6, 2024

@SubhrajyotiSen sorry, i'm not check this on current time, later today/tomorow, i think your brach can help me

@ExNDY
Copy link
Contributor

ExNDY commented Jun 6, 2024

@SubhrajyotiSen thanks for tests.

@Alex009 please check this PR, i think we can include him in release

@ExNDY ExNDY requested a review from Alex009 June 6, 2024 07:28
@Alex009 Alex009 merged commit d0f30dd into icerockdev:develop Jun 8, 2024
28 checks passed
@Alex009 Alex009 added this to the 0.24.0 milestone Jun 8, 2024
@Alex009 Alex009 linked an issue Jun 8, 2024 that may be closed by this pull request
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.

Add option to fill in missing string translations with base translation
5 participants