-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
[iOS] Add additionalSafeAreaInsets prop for SafeAreaView #23784
[iOS] Add additionalSafeAreaInsets prop for SafeAreaView #23784
Conversation
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.
Code analysis results:
flow
found some issues.
e8dfd1c
to
92987ef
Compare
[iOS] Add additionalSafeAreaInsets prop for SafeAreaView [iOS] Add additionalSafeAreaInsets prop for SafeAreaView fix flow check Add EdgeInsetsProp import Rename totalSafeAreaInsetsWithInsets
472bda2
to
21a0e87
Compare
@zhongwuzw Would it be possible to just support padding instead of adding a new prop that basically does the same? I think we override the padding in the shadow node so we could just add safe area and the current padding. |
@janicduplessis Emm, my opinions based on two reasons:
So I did this |
|
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.
See my comment.
@shergin @zhongwuzw What about we add the padding with the safe area insets? There was a couple times I wanted to add padding to a SafeAreaView and the behaviour I was expecting was that. A common case would be a footer bar: <SafeAreaView style={{padding: 16}}>...</SafeAreaView> In this case the padding bottom would be safeAreaInsets.bottom + 16 |
@janicduplessis Before I added a new prop |
Summary
Currently, if we use
SafeAreaView
, we lose thepadding
prop, https://github.com/facebook/react-native/blob/master/React/Views/SafeAreaView/RCTSafeAreaShadowView.m#L36, so the insets can only determined by system, but, actually, user may wants some additional insets based on system safe area, like #22211, and we can also see Apple document, https://developer.apple.com/documentation/uikit/uiviewcontroller/2902284-additionalsafeareainsets?language=objc#, it also hasadditionalSafeAreaInsets
forViewController
, so I think we can addadditionalSafeAreaInsets
prop for user to do custom insets.Changelog
[iOS] [Added] - Add additionalSafeAreaInsets prop for SafeAreaView
Test Plan
After we added
additionalSafeAreaInsets
prop, the final safe area equals to system safe area plus user custom safe area: