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

pixel size (px) value of some style properties (padding, margin) may caused the app crash. #20206

Closed
zhengxiaoyao0716 opened this issue Jul 14, 2018 · 6 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@zhengxiaoyao0716
Copy link

zhengxiaoyao0716 commented Jul 14, 2018

Environment

Run react-native info in your terminal and paste its contents here.

Environment:
  OS: Windows 10
  Node: 8.11.3
  Yarn: 1.7.0
  npm: 5.6.0
  Watchman: Not Found
  Xcode: N/A
  Android Studio: Not Found

Packages: (wanted => installed)
  react: 16.3.1 => 16.3.1
  react-native: ~0.55.2 => 0.55.4

I tried to use the latest version (0.56) but as this issue reported, it even failed to installed.

Description

Just create an new app, and only try to add padding: '20px' into the style of view, then it crashed and could not open again.
Nothing error found, both the app and the Metro Bundler Console. When I connect the android log (react-native log-android), I just found that:

CatalystInstanceImpl.destroy() start

Reproducible Demo

import React from 'react';
import { StyleSheet, Text, View } from 'react-native';

export default class App extends React.Component {
  render() {
    return (
      <View style={styles.container}>
        <Text>Open up App.js to start working on your app!</Text>
        <Text>Changes you make will automatically reload.</Text>
        <Text /* style={{margin: '20px'}} */ >Shake your phone to open the developer menu.</Text>
      </View>
    );
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    backgroundColor: '#fff',
    alignItems: 'center',
    justifyContent: 'center',
    // padding: '20px', // cancel the comment and the app will be broken.
  },
});
@zhengxiaoyao0716
Copy link
Author

zhengxiaoyao0716 commented Jul 14, 2018

But if I use the px value with some others style properties such as height, width, the App would throw an errror like Unknown value: 20px instead of crashed, I think that is better if the px value is not permit.

@jamsch
Copy link
Contributor

jamsch commented Jul 15, 2018

All dimensions in React Native are unitless, and represent density-independent pixels. That means instead of passing a string value to style properties that have measurements, you'll need to pass in a number.

Looking back at your styles object you'll need to change padding: '20px' to padding: 20.

const styles = StyleSheet.create({
  container: {
    flex: 1,
    backgroundColor: '#fff',
    alignItems: 'center',
    justifyContent: 'center',
    padding: 20,
  },
});

@zhengxiaoyao0716
Copy link
Author

@jamsch I know that, but I think it shouldn't cause the app broken without any error message.

@kelset
Copy link
Contributor

kelset commented Jul 23, 2018

I know that, but I think it shouldn't cause the app broken without any error message.

Feel free to open a PR for that, in the meantime I'm closing this since it works as expected 😶

@kelset kelset closed this as completed Jul 23, 2018
@ggtmtmgg
Copy link
Contributor

I confirmed whether it reproduces on iOS or not.
On iOS, the exception is correctly handled.

@regalstreak
Copy link
Contributor

import { StyleSheet, PixelRatio } from "react-native";

...

  container: {
    flex: 1,
    backgroundColor: '#fff',
    alignItems: 'center',
    justifyContent: 'center',
    padding: 20 / PixelRatio.get(),
  },

This will convert the absolute 20dp padding to 20px fixed. Useful for developers following a strict design spec or something like that.

More info on PixelRatio

@zhengxiaoyao0716 zhengxiaoyao0716 changed the title pixel size (px) value of some style properties (padding, margin) may broke the app. pixel size (px) value of some style properties (padding, margin) may caused the app crash. Jul 3, 2019
@facebook facebook locked as resolved and limited conversation to collaborators Jul 23, 2019
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

6 participants