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

Use iOS localization handling for strings. #803

Merged
merged 2 commits into from
Apr 17, 2023
Merged

Conversation

pixlwave
Copy link
Member

@pixlwave pixlwave commented Apr 14, 2023

Previously we had our own calculatePreferredLanguages method based upon Locale.preferredLanguages to decide suitable languages for strings and then before translating we were doing additional work to manage locale regions if necessary.

Turns out that Bundle has a method called preferredLocalizations which does this for us and makes sure to only provide a single language (plus any regional variations) so you don't end up with the app showing strings in 3 or more languages.

I've removed Bundle.elementFallbackLanguage in favour of the developmentLocalization property, but left in an override so that we can still run unit tests.

There's a run of UI tests going on here, to make sure this works there too

@pixlwave pixlwave requested a review from Velin92 April 14, 2023 17:31
@pixlwave pixlwave requested a review from a team as a code owner April 14, 2023 17:31
@pixlwave pixlwave requested review from stefanceriu and removed request for a team and stefanceriu April 14, 2023 17:31
@github-actions
Copy link

github-actions bot commented Apr 14, 2023

Warnings
⚠️ Some of the commits are missing ticket numbers. Please consider squashing all commits that don't have a tracking number.
⚠️

ElementX/Sources/Services/BugReport/BugReportService.swift#L82 - SwiftLint rule 'function_body_length' did not trigger a violation in the disabled region. Please remove the disable command. (superfluous_disable_command)

Generated by 🚫 Danger Swift against 38d40f1

@pixlwave pixlwave force-pushed the doug/localizations branch 2 times, most recently from 0139208 to e02d356 Compare April 14, 2023 17:41
@codecov
Copy link

codecov bot commented Apr 14, 2023

Codecov Report

Patch coverage: 50.00% and project coverage change: -0.31 ⚠️

Comparison is base (3d0d883) 52.07% compared to head (e02d356) 51.77%.

❗ Current head e02d356 differs from pull request most recent head 38d40f1. Consider uploading reports for the commit 38d40f1 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #803      +/-   ##
===========================================
- Coverage    52.07%   51.77%   -0.31%     
===========================================
  Files          297      297              
  Lines        16544    16528      -16     
  Branches      9923     9914       -9     
===========================================
- Hits          8616     8557      -59     
- Misses        7678     7721      +43     
  Partials       250      250              
Flag Coverage Δ
uitests 36.46% <44.44%> (-0.02%) ⬇️
unittests 25.15% <50.00%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
ElementX/Sources/Application/AppCoordinator.swift 56.72% <ø> (-3.14%) ⬇️
...ces/Notification/Manager/NotificationManager.swift 87.60% <0.00%> (ø)
.../Sources/Services/BugReport/BugReportService.swift 69.56% <33.33%> (-0.49%) ⬇️
ElementX/Sources/Other/Extensions/Bundle.swift 76.00% <57.14%> (-8.62%) ⬇️

... and 3 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@Velin92 Velin92 left a comment

Choose a reason for hiding this comment

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

LGTM just one question and one suggestion

@pixlwave pixlwave force-pushed the doug/localizations branch from e02d356 to 38d40f1 Compare April 17, 2023 14:04
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@pixlwave pixlwave enabled auto-merge (squash) April 17, 2023 14:07
@pixlwave pixlwave merged commit d01349a into develop Apr 17, 2023
@pixlwave pixlwave deleted the doug/localizations branch April 17, 2023 14:58
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