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

Paywalls: prioritize Locale.current over Locale.preferredLocales #3657

Merged
merged 4 commits into from
Feb 8, 2024

Conversation

NachoSoto
Copy link
Contributor

@NachoSoto NachoSoto commented Feb 7, 2024

Fixes #3655.
Follow up to #3633.

This improves upon that PR by using Locale.current.removingRegion for each locale before trying to look up Locale.preferredLocales.

This fixes this scenario:

  • Configure your phone with: en_IN and es_ES
  • Launch paywall with localization en_US and es_ES

Prior to this change, we'd be looking up localizations in this order:

  • en_IN
  • es_ES
  • en (no region)

Because there is a localization for es_ES we'd return that one before attempting to look up Locale.current.removingRegion.

Example of the new log:

VERBOSE: Looking up localized configuration for ["en_IN", "es_ES"], searching for ["en_IN", "en", "es_ES", "es"]
VERBOSE: Found localized configuration for 'en'

Fixes #3655.
Follow up to #3633.

This improves upon that PR by using `Locale.current.removingRegion` **before** trying to look up `Locale.preferredLocales`.

This fixes this scenario:
- Configure your phone with: `en_UK` and `es_ES`
- Launch paywall with localization `en_US` and `es_ES`

Prior to this change, we'd be looking up localizations in this order:
- `en_UK`
- `es_ES`
- `en` (no region)

Because there _is_ a localization for `es_ES` we'd return that one before attempting to look up `Locale.current.removingRegion`.
@samkass
Copy link

samkass commented Feb 8, 2024

This looks like it will likely fix my issue #3655. But I think an optimal solution would be to add the removingRegion for EACH locale in the list, and try it right after the full locale is attempted. So [de-CH, en-UK, it-IT] becomes [de-CH, de, en-UK, en, it-IT, it]. That way, if you had a US English and an Italian localization, it would pick up the English one in the above example based on the UK English preference.

@NachoSoto
Copy link
Contributor Author

That makes sense, I'll make that change 👍🏻

@samkass
Copy link

samkass commented Feb 8, 2024

Awesome. Thanks for you work on this @NachoSoto. Just curious if this will go into a 4.x release of the library. I'm literally waiting on this fix to release my app in Switzerland imminently, and will migrate to 5.x later. Otherwise I'll plan to fork and release on a fork that includes this fix.

@NachoSoto
Copy link
Contributor Author

@samkass updated. We'll release this on the next 4.x release this week 👍🏻

}

return result
return [.current] + Locale.preferredLocales
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This goes back to the original implementation. It allows us to test the actual behavior in localizedConfiguration since it no longer relies on this returning the .removingRegion addition.

Logger.verbose(Strings.paywalls.looking_up_localization(locales))
internal func localizedConfiguration(for preferredLocales: [Locale]) -> LocalizedConfiguration {
// Allows us to search each locale in order of priority, both with the region and without.
// Example: [en_UK, es_ES] => [en_UK, en, es_ES, es]
Copy link
Contributor

Choose a reason for hiding this comment

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

🙌 Makes sense!

@NachoSoto NachoSoto enabled auto-merge (squash) February 8, 2024 16:23
@NachoSoto NachoSoto merged commit 6ef1e5d into main Feb 8, 2024
24 checks passed
@NachoSoto NachoSoto deleted the paywalls-default-region branch February 8, 2024 16:58
NachoSoto pushed a commit that referenced this pull request Feb 8, 2024
**This is an automatic release.**

### RevenueCatUI
* `Paywalls`: prioritize `Locale.current` over `Locale.preferredLocales`
(#3657) via NachoSoto (@NachoSoto)
* `Paywalls`: add logs for localization lookup (#3649) via NachoSoto
(@NachoSoto)
### Dependency Updates
* Bump cocoapods from 1.15.1 to 1.15.2 (#3648) via dependabot[bot]
(@dependabot[bot])
### Other Changes
* `Tests`: fix iOS 15 test crash (#3650) via NachoSoto (@NachoSoto)
* `CircleCI`: remove duplicate `install-dependencies` (#3643) via
NachoSoto (@NachoSoto)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Paywall shows in English despite iPhone being set to German
3 participants