-
Notifications
You must be signed in to change notification settings - Fork 526
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Browse the repository at this point in the history
) ## Explanation Fixes #4628 Fixes #4667 The core fix for #4628 is splitting the language configuration between "all languages the app supports" and "production-ready languages." This is now configurable per-binary where all pre-beta binaries include all languages, and the beta & GA binaries only include production-ready languages (those are, languages for which we always aim to have 100% content translations). Tests use the "all languages" configuration, by default, for parity with the developer version of the app. This required a small change where we define the language proto as part of the shared ``//model`` module rather than as a separate package. As was before, this proto is only loaded in Bazel builds of the app since our Gradle pipeline doesn't support automatically textproto->binary proto conversion during build time. To ensure that the app strings are filtered out, the supported languages configuration proto that's included in the app is passed through a new script that strips out all resources (including non-strings) from the binary. This has actually removed ~400KiB even from the pre-release binaries since our third-party assets include a bunch of strings outside our core languages. New output now shows when building the app with Bazel to make it clearer what sorts of resources are being dropped due to incompatible languages: ``` 82 resources are being removed that are tied to unsupported languages: [af, am, as, az, be, bg, bn, bs, ca, cs, da, de, el, en-AU, en-CA, en-GB, en-IN, en-XC, es, es-419, es-US, et, eu, fa, fi, fr, fr-CA, gl, gu, hr, hu, hy, in, is, it, iw, ja, ka, kk, km, kn, ko, ky, lo, lt, lv, mk, ml, mn, mr, ms, my, nb, ne, nl, or, pa, pl, pt, pt-PT, ro, ru, si, sk, sl, sq, sr, sr-Latn, sv, ta, te, th, tl, tr, uk, ur, uz, vi, zh-CN, zh-HK, zh-TW, zu] (size reduction: 391239 bytes). ``` (When building oppia_dev) ``` 85 resources are being removed that are tied to unsupported languages: [af, am, ar, as, az, be, bg, bn, bs, ca, cs, da, de, el, en-AU, en-CA, en-GB, en-IN, en-XC, es, es-419, es-US, et, eu, fa, fi, fr, fr-CA, gl, gu, hi, hr, hu, hy, in, is, it, iw, ja, ka, kk, km, kn, ko, ky, lo, lt, lv, mk, ml, mn, mr, ms, my, nb, ne, nl, or, pa, pl, pt, pt-PT, ro, ru, si, sk, sl, sq, sr, sr-Latn, sv, sw, ta, te, th, tl, tr, uk, ur, uz, vi, zh-CN, zh-HK, zh-TW, zu] (size reduction: 351087 bytes). ``` (When building oppia_beta) #4667 is included in this PR despite it being an asset downloader-only fix out of convenience & since it's a related issue. The problem in the downloader was that translations were being incorrectly copied over to the new proto structure for lessons (which isn't used in the app yet, but is used for image downloading) which led to non-English images not being scraped from content HTMLs and thus not downloaded and included in the embedded assets for releases. We'll be adding a manual QA test to make sure that non-English content images load correctly. ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only Screenshots showing how English, Brazilian Portuguese, and Swahili are handled with the changes in this PR (between developer and beta builds of the app): | | Dev flavor -- with PR changes | Beta flavor -- with PR changes | |----------------------|-----------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------| | English |  |  | | Brazilian Portuguese |  |  | | Swahili |  |  | As can be seen from the table, all three languages are fully supported except when using beta with the changes introduced by this PR (where Swahili is dropped since it's not yet a 100% supported, production-ready language for the app). Note that in the case of Swahili, it falls back to English simply because that's how my device was configured at the time (it will fall back to whatever the system-secondary language is set to). Note that other typical screenshots/tests are not included in this PR, including for Espresso. From a high-level perspective, the main difference in this PR is which app and content strings are ultimately included and supported within the app. Given this is inclusion/exclusion, the screenshots above + manual verification via e2e tests are sufficient. Beyond that, properly testing these changes would require e2e tests that can use the production assets (which current Espresso tests don't have access to).
- Loading branch information
1 parent
76bf81a
commit fea5b7e
Showing
30 changed files
with
1,087 additions
and
42 deletions.
There are no files selected for viewing
Validating CODEOWNERS rules …
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
File renamed without changes.
59 changes: 59 additions & 0 deletions
59
config/src/java/org/oppia/android/config/productionlanguages/supported_languages.textproto
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| language_definitions { | ||
| language: ENGLISH | ||
| min_android_sdk_version: 1 | ||
| app_string_id { | ||
| ietf_bcp47_id { | ||
| ietf_language_tag: "en" | ||
| } | ||
| android_resources_language_id { | ||
| # Note that while English is the default language for Oppia & no special language code is | ||
| # needed for it, "en" is still used here for consistency. Technically this will match strings | ||
| # against "values-en", but because the team will never define strings with that qualifier they | ||
| # will always fallback to the default English strings under "values". | ||
| language_code: "en" | ||
| } | ||
| } | ||
| content_string_id { | ||
| ietf_bcp47_id { | ||
| ietf_language_tag: "en" | ||
| } | ||
| } | ||
| audio_translation_id { | ||
| ietf_bcp47_id { | ||
| ietf_language_tag: "en" | ||
| } | ||
| } | ||
| } | ||
| language_definitions { | ||
| language: PORTUGUESE | ||
| min_android_sdk_version: 1 | ||
| content_string_id { | ||
| ietf_bcp47_id { | ||
| ietf_language_tag: "pt" | ||
| } | ||
| } | ||
| } | ||
| language_definitions { | ||
| language: BRAZILIAN_PORTUGUESE | ||
| fallback_macro_language: PORTUGUESE | ||
| min_android_sdk_version: 1 | ||
| app_string_id { | ||
| ietf_bcp47_id { | ||
| ietf_language_tag: "pt-BR" | ||
| } | ||
| android_resources_language_id { | ||
| language_code: "pt" | ||
| region_code: "BR" | ||
| } | ||
| } | ||
| content_string_id { | ||
| ietf_bcp47_id { | ||
| ietf_language_tag: "pt-BR" | ||
| } | ||
| } | ||
| audio_translation_id { | ||
| ietf_bcp47_id { | ||
| ietf_language_tag: "pt-BR" | ||
| } | ||
| } | ||
| } |
22 changes: 22 additions & 0 deletions
22
config/src/java/org/oppia/android/config/productionlanguages/supported_regions.textproto
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| region_definitions { | ||
| region: BRAZIL | ||
| region_id { | ||
| ietf_region_tag: "BR" | ||
| } | ||
| languages: PORTUGUESE | ||
| languages: BRAZILIAN_PORTUGUESE | ||
| } | ||
| region_definitions { | ||
| region: UNITED_STATES | ||
| region_id { | ||
| ietf_region_tag: "US" | ||
| } | ||
| languages: ENGLISH | ||
| } | ||
| region_definitions { | ||
| region: KENYA | ||
| region_id { | ||
| ietf_region_tag: "KE" | ||
| } | ||
| languages: ENGLISH | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.