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

Feature branch: per-app language preferences #21538

Merged
merged 60 commits into from
Dec 20, 2024

Conversation

nbradbury
Copy link
Contributor

@nbradbury nbradbury commented Dec 17, 2024

This is the feature branch for per-app language preferences. All code has been reviewed in previous PRs, but a quick test is still recommended:

  • Run on an Android 13+ device
  • Go to App Settings
  • Change the language
  • Verify the change is reflected in the UI
  • Force quit and restart the app and verify the language change is still reflected
  • Do the same on a pre-13 device

@dangermattic
Copy link
Collaborator

dangermattic commented Dec 17, 2024

3 Warnings
⚠️ strings.xml files should only be updated on release branches, when the translations are downloaded by our automation.
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@nbradbury nbradbury requested a review from ParaskP7 December 20, 2024 14:33
@nbradbury nbradbury marked this pull request as ready for review December 20, 2024 14:33
@ParaskP7
Copy link
Contributor

👋 @nbradbury !

Quick question, because I'll be off on AFK from Monday, actually approaching EOD atm, do you need that merged today? 🤔

FYI: I am asking that because it would be valuable for someone other than me to review this final merge to trunk, just so that gets some additional set of eyes on it. Otherwise, I can just proceed with the testing part, just one more time, and approve it to unblock the merge, without much reviewing (did all the reviews in the previous PRs anyway).

@nbradbury
Copy link
Contributor Author

@ParaskP7 If possible, it would be best to get this merged today as I'm AFK all of next week. But perhaps after this is merged, then next week @jkmassel could give testing changing language a spin?

@ParaskP7
Copy link
Contributor

Noted @nbradbury , I'll then test this once more in about an hour (or so) and then (most probably) approve it too (✅), I am sure there are no changes between this and the previous PR we were working on just few minutes ago. 😊

Copy link
Contributor

@ParaskP7 ParaskP7 left a comment

Choose a reason for hiding this comment

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

👋 @nbradbury !

I have reviewed (quickly again) and tested this (once more), everything works as expected, a great great job well done, kudos! 🚀 x 🚀 ^ 🚀

image

So much code removed, love it! ❤️

@nbradbury nbradbury merged commit 01f6aca into trunk Dec 20, 2024
24 checks passed
@nbradbury nbradbury deleted the feature/per-app-language-prefs branch December 20, 2024 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants