feat: edge-to-edge mode on Android#52392
Conversation
|
@shawnborton would it be possible to ask you to test this PR? This PR should unify how Android app is handling insets (now it'll be identical to iOS). This PR is a logical continuation of this #51956 but the key difference is that now we don't manage color of navigation bar independtly - now we make all system bars (navigation bar, status bar) transparent by default, so it matches iOS app style. The reason why I want you to test and give your feedback, is because some elements look good on iOS, but looks not very good on Android. For example, navigation bar padding on Android vs iOS:
This is the one issue I spotted at the moment, but maybe you find more - your feedback is very valuable 🙏 @mountiny would it be possible to trigger a build so that @shawnborton can test the app? |
|
I can trigger a test build. I don't have an Android with me at the moment but I believe @dubielzyk-expensify can help us test! |
Sounds good! Any look from UX/UI team will help a lot ❤️ |
|
We might need to take the PR out of draft in order to run the test build. If we don't get a test build posted in the next 20 minutes or so, let's try that. |
This comment has been minimized.
This comment has been minimized.
|
Well wouldya just look at that, ready for testing. |
|
Assigned @c3024 for c+ review |
|
Nice testing Jon, thanks for running through the screens and I agree with your feedback. |
|
Awesome @dubielzyk-expensify ! 🔥 Thank you for your feedback! |
|
Do we need a new build @kirillzyusko |
|
@mountiny no, not yet. Tomorrow I want to test on more devices and if everything works well, then I will ask for additional testing 😊 |
package.json
Outdated
| "expo-av": "14.0.7", | ||
| "expo-image": "1.12.15", | ||
| "expo-image-manipulator": "12.0.5", | ||
| "expo-navigation-bar": "~3.0.7", |
There was a problem hiding this comment.
I had to add this library again, because we need to manage navigation bar button style (dark/light).
Since user can customize the theme of app independently to device settings we also have to manage it dynamically from JS code.
I checked and one method that we use (setButtonStyleAsync) doesn't use deprecated API. However I remember, that this particular library wasn't accepted in the first round of fixing the nav bar color management, so i'm happy to discuss further steps here 🙌
There was a problem hiding this comment.
@kirillzyusko I noticed you are adding the library here again even though in Slack we agreed not to use it https://expensify.slack.com/archives/C01GTK53T8Q/p1731618817760309?thread_ts=1731092817.909359&cid=C01GTK53T8Q
Can you please come back to the thread and summarize the problems you are facing with not using the library or explain clearly why we cannot just handle this on our own? Thanks!
There was a problem hiding this comment.
Thanks Vit, I missed this but agree with needing to discuss this further.
|
Mobile-Expensify pr merged |
|
✋ 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/mountiny in version: 9.0.67-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.0.67-9 🚀
|
|
Coming from this issue #53180 , we were using the height and width values from the useWindowDimensions hook, which caused extra height above the suggestion modal. To address this, we have switched to using useWindowDimensions from 'react-native' instead context on this is here #53180 (comment). |
| @@ -0,0 +1,33 @@ | |||
| import {KeyboardController, KeyboardEvents} from 'react-native-keyboard-controller'; | |||
There was a problem hiding this comment.
Due to newer implementation of native-stack, the KeyboardController is not working properly. This caused #53351
|
This issue was caused by the changes introduced in this PR. |
|
Coming from checklist #53182, the screen height with the keyboard shown is delayed after navigating to another screen, so we should handle hiding the keyboard before navigating to another screen, similar to the change in FormProvider. |
| const subscription = KeyboardEvents.addListener('keyboardDidHide', () => { | ||
| resolve(undefined); | ||
| subscription.remove(); |
There was a problem hiding this comment.
On native, modal screens does not blur the input which will not trigger this event. It prevents the FormProvider to hit submit callback https://github.com/Expensify/App/pull/52392/files#diff-16b60eed4ca05874fc3fadcb18fc790efcb94648af3991004dfa67c7e7f39923R210.
It caused #56696







Explanation of Change
KeyboardAvoidingViewfor Android - ideally we should use the implementation fromreact-native-keyboard-controller, but in this case animations have a very poor performance and it's better to investigate/fix as a separate issueModal(that addsnavigationBarTranslucentproperty - applied forreact-nativeandreact-native-modal)Linked HybridApp PR: https://github.com/Expensify/Mobile-Expensify/pull/13299
Fixed Issues
$ #51673
$ #52116
PROPOSAL: #52116 (comment)
Tests
Offline tests
N/A
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop