Skip to content

Commit

Permalink
fix(native-stack): set headerTopInstetEnabled to the same value as …
Browse files Browse the repository at this point in the history
…`statusBarTranslucent` to prevent header content jump (#12014)

**Motivation**

*copied & slightly redacted message from discord* 😄 

Currently we set `headerTopInsetEnabled: true` when `safe-area-context`
reports any non-zero top inset. This prop is intended to handle missing
inset when `statusBarTranslucent` is set to `true` (so whole header is
moved up, and headerTitle is partially obfuscated by the status bar,
just like on the first screenshot, so in the place I've sent you it
seems that it is a bit misused?

I'm opening this PR, because I'm handling all header-related issues
right now and: when `statusBarTranslucent: false` &
`headerTopInsetEnabled == topInset !== 0` we get buggy behaviour on
native stack, because `safe-area-context` does report `topInset > 0`
only during the initial render & on every consecutive render `topInset
=== 0`, thus `headerTopInsetEnabled === false` <- this results in
jumping header content (sometimes even whole screen contents do jump),
because during first render we apply the headerTopInset (which basically
means adding topPadding to header with the value of status bar's
height), and on every consecutive render this padding is set to 0. See
the attached recording (best to download and go frame by frame).

What's also intriguing, `safe-area-context` does report non-zero top
inset during initial JS render only because we pass
`initialWindowMetrics` as `initialMetrics` [prop to
SafeAreaProvider](https://github.com/react-navigation/react-navigation/blob/25e834b4f330fb05b4ecdbe81ef7f2f065fd26c8/packages/elements/src/SafeAreaProviderCompat.tsx#L55),
and these (`initialWindowMetrics`) do have non zero top padding (thus
inconsistency with what we get in first render.

I digged into `safe-area-context` implementation & I won't be really
able to fix these initial window metrics there, because they are "true",
and I mean by that they are reporting real window dimensions with
existing status bar / notch / navigation bar offsets, but the insets
that come after first render are from perspective of the
SafeAreaProvider view that resides in native tree, and not from window
anymore, thus it reports 0, 0, 0, 0 for insets <- and this is also
technically correct.

| statusBarTranslucent: true, headerTopInsetEnabled: false | Jumping
header title video |
| ---- | ---- |
| <img
src=https://github.com/react-navigation/react-navigation/assets/50801299/45207a46-9260-4b4e-b614-c86c77e46cf8
width="270" height="555" /> | <video
src="https://github.com/react-navigation/react-navigation/assets/50801299/70c0aa1f-b49a-4500-8d81-88f4ea778e6a"
controls width="270" height="555" /> |

### Proposed solution

For me, the easiest way to go forward would be to set
`headerTopInsetEnabled` to `false` everytime `statusBarTranslucent ==
false`.

`headerTopInsetEnabled` is not exposed as a part of public API, so
that's feasible without introducing breaking changes if we come to
conclusion that suggested fix is ok.

### Ideal solution

`react-native-safe-area-context` frame reported in the very first render
(so the constant we use for `initialMetrics`) should be the same as in
consecutive renders.

Action points: dig into whether we can use some different initial
metrics.

**Test plan**

I've tested on the example prepared for #10827 by @kacperkapusciak:
[link](https://snack.expo.dev/@kacperkapusciak/native-stack-header-height).

As `headerTopInsetEnabled` prop impacts only Android, I've tested only
Android.

**I do see an regression here**, on fifth screen the reported header
height is *negative*, **however this happens also on main** so it is not
the impact of this PR. Other behaviour seems ok & it also fixes my
problem.

Video with regression:


https://github.com/react-navigation/react-navigation/assets/50801299/84f43a56-b6a0-4d0b-baad-39ed72847d6d


The change must pass lint, typescript and tests.
  • Loading branch information
kkafar committed Jun 17, 2024
1 parent 25e834b commit 7791191
Showing 1 changed file with 10 additions and 1 deletion.
11 changes: 10 additions & 1 deletion packages/native-stack/src/views/NativeStackView.native.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,16 @@ const SceneView = ({
[headerHeightCorrectionOffset, rawAnimatedHeaderHeight]
);

const headerTopInsetEnabled = topInset !== 0;
// During the very first render topInset is > 0 when running
// in non edge-to-edge mode on Android, while on every consecutive render
// topInset === 0, causing header content to jump, as we add padding on the first frame,
// just to remove it in next one. To prevent this, when statusBarTranslucent is set,
// we apply additional padding in header only if its true.
// For more details see: https://github.com/react-navigation/react-navigation/pull/12014
const headerTopInsetEnabled =
typeof statusBarTranslucent === 'boolean'
? statusBarTranslucent
: topInset !== 0;

const backTitle = previousDescriptor
? getHeaderTitle(previousDescriptor.options, previousDescriptor.route.name)
Expand Down

0 comments on commit 7791191

Please sign in to comment.