Skip to content

Update statusBarHeight calculation #985

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

Merged
merged 1 commit into from
Oct 19, 2020
Merged

Conversation

ethanshar
Copy link
Collaborator

Description

I Simplified the logic for setting statusBarHeight constants.
I noticed that the previous logic created flakiness with the layout of Modal.TopBar component.
My guess is that in earlier versions of react-native it was more complicated to extract this value, but now
It seems like NativeModules.StatusBarManager initially return the correct value for all platforms (checked iPhoneX, 8, iPad and Android)

Anyway, kept the old logic in a comment in case I missed something and in case we'll get a weird issue in the near future.

Changelog

Fix statusBarHeight constant value and how it affect Modal.TopBar layout.

// override guesstimate height with the actual height from StatusBarManager
StatusBarManager.getHeight((data: any) => (statusBarHeight = data.height));
}
statusBarHeight = StatusBarManager.HEIGHT || 0; // So there will be a value for any case
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not apply the old logic instead of setting to 0 in case 'StatusBarManager.HEIGHT' returns null/undefined?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Technically it works. We don't even need the 0 for that matter.
Hmm The reason I didn't kept the old logic is cause I would prefer to remove it eventually if everything goes well.

@Inbal-Tish Inbal-Tish merged commit cff347a into master Oct 19, 2020
@ethanshar ethanshar deleted the fix/statusBarHeightConstant branch March 8, 2021 09:02
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