-
-
Notifications
You must be signed in to change notification settings - Fork 583
fix(Android, SAV+Tabs+Stack v4): fix CustomToolbar's insets handling to use insets received from ancestor views #3240
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
Open
kligarski
wants to merge
1
commit into
main
Choose a base branch
from
@kligarski/safe-area-view-tabs-with-stack-android
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+32
−26
Conversation
This file contains hidden or 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
14 tasks
kmichalikk
approved these changes
Sep 24, 2025
kligarski
added a commit
that referenced
this pull request
Sep 26, 2025
## Description Adds implementation for handling safe area on Android for bottom tabs. Implementation has been adapted from [`react-native-safe-area-context`](https://github.com/AppAndFlow/react-native-safe-area-context). TODO: - [x] change native layout so that TabScreen renders under the tab bar (to allow using transparent tab bar) - [x] check implementation for older Android APIs (I checked APIs: 25, 28, 29, 30, 36 and 24 on Paper) - [x] add simple API (for all edges) to control which types of insets are used by `SafeAreaView` component (system and/or interface bars) - [ ] [Separate PR] add per-edge API to control which types of insets are used by `SafeAreaView` component (system and/or interface bars) (software-mansion/react-native-screens-labs#434) - [ ] [[Separate PR](#3240)] fix interaction with Stack v4 (`CustomToolbar`) (software-mansion/react-native-screens-labs#435) - [ ] [Separate PR] There were no problems with using margins for now so I left it as it was. We can change this later to padding or add a prop to switch between padding and margins on both Android and iOS. (software-mansion/react-native-screens-labs#436) https://github.com/user-attachments/assets/c59af116-a653-40e3-9345-fe8d3ba170ee ### Transparent tab bar | `top: false, bottom: false` | `top: true, bottom: false` | `top: true, bottom: true` | | --- | --- | --- | | <img width="1280" height="2856" alt="Screenshot_20250916_091258" src="https://github.com/user-attachments/assets/03afdf11-b4d8-4ee9-b32f-d893074208b3" /> | <img width="1280" height="2856" alt="Screenshot_20250916_091303" src="https://github.com/user-attachments/assets/7d70b007-63ae-4d62-884b-59d04f96158e" /> | <img width="1280" height="2856" alt="Screenshot_20250916_091311" src="https://github.com/user-attachments/assets/0ad3d293-0f1b-4a58-8139-dfd797bfc98b" /> | ### Changes to TabsHost's layout #### SafeAreaView After internal discussion about approach to `SafeAreaView`, we had following conclusions: - as edge-to-edge becomes desirable (and is the default for apps targeting Android SDK 35 or above), and to simplify layout handling, we want the `Screen`s of our navigation containers (e.g. `StackScreen`, `BottomTabsScreen`) to have **full dimensions of their parents, even if it means that they will be laid out behind navigation bars** (header in Stack, tab bar), - `SafeAreaView` will provide unified way to handle the safe area, - on Android, we want to control which insets we want to handle: - **system insets** (received from `onApplyWindowInsets`), e.g. `systemBars`, `displayCutout` - **interface insets** - custom insets from navigation bars, e.g. `bottomNavigationView` #### Before this PR Prior to this PR, we were using `LinearLayout` for `TabsHost`: ```kotlin class TabsHost( val reactContext: ThemedReactContext, ) : LinearLayout(reactContext), TabScreenDelegate { // ... private val bottomNavigationView: BottomNavigationView = BottomNavigationView(wrappedContext).apply { layoutParams = LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT) } private val contentView: FrameLayout = FrameLayout(reactContext).apply { layoutParams = LinearLayout .LayoutParams( LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT, ).apply { weight = 1f } id = ViewIdGenerator.generateViewId() } // ... } ``` <img width="946" height="817" alt="Screenshot 2025-09-24 at 17 23 20" src="https://github.com/user-attachments/assets/784a769a-d98a-4d4e-a9aa-0413a8d85fed" /> This approach had following problems: - `contentView` did not have dimensions of its parent, which is not what we wanted, - Yoga wasn't aware of `contentView`'s height - all content inside `TabScreen` was laid out as if the screen had full height of its parent -> this meant that `contentView`'s dimensions and actual content dimensions were not in sync, - it did not support using translucent tab bar - screen's content was cut off outside of `contentView`'s bounds. <img width="1280" height="2856" alt="3215_before_transparent" src="https://github.com/user-attachments/assets/9af6b066-7f2f-4927-ac28-00277e4077ce" /> #### Approach in this PR In this PR, we change `TabsHost`'s layout to `FrameLayout` which allows multiple views placed on top of each other - this is what we want to achieve (tab bar floating over content, attached to the bottom). To attach `bottomNavigationView` to *the bottom*, we use `Gravity.BOTTOM`. ```kotlin class TabsHost( val reactContext: ThemedReactContext, ) : FrameLayout(reactContext), TabScreenDelegate, SafeAreaProvider, View.OnLayoutChangeListener { // ... private val bottomNavigationView: BottomNavigationView = BottomNavigationView(wrappedContext).apply { layoutParams = LayoutParams( LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT, Gravity.BOTTOM, ) } private val contentView: FrameLayout = FrameLayout(reactContext).apply { layoutParams = LayoutParams( LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT, ) id = ViewIdGenerator.generateViewId() } // ... } ``` <img width="447" height="826" alt="Screenshot 2025-09-24 at 17 23 29" src="https://github.com/user-attachments/assets/e3b67014-a193-4bb8-ba19-5b7c8f7816f8" /> Now, we can: 1. use opaque or translucent `bottomNavigationView`, 2. use `SafeAreaView` to control how much space can the actual content take (do we allow it to render under `bottomNavigationView`). <img width="1280" height="2856" alt="3215_after_transparent" src="https://github.com/user-attachments/assets/2f2944f9-1984-4c01-b335-53051c58cbd3" /> ### Support for older Android versions On Android versions prior to R, insets dispatch is broken (children of ViewGroup receive insets from previous child; they should all receive the same insets). That's why we need to override `dispatchApplyWindowInsets` implementation in `TabsHost`. In `ViewGroup`'s implementation of this method, `View`'s implementation is used (via `super`) - we can't access this directly. Unfortunately, `View`'s implementation sets some private flags which are used by default `onApplyWindowInsets` implementation. If we try to use `onApplyWindowInsets` on API 28 without setting the private flag, application goes into infinite loop (`fitSystemWindows` calls `dispatchApplyWindowInsets` -> we might want to investigate this in more detail). As we don't use insets in `TabsHost`, I decided not to call `onApplyWindowInsets` in `TabsHost` at all. I haven't found any problems with it yet. ## Changes - add implementation for `SafeAreaView` and related classes for both architectures - add `SafeAreaProvider` interface and implement it for `TabsHost` - change `TabsHost` to use `FrameLayout`: - make `TabsScreen` layout behind tab bar (take full available height to match JS screen) - override `dispatchApplyWindowInsets` in `TabsHost` in order to fix insets for older Android versions - add `insetType` prop to control what kind of insets should SafeAreaView respect (all, only system, only interface) ## Test code and steps to reproduce Run `TestBottomTabs` with uncommented SafeAreaView. You can change which edges are enabled and add `insetType` prop. ## Checklist - [x] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Updated documentation: <!-- For adding new props to native-stack --> - [ ] https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx - [x] Ensured that CI passes
Base automatically changed from
@kligarski/safe-area-view-android-poc
to
main
September 26, 2025 10:01
022ec00
to
0a0ef2e
Compare
kkafar
reviewed
Sep 26, 2025
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.
I like the PR overall & appreciate the description.
I'll test the runtime soon.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adapts
CustomToolbar
from Stack v4 to use insets received from ancestor views viaonApplyWindowInsets
.Screen.Recording.2025-09-23.at.10.12.13.mov
Closes https://github.com/software-mansion/react-native-screens-labs/issues/435.
Reasoning
On API >= 30, changing
rootWindowInsets
tounhandledInsets
(insets received fromonApplyWindowInsets
) is enough. Unfortunately, on API < 30, there is no insets section for display cutout, only cutout details are passed (.getDisplayCutout()
) with possibility to setmDisplayCutoutConsumed
flag. SettingsetInsets(WindowInsetsCompat.Type.displayCutout(), Insets.NONE)
does nothing on API < 30. When we read those insets, we get insets returned from.getDisplayCutout()
's safe area. IfWindowInsetsCompat
has been created withWindowInsetsCompat toWindowInsetsCompat(@NonNull WindowInsets insets, @Nullable View view)
, cutout insets from root view are used instead of insets passed from ancestor views (source):We want to:
displayCutout
insets:.consumeDisplayCutout()
API to consume insets on SDK < 30displayCutout
insets from ancestor views, also on SDK < 30:.toWindowInsetsCompat()
inutils/InsetsKt.kt
I also removed insets
ignoringVisibility
, as we want to handle only visible bars.Testing
(**) is described in more detail in the section below
Issue mentioned in CustomToolbar
I did not manage to reproduce the issue mentioned in the comment above, even on older API versions.
Current problems
Nested header redundantly accounts for insets
(visible in the video below)
This will be fixed only if we decide to add support to
SafeAreaView
for Stack v4 on Android. This will require some additional logic (overridingdispatchApplyWindowInsets
) to pass insets consumed byCustomToolbar
to Screen's child.Ticket for this: https://github.com/software-mansion/react-native-screens-labs/issues/464.
Oversized margin top for title in nested header
Screen_recording_20250923_112144.mp4
This is not a regression, it also happens on current main. It will require more investigation - ticket for this: https://github.com/software-mansion/react-native-screens-labs/issues/463.
Bug when dynamically changing safe area view edges
This is the specific case when I noticed this happening:
Screen.Recording.2025-09-23.at.10.15.45.mov
I haven't debugged this yet. Ticket for this: https://github.com/software-mansion/react-native-screens-labs/issues/462.
Insets behave odd when changing orientation between landscapeLeft/Right to landscapeRight/left
Screen.Recording.2025-09-22.at.14.15.40.mov
This is not a regression, ticket for this: https://github.com/software-mansion/react-native-screens-labs/issues/461.
Changes
onApplyWindowInsets
insetad of root view insets inCustomToolbar
.toWindowInsetsCompat
that does not use root view to determine insetsTest code and steps to reproduce
Run
TestBottomTabs
withSafeAreaView
inTab4
.Checklist