-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[Audit][Implementation] - Use only supported locales to reduce the APK size #39688
[Audit][Implementation] - Use only supported locales to reduce the APK size #39688
Conversation
…y-App into audit/app-startup/use-only-required-locales
@allroundexperts Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
@hurali97 Conflicts. |
…y-App into audit/app-startup/use-only-required-locales
@allroundexperts resolved ✅ |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2024-04-08.at.7.25.54.PM.movAndroid: mWeb ChromeScreen.Recording.2024-04-08.at.7.24.41.PM.moviOS: NativeScreen.Recording.2024-04-08.at.7.23.52.PM.moviOS: mWeb SafariScreen.Recording.2024-04-08.at.7.21.43.PM.movMacOS: Chrome / SafariScreen.Recording.2024-04-08.at.7.17.45.PM.movMacOS: DesktopScreen.Recording.2024-04-08.at.7.20.15.PM.mov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works good. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hurali97 sorry to not mention this sooner, but I think Android provides a method of automatically removing unused languages here.
Can we use that instead please, because someone trying to add a new language that is unfamiliar with the gradle config is going to waste a lot of time trying to find out why the Android app keeps stripping out the language resources 😄
Hey @Julesssss I went through automatic per-app language support and it looks like it's designed for android only projects. Which means the projects which keep their language resources in this format This is to only enable the language switcher for the app from settings and it doesn't remove any other unused locales from the dependencies, so it's not worth adding. If we are concerned about adding new language in the future, can we update |
@hurali97 ah okay no worries and thanks for explaining. That's a shame. I'm still worried that this is going to confuse people who add languages in the future, but maybe a comment above the Gradle config would make this a bit easier to locate. Something like this maybe: |
Yes makes sense, I will add it 👍 |
Conflicts resolved on behalf of @hurali97 as he is out of office. Feel free to hit me up anytime needed with the PR 👍 |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/Julesssss in version: 1.4.62-0 🚀
|
1 similar comment
🚀 Deployed to staging by https://github.com/Julesssss in version: 1.4.62-0 🚀
|
🚀 Deployed to staging by https://github.com/Julesssss in version: 1.4.62-0 🚀
|
🚀 Deployed to production by https://github.com/thienlnam in version: 1.4.62-17 🚀
|
Details
This PR is part of Audit by callstack and is concerned with reducing the APK size. Since it's regarding APK size, it only affects android.
Applications often have dependencies which supports multiple locales. These translations strings are then made part of the APK that is installed by the user. So the user is installing an APK which has translations which he will not even use because it's not supported by the application. For eg, Expensify only supports
en
andes
, so there's no point compiling all other locales in the APK.Take a look at below screen shot, which has whole lot of other locales like Arabic which we don't even need. They come as part of google-play-services-core, material etc.
We can remove all the non-supported locales by explicitly telling android build tools that we support only
en
andes
. See below how unused strings are then removed fromresources.arsc
We managed to reduce the
resources.arsc
from 3.4MB to 1MB in raw file size. Let's see the impact of this on the APK size. We will benchmark this on the results gained from this PR, they are not related but to only show the final benchmarks from these two PRs:Before ( linked PR ):
After:
We get 2.3 MB of reduction in APK size and 0.6 MB of reduction in Download size 🚀
Fixed Issues
$ #39700
PROPOSAL: #39700
Tests
Testing Steps:
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Screen.Recording.2024-04-05.at.3.25.53.PM.mov
Android: mWeb Chrome
N/A
iOS: Native
N/A
iOS: mWeb Safari
N/A
MacOS: Chrome / Safari
N/A
MacOS: Desktop
N/A