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

SafeAreaView doesn't respect padding property in style #22211

Open
dani-mp opened this issue Nov 8, 2018 · 48 comments
Open

SafeAreaView doesn't respect padding property in style #22211

dani-mp opened this issue Nov 8, 2018 · 48 comments
Labels
Bug Component: SafeAreaView Component: View Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue.

Comments

@dani-mp
Copy link

dani-mp commented Nov 8, 2018

Environment

React Native Environment Info:
    System:
      OS: macOS 10.14
      CPU: x64 Intel(R) Core(TM) i5-4258U CPU @ 2.40GHz
      Memory: 34.00 MB / 8.00 GB
      Shell: 3.2.57 - /bin/bash
    Binaries:
      Node: 9.7.1 - ~/.nvm/versions/node/v9.7.1/bin/node
      Yarn: 1.2.1 - /usr/local/bin/yarn
      npm: 6.4.1 - ~/.nvm/versions/node/v9.7.1/bin/npm
      Watchman: 4.7.0 - /usr/local/bin/watchman
    SDKs:
      iOS SDK:
        Platforms: iOS 12.0, macOS 10.14, tvOS 12.0, watchOS 5.0
      Android SDK:
        Build Tools: 21.1.2, 22.0.1, 23.0.1, 23.0.3, 25.0.0, 25.0.2, 26.0.1, 26.0.2, 27.0.1, 27.0.3, 28.0.2, 28.0.3
        API Levels: 17, 18, 19, 22, 23, 25, 26, 27, 28
    IDEs:
      Android Studio: 3.2 AI-181.5540.7.32.5056338
      Xcode: 10.0/10A254a - /usr/bin/xcodebuild
    npmPackages:
      react: 16.6.0 => 16.6.0
      react-native: ^0.57.3 => 0.57.3
    npmGlobalPackages:
      react-native-cli: 2.0.1

Description

Applying padding to SafeAreaView's style doesn't work.

Reproducible Demo

https://snack.expo.io/@danielmartin/c2FmZW

@bartolkaruza
Copy link

renders nested content and automatically applies paddings

Seems like intended behaviour from the docs. Try adding padding to your view inside the SafeAreaView instead.

@dani-mp
Copy link
Author

dani-mp commented Nov 9, 2018

Thanks for the response, @bartolkaruza. I was wondering if it would be better if the padding applied by the consumer was added to the safe area one, instead of being overwritten. Thinking about it, the way the SafeAreaView component achieves its behaviour is more an implementation detail.

@bartolkaruza
Copy link

If the padding is added to the already in place padding that the SafeAreaView adds automatically, the next opened issue will be from someone asking why the padding value is not exactly the number of pixels specified by the style. Maybe someone would expect to be able to override the padding by using the padding style for some cases, which wouldn't work... Are the complexity and additional discussion worth it when the solution is putting in a containing view on which you can apply all the styles you want?

Perhaps a note in the docs specifying more clearly that the padding style property is ignored would save the next person some time? Maybe a warning message could take it even a step further. Feel free to open a PR for that if you have the time.

@dani-mp
Copy link
Author

dani-mp commented Nov 9, 2018

I wanted to share my thoughts regarding this because I found the current approach not ideal. I discussed this internally before creating the issue and people agreed with me that this was confusing.

If someone uses SafeAreaView I expect them to know that it will define new bounds and that all style properties applied to it would work in base to those new bounds (otherwise they would have used a View in the first place). The fact that internally SafeAreaView uses padding to accomplish this it's just, IMO, an implementation detail that shouldn't concern the consumer.

I thought the additional discussion was worth it and that's why I brought it up.

Adding a containing view is what we're doing now and it feels unnecessary, given that SafeAreaView already takes a View's style property. I also thought about the warning message, there are components out there that filter out forbidden properties and warn about their usage in dev mode, this could rise awareness about this particularity, but it wouldn't solve the underlying issue.

To add another argument, in native iOS you can create a constraint relative to the safeAreaLayoutGuide without having to create a containing view to do it.

I would love to find time to help improving this somehow, I just wanted to know what people think about it first!

@esr360
Copy link

esr360 commented May 1, 2019

Sorry but I'm a bit confused about this issue.

I have applied padding to a SafeAreaView component. In Android it is applied, in iOS it isn't. What is the reason, and what is the solution?

Thanks.

@dani-mp
Copy link
Author

dani-mp commented May 1, 2019

@esr360 Until this issue is solved, you need to add another view inside the SafeAreaView and apply your padding there, because SafeAreaView uses the padding style prop to implement its behaviour, overriding the one you pass to it.

@stale
Copy link

stale bot commented Aug 4, 2019

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Aug 4, 2019
@dani-mp
Copy link
Author

dani-mp commented Aug 5, 2019

:(

@stale stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Aug 5, 2019
@JLWalsh
Copy link

JLWalsh commented Aug 21, 2019

Since this causes inconsistent behaviour between iOS and Android, I don't think that adding a simple message in the doc is enough, as it will falsely indicate that this behaviour is the same on Android & iOS. Whether the user provided paddings are applied or not, I think that the behaviour atleast needs to be consistent across all OSes.

@aldebout
Copy link

Wouldn't a contentContainerStyle prop be appropriate, to be consistent with KeyboardAvoidingView?

@stale
Copy link

stale bot commented Jan 27, 2020

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Jan 27, 2020
@esr360
Copy link

esr360 commented Jan 27, 2020

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

Not been fixed, please don't close issues that are not fixed, I hate this bot. It makes things go away for project maintainers but does not help users.

@stale stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Jan 27, 2020
@ianmartorell
Copy link

Definitely not been fixed.

@huangkaiw3n
Copy link

confusing behavior. could be improved as suggested

@gavin-gyle
Copy link

It took me a while yesterday to figure out why my padding wasn't working. Looked at the docs and saw SafeAreaView inherits View properties so I got even more confused. I would think add the padding to the calculated padding as suggested, or don't allow SafeAreaView to take in a style to remove the confusion.

@stale stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Apr 8, 2021
@AwakenedMind
Copy link

as a beginner to RN I found SafeAreaView styling non-intuitive

@sekizlipenguen
Copy link

+1

@Teddir
Copy link

Teddir commented May 8, 2021

is it true that using SafeAreaView in android with Flatlist as its child will cause onEndReached prop to not fire? I am currently having this problem.

@christopher-caldwell
Copy link

Don't make it stale, bot! Still a problem.

@Aaqib925
Copy link

is it true that using SafeAreaView in android with Flatlist as its child will cause onEndReached prop to not fire? I am currently having this problem.

It doesn't cause any issue. Make sure your flatlist parent component is not scrollview, if not try using onEndReachedThreshold prop to value 0 in iOS or 0.5 in Android. This shall work.

@andrelfnavarro
Copy link

It at least should be similar to KeyboardAvoidingView, with a contentContainerStyle prop. So weird needing to add another view just for padding

facebook-github-bot pushed a commit that referenced this issue Sep 11, 2021
Summary:
This sync includes the following changes:
- **[95d762e40](facebook/react@95d762e40 )**: Remove duplicate test //<Andrew Clark>//
- **[d4d1dc085](facebook/react@d4d1dc085 )**: Reorder VARIANT feature flags ([#22266](facebook/react#22266)) //<Dan Abramov>//
- **[2f156eafb](facebook/react@2f156eafb )**: Adjust consoleManagedByDevToolsDuringStrictMode feature flag ([#22253](facebook/react#22253)) //<Dan Abramov>//
- **[cfd819332](facebook/react@cfd819332 )**: Add useSyncExternalStore to react-debug-tools ([#22240](facebook/react#22240)) //<Andrew Clark>//
- **[8e80592a3](facebook/react@8e80592a3 )**: Remove state queue from useSyncExternalStore ([#22265](facebook/react#22265)) //<Andrew Clark>//
- **[06f98c168](facebook/react@06f98c168 )**: Implement useSyncExternalStore in Fiber ([#22239](facebook/react#22239)) //<Andrew Clark>//
- **[77912d9a0](facebook/react@77912d9a0 )**: Wire up the native API for useSyncExternalStore ([#22237](facebook/react#22237)) //<Andrew Clark>//
- **[031abd24b](facebook/react@031abd24b )**: Add warning and test for useSyncExternalStore when getSnapshot isn't cached ([#22262](facebook/react#22262)) //<salazarm>//
- **[b8884de24](facebook/react@b8884de24 )**: break up import keyword to avoid being accidentally parsed as dynamic import statement in external code ([#21918](facebook/react#21918)) //<Jianhua Zheng>//
- **[6d6bba5bf](facebook/react@6d6bba5bf )**: Fix typo in ReactUpdatePriority-test.js ([#21958](facebook/react#21958)) //<Ikko Ashimine>//
- **[0c0d1ddae](facebook/react@0c0d1ddae )**: feat(eslint-plugin-react-hooks): support ESLint 8.x ([#22248](facebook/react#22248)) //<Michaël De Boey>//
- **[1314299c7](facebook/react@1314299c7 )**: Initial shim of useSyncExternalStore ([#22211](facebook/react#22211)) //<Andrew Clark>//
- **[fc40f02ad](facebook/react@fc40f02ad )**: Add consoleManagedByDevToolsDuringStrictMode feature flag in React Reconciler ([#22196](facebook/react#22196)) //<Luna Ruan>//
- **[46a0f050a](facebook/react@46a0f050a )**: Set up use-sync-external-store package ([#22202](facebook/react#22202)) //<Andrew Clark>//
- **[8723e772b](facebook/react@8723e772b )**: Fix a string interpolation typo in ReactHooks test ([#22174](facebook/react#22174)) //<Matt Hargett>//
- **[60a30cf32](facebook/react@60a30cf32 )**: Console Logging for StrictMode Double Rendering ([#22030](facebook/react#22030)) //<Luna Ruan>//
- **[76bbad3e3](facebook/react@76bbad3e3 )**: Add maxYieldMs feature flag in Scheduler ([#22165](facebook/react#22165)) //<Ricky>//
- **[b0b53ae2c](facebook/react@b0b53ae2c )**: Add feature flags for scheduler experiments ([#22105](facebook/react#22105)) //<Ricky>//

Changelog:
[General][Changed] - React Native sync for revisions bd5bf55...95d762e

jest_e2e[run_all_tests]

Reviewed By: mdvacca

Differential Revision: D30809906

fbshipit-source-id: 131cfdf91e15f67fa59a5d925467e538ee89fe10
@palcisto
Copy link

So what is the status of this issue amongst the authoring team? It appears to have gained almost no traction or attention from the team other than adding a note to the documentation since this issue was first created 3.5 years ago at this point.

@lautaroml
Copy link

Any solution?

@a-eid
Copy link

a-eid commented Mar 20, 2022

use https:/th3rdwave/react-native-safe-area-context, it has a hook api and you can change the mode from padding to margin.

@Pat-Gekoski
Copy link

@esr360 Until this issue is solved, you need to add another view inside the SafeAreaView and apply your padding there, because SafeAreaView uses the padding style prop to implement its behaviour, overriding the one you pass to it.

this makes sense. @esr360 .....does it work with Android phones that have a notch too?

@abdulbari149
Copy link

Maybe this issue should be closed. Because the padding problem is resolved by react-native-safe-area-context. Just a suggestion

@magom001
Copy link

magom001 commented Nov 21, 2022

I personally would like to have an option to override the padding. E.g.: I want the default vertical padding but I want to disable the horizontal padding. The SafeAreaView from https:/th3rdwave/react-native-safe-area-context has edges property that allows to achieve exactly that.

@phthhieu
Copy link

And if padding doesn't work, the style prop of SafeAreaView shouldn't accept the padding attribute, it would be more obvious and not take so much time on the investigation or go back and forth to find out this thread or the related documentation.

declare class SafeAreaViewComponent extends React.Component<ViewProps> {}

Should be sth like:

declare class SafeAreaViewComponent extends React.Component<Omit<ViewProps, 'style'> {
   style: AStyleWithoutPaddingAttributes
}

@ankitpunchh
Copy link

Dont know about the perfect solution but once i have added another view on top of SafeAreaView and gave the style to that View. Problem gets solved but yeah its weird as it adds one extra node.

@07kushalsaini
Copy link

I have use margin instead on padding in SafeAreaView.
and it;s working for me.

@07kushalsaini
Copy link

Still doesn't work. easily fixable with view inside. Slight confusion though. a note in the docs could help. Screenshot 2020-06-11 at 8 27 48 AM

use margin it will work in SafeAreaView

@teja2495
Copy link

@07kushalsaini margin will affect the background color unless you wrap it in another view which is not ideal.

@github-actions github-actions bot added the Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. label Jul 24, 2023
@github-actions

This comment was marked as off-topic.

@NorseGaud
Copy link

Just stumbled across this issue myself what a strange waste of my time trying to figure out what I was doing wrong, only to be surprised.

@brunaporato
Copy link

sad to bump into this unfixed problem :(

@Mahmoudz11
Copy link

6 years until now

@andshonia
Copy link

My approach maybe will help you:

  • Create simple custom container
 <View style={{  paddingTop: Platform.OS === "android"
      ? Constants?.statusBarHeight + 10
      : Constants?.statusBarHeight,}}>
 ...you code 
 </View>

@DDunc
Copy link

DDunc commented May 16, 2024

Based on how popular silently ignoring and overriding a style attribute has been with the react-native core team & community for the last six years, I think it would be worth rolling out this feature to more components.

Many components could benefit from the functionality and expressiveness SafeAreaView gives developers who would like to be able to safely store integer values inside of style attributes without the unwanted side-effect of applying layout changes on some platforms.

@tilltu-brian
Copy link

I'm confused about what the expected behavior is for padding in a SafeAreaView. I landed here because I have 5 pages, each set up exactly the same with a SafeAreaView wrapping some Text and a ScrollView. Each page has the exact same layout and exact same styles (imported from a shared file). Four of the pages respect the padding, one does not, and I cannot figure out why.

I was able to get padding on that page by adding an extra View and putting the padding on it, but everything else gets messed up because the shared styles weren't designed for that extra layer of nesting. After reading the comments in this thread I'm thinking I'll need to redo all of the pages to have that extra View within the SafeAreaView, which seems silly.

@shivang98
Copy link

This works for me!

Add a View inside SafeAreaView and provide styling separately, as shown below.

import { Platform, SafeAreaView, StyleSheet, View } from "react-native";
import { NavigationContainer } from "@react-navigation/native";
import HomeNavigator from "./App/Navigations/HomeNavigator";

export default function App() {
  return (
    <SafeAreaView style={styles.safeContainer}>
      <View style={styles.container}>
        <NavigationContainer>
          <HomeNavigator />
        </NavigationContainer>
      </View>
    </SafeAreaView>
  );
}

const styles = StyleSheet.create({
  safeContainer: {
    flex: 1,
    backgroundColor: "#fff",
  },
  container: {
    flex: 1,
    backgroundColor: "#fff",
    paddingTop: Platform.OS == "android" ? 20 : 0,
    paddingRight: 20,
    paddingLeft: 20,
  },
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Component: SafeAreaView Component: View Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue.
Projects
None yet
Development

No branches or pull requests