fix: move paddingHorizontal out from content style #1078
Conversation
The current implementation enforces a paddingHorizontal of 16 as default for the content style. This style is inherited by both TouchableRipple and its child view. TouchableRipple container values can be overridden by passing the style property value when creating a DataTableRow, but the child view will always have the paddingHorizontal value of 16. As part of this commit we will allow the style class property to override the child view as well.
|
Hey @jose96043, thank you for your pull request 🤗. The documentation from this branch can be viewed here. Please remember to update Typescript types if you changed API. |
| @@ -40,7 +40,7 @@ class DataTableRow extends React.Component<Props> { | |||
| onPress={onPress} | |||
| style={[styles.container, { borderBottomColor }, style]} | |||
| > | |||
| <View style={styles.content}>{this.props.children}</View> | |||
| <View style={[styles.content, style]}>{this.props.children}</View> | |||
There was a problem hiding this comment.
I don't think we want to duplicate style like this, you apply some padding and now it'll get applied 2 times
There was a problem hiding this comment.
sorry good point can we have a content field exposed as part of the class? Or can we remove the hard coded padding of 16? This limits how customizable the class can be.
There was a problem hiding this comment.
I don't remember why the inner view is needed at all. Can we remove it? cc @jaulz
There was a problem hiding this comment.
I think I put it there because a TouchableRipple can only have a single child.
There was a problem hiding this comment.
@jaulz Is there a reason why there needs to be a horizontalPadding of 16?
There was a problem hiding this comment.
How about moving the padding to the TouchableRipple styles so it can be overriden by user's passed style?
There was a problem hiding this comment.
Yep, that should be fine. The padding is part of the specs.
The ability of overriding style properties makes things more flexible. In order to achieve this within DataTableRow, we have moved paddingHorizontal out from content style which can't be overridden, and put it in container style which can be overridden with the style class property.
jose96043
changed the title
Motivation
The ability of overriding style properties makes things more flexible. In order to achieve this within DataTableRow, we have moved paddingHorizontal out from content style which can't be overridden, and put it in container style which can be overridden with the style class property.
Test plan