Skip to content
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

fix: Allow Reanimated Screen to check large header #1915

Merged
merged 9 commits into from
Oct 11, 2023

Conversation

tboba
Copy link
Member

@tboba tboba commented Oct 7, 2023

Description

It looks that on Reanimated Screen we didn't check if on iOS there's large header enabled, which led to the wrong initial values, when user was using at least ReanimatedScreenProvider.

This feature fixes that by adding an ability to get screen descriptor by Reanimated Screen, which contains an information if user has enabled headerLargeTitle prop.

Changes

  • Changed the useReanimatedTransitionProgress leftover inside the useReanimatedHeaderHeight file
  • Created new hook useScreenInfo that provides an information about the route view which is being used by ReanimatedNativeStackScreen (this hook is not meant to be public [at least for now] - that's why I didn't exported it)
  • Moved the implementation of getStatusBarHeight to the separate file

Screenshots / GIFs

Before

image

After

image

Test code and steps to reproduce

Add to Test42 ReanimatedScreenProvider and enable headerLargeTitle prop for the Home screen.

Checklist

  • Included code example that can be used to test this change
  • Updated TS types
  • Ensured that CI passes

@tboba tboba self-assigned this Oct 7, 2023
@tboba tboba requested review from kkafar and WoLewicki October 7, 2023 13:39
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey,
wouldn't it be simpler just to pass additional contextual information as prop to screen?

Leaving few remarks below

src/native-stack/utils/ScreenInfoContext.tsx Outdated Show resolved Hide resolved
src/native-stack/utils/useScreenInfo.tsx Outdated Show resolved Hide resolved
src/native-stack/views/NativeStackView.tsx Outdated Show resolved Hide resolved
src/reanimated/ReanimatedNativeStackScreen.tsx Outdated Show resolved Hide resolved
src/reanimated/useReanimatedHeaderHeight.tsx Show resolved Hide resolved
@tboba tboba requested a review from kkafar October 11, 2023 10:09
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides one note below it looks good. GJ 🎉

However I'm still doubtful that this is needed. Why only ReanimatedNativeStackScreen requires this additional information? What happens to e.g. InnerScreen?

src/native-stack/views/NativeStackView.tsx Outdated Show resolved Hide resolved
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎉

@tboba tboba merged commit 20fccf1 into main Oct 11, 2023
3 checks passed
@tboba tboba deleted the @tboba/fix-status-bar-height branch October 11, 2023 17:27
renovate bot referenced this pull request in valora-inc/wallet Nov 2, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[react-native-screens](https://togithub.com/software-mansion/react-native-screens)
| [`^3.25.0` ->
`^3.26.0`](https://renovatebot.com/diffs/npm/react-native-screens/3.25.0/3.26.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/react-native-screens/3.26.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/react-native-screens/3.26.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/react-native-screens/3.25.0/3.26.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/react-native-screens/3.25.0/3.26.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>software-mansion/react-native-screens
(react-native-screens)</summary>

###
[`v3.26.0`](https://togithub.com/software-mansion/react-native-screens/releases/tag/3.26.0)

[Compare
Source](https://togithub.com/software-mansion/react-native-screens/compare/3.25.0...3.26.0)

Minor release adding new useAnimatedHeaderHeight and
useReanimatedHeaderHeight hooks, providing fixes for search bar and
introducing internal changes. *Please note that new hooks introduced in
this release are fully functional on Paper, on Fabric there are few edge
cases we are still working on*.

#### What's Changed

#### 🐛 Bug fixes

- Change implementation of `headerConfig` prop on Android by
[@&#8203;tboba](https://togithub.com/tboba) in
[https://github.com/software-mansion/react-native-screens/pull/1883](https://togithub.com/software-mansion/react-native-screens/pull/1883)
- Change elements visibility on search bar open by
[@&#8203;tboba](https://togithub.com/tboba) in
[https://github.com/software-mansion/react-native-screens/pull/1903](https://togithub.com/software-mansion/react-native-screens/pull/1903)
- Fix positioning of large header and search bar by
[@&#8203;tboba](https://togithub.com/tboba) in
[https://github.com/software-mansion/react-native-screens/pull/1895](https://togithub.com/software-mansion/react-native-screens/pull/1895)
- Change implementation of calculating status bar, refactor methods used
on header height change by [@&#8203;tboba](https://togithub.com/tboba)
in
[https://github.com/software-mansion/react-native-screens/pull/1917](https://togithub.com/software-mansion/react-native-screens/pull/1917)
- Fix calculating header height when changing status/action bar
visibility by [@&#8203;tboba](https://togithub.com/tboba) in
[https://github.com/software-mansion/react-native-screens/pull/1922](https://togithub.com/software-mansion/react-native-screens/pull/1922)
- Allow Reanimated Screen to check large header by
[@&#8203;tboba](https://togithub.com/tboba) in
[https://github.com/software-mansion/react-native-screens/pull/1915](https://togithub.com/software-mansion/react-native-screens/pull/1915)
- Fix issue when emptying nav stack on Windows by
[@&#8203;chrisglein](https://togithub.com/chrisglein) in
[https://github.com/software-mansion/react-native-screens/pull/1890](https://togithub.com/software-mansion/react-native-screens/pull/1890)
- Update podspec to use install_modules_dependencies by
[@&#8203;cipolleschi](https://togithub.com/cipolleschi) in
[https://github.com/software-mansion/react-native-screens/pull/1920](https://togithub.com/software-mansion/react-native-screens/pull/1920)
- Remove MaxPerm args from JVM invocation by
[@&#8203;kkafar](https://togithub.com/kkafar) in
[https://github.com/software-mansion/react-native-screens/pull/1888](https://togithub.com/software-mansion/react-native-screens/pull/1888)

#### 👍 Improvements

- Calculate values of useHeaderHeight natively by
[@&#8203;tboba](https://togithub.com/tboba) in
[https://github.com/software-mansion/react-native-screens/pull/1802](https://togithub.com/software-mansion/react-native-screens/pull/1802)
- Allow for different fragment types inside ScreenContainer by
[@&#8203;kkafar](https://togithub.com/kkafar) in
[https://github.com/software-mansion/react-native-screens/pull/1887](https://togithub.com/software-mansion/react-native-screens/pull/1887)
- Add focused states on page transitions by
[@&#8203;tboba](https://togithub.com/tboba) in
[https://github.com/software-mansion/react-native-screens/pull/1894](https://togithub.com/software-mansion/react-native-screens/pull/1894)

#### 🔢 Miscellaneous

- **Create FUNDING.yml by
[@&#8203;aleqsio](https://togithub.com/aleqsio) in
[https://github.com/software-mansion/react-native-screens/pull/1886](https://togithub.com/software-mansion/react-native-screens/pull/1886)**
- Migrate from deprecated `RCTEventEmitter` by
[@&#8203;kkafar](https://togithub.com/kkafar) in
[https://github.com/software-mansion/react-native-screens/pull/1867](https://togithub.com/software-mansion/react-native-screens/pull/1867)
- Use `require` syntax for resolution of all native components by
[@&#8203;kkafar](https://togithub.com/kkafar) in
[https://github.com/software-mansion/react-native-screens/pull/1909](https://togithub.com/software-mansion/react-native-screens/pull/1909)
- Run Android e2e with JDK 17 by
[@&#8203;kkafar](https://togithub.com/kkafar) in
[https://github.com/software-mansion/react-native-screens/pull/1892](https://togithub.com/software-mansion/react-native-screens/pull/1892)
- Put timelimit on execution of each workflow by
[@&#8203;kkafar](https://togithub.com/kkafar) in
[https://github.com/software-mansion/react-native-screens/pull/1893](https://togithub.com/software-mansion/react-native-screens/pull/1893)
- Trigger e2e tests on JS-only changes by
[@&#8203;kkafar](https://togithub.com/kkafar) in
[https://github.com/software-mansion/react-native-screens/pull/1910](https://togithub.com/software-mansion/react-native-screens/pull/1910)
- Update deprecated expo install instructions to `npx expo install` by
[@&#8203;GabrieldosSantosOliveira](https://togithub.com/GabrieldosSantosOliveira)
in
[https://github.com/software-mansion/react-native-screens/pull/1899](https://togithub.com/software-mansion/react-native-screens/pull/1899)
- Bump activesupport from 6.1.7.3 to 7.0.7.2 in /TestsExample by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/software-mansion/react-native-screens/pull/1877](https://togithub.com/software-mansion/react-native-screens/pull/1877)
- Update deps & RN in example apps after release by
[@&#8203;kkafar](https://togithub.com/kkafar) in
[https://github.com/software-mansion/react-native-screens/pull/1878](https://togithub.com/software-mansion/react-native-screens/pull/1878)
- Migrate `Example` app & e2e tests to RN 0.72.4 by
[@&#8203;kkafar](https://togithub.com/kkafar) in
[https://github.com/software-mansion/react-native-screens/pull/1880](https://togithub.com/software-mansion/react-native-screens/pull/1880)
- Bump library deps to recent versions (including RN) by
[@&#8203;kkafar](https://togithub.com/kkafar) in
[https://github.com/software-mansion/react-native-screens/pull/1881](https://togithub.com/software-mansion/react-native-screens/pull/1881)
- Bump library native Android deps & config by
[@&#8203;kkafar](https://togithub.com/kkafar) in
[https://github.com/software-mansion/react-native-screens/pull/1891](https://togithub.com/software-mansion/react-native-screens/pull/1891)

#### New Contributors

- [@&#8203;chrisglein](https://togithub.com/chrisglein) made their first
contribution in
[https://github.com/software-mansion/react-native-screens/pull/1890](https://togithub.com/software-mansion/react-native-screens/pull/1890)
-
[@&#8203;GabrieldosSantosOliveira](https://togithub.com/GabrieldosSantosOliveira)
made their first contribution in
[https://github.com/software-mansion/react-native-screens/pull/1899](https://togithub.com/software-mansion/react-native-screens/pull/1899)
- [@&#8203;cipolleschi](https://togithub.com/cipolleschi) made their
first contribution in
[https://github.com/software-mansion/react-native-screens/pull/1920](https://togithub.com/software-mansion/react-native-screens/pull/1920)

**Full Changelog**:
software-mansion/react-native-screens@3.25.0...3.26.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 5pm,every weekend" in timezone
America/Los_Angeles, Automerge - "after 5pm,every weekend" in timezone
America/Los_Angeles.

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/valora-inc/wallet).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xOS4yIiwidXBkYXRlZEluVmVyIjoiMzcuMTkuMiIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: valora-bot <[email protected]>
Co-authored-by: Silas Boyd-Wickizer <[email protected]>
Co-authored-by: Silas Boyd-Wickizer <[email protected]>
ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this pull request Oct 9, 2024
…1915)

## Description

It looks that on Reanimated Screen we didn't check if on iOS there's
large header enabled, which led to the wrong initial values, when user
was using *at least* ReanimatedScreenProvider.

This feature fixes that by adding an ability to get screen descriptor by
Reanimated Screen, which contains an information if user has enabled
`headerLargeTitle` prop.

## Changes

- Changed the `useReanimatedTransitionProgress` leftover inside the
`useReanimatedHeaderHeight` file
- Created new hook `useScreenInfo` that provides an information about
the route view which is being used by ReanimatedNativeStackScreen (this
hook is not meant to be public [at least for now] - that's why I didn't
exported it)
- Moved the implementation of getStatusBarHeight to the separate file

## Screenshots / GIFs

### Before

<img width="995" alt="image"
src="https://github.com/software-mansion/react-native-screens/assets/23281839/244782fb-8ef0-433e-b84f-7a7159f800d9">

### After

<img width="996" alt="image"
src="https://github.com/software-mansion/react-native-screens/assets/23281839/2114196c-1528-409a-8c04-0e6d1d9cc7c7">

## Test code and steps to reproduce

Add to `Test42` ReanimatedScreenProvider and enable `headerLargeTitle`
prop for the `Home` screen.

## Checklist

- [X] Included code example that can be used to test this change
- [X] Updated TS types
- [ ] Ensured that CI passes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants