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: 2234 - Remove SafeAreaView in favor of useSafeAreaInsets hook #2238

Merged
merged 3 commits into from Oct 4, 2022

Conversation

carlinisaacson
Copy link
Member

@carlinisaacson carlinisaacson commented Sep 30, 2022

Please verify the following:

  • yarn test jest tests pass with new tests, if relevant
  • README.md has been updated with your changes, if relevant

Describe your PR

There is an issue with the SafeAreaView from react-native-safe-area-context that causes the top inset to not be set on first render. So you see the content, then it re-renders and jumps down to it's final resting place. The fix is to not use SafeAreaView and use the useSafeAreaInsets hook instead.

This PR changes the Screen component to use the hook and apply the insets as screen padding if they are passed in, which fixes #2234.

Old behavior

old-behavior.MP4

New (fixed) behavior

new-behavior.MP4

Multiple edges passed in (top and bottom)

Ugly, but it shows it still works

top-and-bottom

@carlinisaacson carlinisaacson marked this pull request as draft September 30, 2022 23:43
@carlinisaacson carlinisaacson marked this pull request as ready for review September 30, 2022 23:50
@carlinisaacson carlinisaacson changed the title Remove SafeAreaView in favor of useSafeAreaInsets hook fix: 2234 - Remove SafeAreaView in favor of useSafeAreaInsets hook Oct 1, 2022
Copy link
Contributor

@frankcalise frankcalise left a comment

Choose a reason for hiding this comment

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

Tested it on my device and definitely fixed the jank for me! Nicely done.

LGTM

Copy link
Contributor

@mazenchami mazenchami left a comment

Choose a reason for hiding this comment

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

This looks great 🚀

boilerplate/app/components/Screen.tsx Outdated Show resolved Hide resolved
Co-authored-by: Jamon Holmgren <jamonholmgren@gmail.com>
@carlinisaacson carlinisaacson merged commit 813c34e into master Oct 4, 2022
@carlinisaacson carlinisaacson deleted the fix/2234-safe-area-insets branch October 4, 2022 17:01
infinitered-circleci pushed a commit that referenced this pull request Oct 4, 2022
## [8.2.2](v8.2.1...v8.2.2) (2022-10-04)

### Bug Fixes

* Remove SafeAreaView in favor of useSafeAreaInsets hook ([#2238](#2238)) ([813c34e](813c34e))
@infinitered-circleci
Copy link

🎉 This PR is included in version 8.2.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Maverick] Demo screens glitch on first render
5 participants