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

set selectTextStyle propType to Text.propTypes.style #26

Merged
merged 1 commit into from
Sep 23, 2017

Conversation

mienaikoe
Copy link

@mienaikoe mienaikoe commented Sep 17, 2017

#25

@peacechen
Copy link
Owner

peacechen commented Sep 17, 2017

Thanks @mienaikoe

React native allows both style objects and arrays for style properties. For example:

style={[styles.default, {color:'red'}, this.props.overrideStyle]}

It would be more flexible to allow both. Just noticed that all the other style props should be updated the same way :)

@mienaikoe
Copy link
Author

mienaikoe commented Sep 17, 2017

Sure, Maybe that should be done under a different issue ticket tho.

@peacechen
Copy link
Owner

If it's not too much trouble, would you mind updating all the style props with PropTypes.oneOfType ? I won't be able to get to it right now and it'll likely fall through the cracks with my bad memory :D

@mikaello
Copy link
Collaborator

I have tried to investigate prop types for arrays in style-properties, but it seems that this is somehow included in Text.propTypes.style (and other prop types for styles from RN). There is no warning in debug-mode when using arrays without explicit specifying it.

In Text from RN all properties goes trough a function which flattens any array present. So as far as I can see Text.propTypes.style accepts both objects and arrays with objects. ViewPropTypes uses the same function.

Other well maintained react-native modules seem to just use Text.propTypes.style, and not PropTypes.oneOfType to include array, e.g. react-native-elements Search inputStyle.

So I think this PR is good to go, and no further editing is necessary.

@peacechen
Copy link
Owner

Thanks @mikaello for clarifying the style proptypes behavior. Wasn't aware that arrays are automatically flattened first.

@peacechen peacechen merged commit b9bd846 into peacechen:master Sep 23, 2017
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.

3 participants