Fix splash screen logo size jump on Samsung OneUI v4 #28644

zoontek · 2023-10-02T18:57:15Z

Details

This PR introduces a specific fix for Samsung OneUI v4.

As mentioned here, Samsung SplashScreen API implementation is incorrect on Android 12 (OneUI v4), it appears that the logo size seems 2x smaller than other manufacturers.

Samsung seems to have corrected the issue on OneUI v5.

References:

As we cannot fix OS implementation, the solution here consist in two parts:

If the phone is running OneUI v4, we apply a custom bootsplash with 2x smaller logo, aligned with the system handled splash screen logo size
We also pass a constant, logoSizeRatio of 0.5 in such cases, to properly set SVG logo dimensions on the animated splash screen

⚠️ As I don't have concerned testing devices, don't hesitate to ping me if the ratio of 0.5 is incorrect (I used video captures to determine it)

This PS also fix a regression on the splash screen logo. A few changes have been made by @kowczarz, and the version currently on main is incorrect, making a 1px shift and the logo being cut on the right.

Fixed Issues

$ #26681

Tests

Open the app on a Samsung with OneUI v4 (android 12): the logo size is 2x smaller, but at least there's no size jump
Open the app on other android devices (non Samsung, Samsung using other OneUI versions): the logo size is correct, there's no size jump

Offline tests

QA Steps

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

⚠️ As I don't have concerned testing devices, I had to invert the isSamsungOneUI4 result in order to test the issue case. That's why in this video, the system handled splash screen show a correctly sized logo, then the logo become smaller. By keeping isSamsungOneUI4 untouched, it stays smaller on Samsung OneUI v4 and correctly sized on other devices.

screen-20231002-203149.mp4

melvin-bot · 2023-10-02T18:57:20Z

@ArekChr 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]

zoontek · 2023-10-02T19:08:06Z

android/app/src/main/res/values/styles.xml

@@ -72,7 +72,7 @@

    <style name="BootTheme" parent="Theme.SplashScreen">
        <item name="android:statusBarColor">@android:color/transparent</item>
-        <item name="windowSplashScreenAnimatedIcon">@mipmap/bootsplash_logo</item>
+        <item name="windowSplashScreenAnimatedIcon">@drawable/bootsplash_logo</item>


I moved the bootsplash logo from mipmap-* to drawable-* directories to align with react-native-bootsplash v5 and AndroidX core-splashscreen

Julesssss · 2023-10-03T08:00:03Z

android/app/src/main/java/com/expensify/chat/bootsplash/BootSplashModule.java

+  // From https://stackoverflow.com/a/61062773
+  public static boolean isSamsungOneUI4() {
+    String name = "SEM_PLATFORM_INT";
+
+    try {
+      Field field = Build.VERSION.class.getDeclaredField(name);
+      int version = (field.getInt(null) - 90000) / 10000;
+      return version == 4;
+    } catch (Exception ignored) {
+      return false;
+    }
+  }


I understand not wanting to bundle this in the library, but just curious if you thought about this?

I will apply the same in react-native-bootsplash in a upcoming version.

This is not pretty, but it's extracted from a Samsung APK so it should be correct, and we unfortunately need it 😕

I have a similar method in react-native-localize for Xiaomi devices. I always rather use a workaround than let users reporting issues, as they don't know that it's their phone Android implementation fault.

I often wish Google enforce Android stock usage (without any modifications) to manufacturers, but it's a wild dream 😅

I will apply the same in react-native-bootsplash in a upcoming version.

Okay, that's great. Can you let us know when this is ready so that we can update our code?

I often wish Google enforce Android stock usage

As an ex-native Android dev I 100% agree.

@Julesssss Yes, I will. The module in Expensify is based on my initial work on v5, but has been merged way before I released the latest version (as I added support for brand image, dark mode, web, in this release)

Once bootsplash catch up on this, I will do a proposal to migrate Expensify back to it (and delete the custom module in your codebase). I know that light / dark mode support is planned on your side + you will be able to remove your web custom implementation.

Meanwhile, I'd rather be able to merge and check that this indeed fix the Samsung issue (as I don't have such test devices)

OSBotify · 2023-10-03T08:27:20Z

🧪🧪 Use the links below to test this build in android and iOS. Happy testing! 🧪🧪

android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/28644/index.html	https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/28644/index.html

desktop 💻	web 🕸️
https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/28644/NewExpensify.dmg	https://28644.pr-testing.expensify.com

Julesssss · 2023-10-13T15:32:04Z

Bumping for review @ArekChr 👍

aimane-chnaif · 2023-10-13T16:01:26Z

I can help review as I have various Samsung devices to test.

Galaxy S10: (One UI 4.1)

XRecorder_03102023_014232.mp4

Galaxy Tab A7: (One UI 4.1)

XRecorder_13102023_164713.mp4

Galaxy S8+ (One UI 1.0)

20231003_015816.mp4

aimane-chnaif

Works as expected but is it possible to increase system logo size by 2x to be same as zoomed one?

We can't really fix the initial size during the launch sequence, as it's handled by the OS (OneUI v4). So I apply a dedicated background drawable in such case (with 144dp logo) + pass a 0.5 logo size ratio to the react-native app, which will update the logo accordingly (react-native-bootsplash offers a hook, useHideAnimation, that handles that for you)

I understand the limitation but I checked other apps. Slack logo size looks good

XRecorder_03102023_021311.mp4

zoontek · 2023-10-13T16:36:11Z

Slack doesn't seem to keep the splash screen, we see a black screen before the app first render. I'm sure there's other hacks (maybe by decompiling some APKs?), but does it worth the effort as it only impacts Samsungs stucked on Android 12 (?)

trim.24B9C8E8-FE9A-4963-8271-5687DDB9E8AD.MOV

aimane-chnaif · 2023-10-13T16:37:50Z

Slack doesn't seem to keep the splash screen, we see a black screen before the app first render. I'm sure there's other hacks (maybe by decompiling some APKs?), but does it worth the effort as it only impacts Samsungs stucked on Android 12 (?)

It's not worth 😄

Julesssss · 2023-10-16T08:11:29Z

Hey @aimane-chnaif, thanks for testing. If you could complete the checklist I'll assign you as the C+ 👍

aimane-chnaif · 2023-10-16T08:24:36Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android / native
- Android / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

web.mov

Mobile Web - Chrome

mchrome.mov

Mobile Web - Safari

msafari.mov

Desktop

desktop.mov

iOS

iPhone 14 Plus

ios.mov

iPhone SE

ios2.mov

Android

Galaxy S10: (One UI 4.1)

XRecorder_03102023_014232.mp4

Galaxy Tab A7: (One UI 4.1)

XRecorder_13102023_164713.mp4

Galaxy S8+ (One UI 1.0)

20231003_015816.mp4

Julesssss

@aimane-chnaif I'd also like to see iOS tested as we did change native code.

aimane-chnaif

Confirmed fix on One UI 4.x devices.
And other devices are not affected by this PR expect the fix of right edge cut.

aimane-chnaif · 2023-10-16T08:28:36Z

@aimane-chnaif I'd also like to see iOS tested as we did change native code.

On it now

aimane-chnaif · 2023-10-16T09:05:36Z

All platforms are tested and updated checklist

Julesssss · 2023-10-16T09:22:44Z

@aimane-chnaif thank you!

Julesssss

Excellent, thanks all!

OSBotify · 2023-10-16T09:29:21Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2023-10-16T21:48:48Z

🚀 Deployed to staging by https://github.com/Julesssss in version: 1.3.85-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2023-10-17T23:13:33Z

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.85-4 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

OSBotify · 2023-10-17T23:59:57Z

🚀 Deployed to staging by https://github.com/Julesssss in version: 1.3.86-0 🚀

platform	result
🤖 android 🤖	failure ❌
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2023-10-18T23:57:27Z

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.86-5 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

zoontek added 6 commits October 2, 2023 19:31

Add isSamsungOneUI4 method

f788f2e

Move bootsplash logo from mipmap to drawable

30eb8d7

Apply custom bootsplash layout for Samsung OneUI 4

c202b22

Add logoSizeRatio constant

6ca78e6

Apply logo size ratio on splash screen logo

357d7b6

Fix splash screen logo svg file

4daaf3b

zoontek requested a review from a team as a code owner October 2, 2023 18:57

melvin-bot bot requested review from ArekChr and removed request for a team October 2, 2023 18:57

zoontek referenced this pull request Oct 2, 2023

Fix production logo flashing

282105f

zoontek mentioned this pull request Oct 2, 2023

[HOLD for payment 2023-10-24] [$1000] Android - Expensify logo zooms in when the app is opened #26681

Closed

6 tasks

zoontek commented Oct 2, 2023

View reviewed changes

Julesssss assigned zoontek Oct 3, 2023

Julesssss added the Ready To Build label Oct 3, 2023

Julesssss reviewed Oct 3, 2023

View reviewed changes

aimane-chnaif reviewed Oct 13, 2023

View reviewed changes

Julesssss removed the request for review from ArekChr October 16, 2023 08:09

Julesssss reviewed Oct 16, 2023

View reviewed changes

aimane-chnaif approved these changes Oct 16, 2023

View reviewed changes

melvin-bot bot requested a review from Julesssss October 16, 2023 08:28

Julesssss approved these changes Oct 16, 2023

View reviewed changes

Julesssss merged commit e38b53f into Expensify:main Oct 16, 2023
16 of 23 checks passed

zoontek deleted the fix-oneui4-splashscreen branch October 16, 2023 11:37

OSBotify mentioned this pull request Oct 16, 2023

Deploy Checklist: New Expensify 2023-10-16 #29717

Closed

zoontek mentioned this pull request Oct 17, 2023

Logo is half size on Samsung Galaxy device zoontek/react-native-bootsplash#466

Closed

OSBotify mentioned this pull request Oct 17, 2023

Deploy Checklist: New Expensify 2023-10-17 #29837

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix splash screen logo size jump on Samsung OneUI v4 #28644

Fix splash screen logo size jump on Samsung OneUI v4 #28644

zoontek commented Oct 2, 2023

melvin-bot bot commented Oct 2, 2023

zoontek Oct 2, 2023

Julesssss Oct 3, 2023

zoontek Oct 3, 2023

Julesssss Oct 3, 2023

zoontek Oct 3, 2023 •

edited

Loading

OSBotify commented Oct 3, 2023

Julesssss commented Oct 13, 2023

aimane-chnaif commented Oct 13, 2023 •

edited

Loading

aimane-chnaif left a comment •

edited

Loading

zoontek commented Oct 13, 2023

aimane-chnaif commented Oct 13, 2023

Julesssss commented Oct 16, 2023

aimane-chnaif commented Oct 16, 2023 •

edited

Loading

Julesssss left a comment

aimane-chnaif left a comment

aimane-chnaif commented Oct 16, 2023

aimane-chnaif commented Oct 16, 2023

Julesssss commented Oct 16, 2023

Julesssss left a comment

OSBotify commented Oct 16, 2023

OSBotify commented Oct 16, 2023

OSBotify commented Oct 17, 2023

OSBotify commented Oct 17, 2023

OSBotify commented Oct 18, 2023

Fix splash screen logo size jump on Samsung OneUI v4 #28644

Fix splash screen logo size jump on Samsung OneUI v4 #28644

Conversation

zoontek commented Oct 2, 2023

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Android

melvin-bot bot commented Oct 2, 2023

zoontek Oct 2, 2023

Choose a reason for hiding this comment

Julesssss Oct 3, 2023

Choose a reason for hiding this comment

zoontek Oct 3, 2023

Choose a reason for hiding this comment

Julesssss Oct 3, 2023

Choose a reason for hiding this comment

zoontek Oct 3, 2023 • edited Loading

Choose a reason for hiding this comment

OSBotify commented Oct 3, 2023

Julesssss commented Oct 13, 2023

aimane-chnaif commented Oct 13, 2023 • edited Loading

aimane-chnaif left a comment • edited Loading

Choose a reason for hiding this comment

zoontek commented Oct 13, 2023

aimane-chnaif commented Oct 13, 2023

Julesssss commented Oct 16, 2023

aimane-chnaif commented Oct 16, 2023 • edited Loading

Reviewer Checklist

Screenshots/Videos

Julesssss left a comment

Choose a reason for hiding this comment

aimane-chnaif left a comment

Choose a reason for hiding this comment

aimane-chnaif commented Oct 16, 2023

aimane-chnaif commented Oct 16, 2023

Julesssss commented Oct 16, 2023

Julesssss left a comment

Choose a reason for hiding this comment

OSBotify commented Oct 16, 2023

OSBotify commented Oct 16, 2023

OSBotify commented Oct 17, 2023

OSBotify commented Oct 17, 2023

OSBotify commented Oct 18, 2023

zoontek Oct 3, 2023 •

edited

Loading

aimane-chnaif commented Oct 13, 2023 •

edited

Loading

aimane-chnaif left a comment •

edited

Loading

aimane-chnaif commented Oct 16, 2023 •

edited

Loading