-
-
Notifications
You must be signed in to change notification settings - Fork 156
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
Imperial Units Feature #365
Changes from 9 commits
ad539b5
c3abd32
4d28a21
9e2f2d0
2df6b40
dbccc70
0af7c7e
59252fd
d03df83
c64bc6c
d687255
00551ad
f390d9f
efd101a
6015200
f885e3f
33482d9
3533ca7
0abca19
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import { StyleSheet } from "react-native"; | ||
|
||
import { Colors } from "style"; | ||
|
||
export default StyleSheet.create({ | ||
container: { | ||
flexDirection: "row", | ||
justifyContent: "space-between", | ||
alignItems: "center", | ||
paddingVertical: 18, | ||
flex: 1, | ||
}, | ||
switch: { | ||
height: 18 | ||
}, | ||
topLine: { | ||
borderTopColor: Colors.grey10, | ||
borderTopWidth: 1.6, | ||
}, | ||
bottomLine: { | ||
borderBottomColor: Colors.grey10, | ||
borderBottomWidth: 1.6, | ||
}, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
import React from "react"; | ||
import { View, ViewStyle, StyleProp } from "react-native"; | ||
import { Switch } from "react-native-gesture-handler"; | ||
|
||
import styles from "./ListItemSwitch.styles"; | ||
import Text from "../Text"; | ||
|
||
interface Props { | ||
title: string; | ||
value: boolean; | ||
onChange: () => void; | ||
showTopLine?: boolean; | ||
showBottomLine?: boolean; | ||
} | ||
|
||
const ListItemSwitch: React.FC<Props> = ({ title, value, onChange, showTopLine, showBottomLine }) => { | ||
const containerStyle: StyleProp<ViewStyle> = [styles.container]; | ||
const switchStyle: StyleProp<ViewStyle> = styles.switch; | ||
|
||
if (showTopLine) { | ||
containerStyle.push(styles.topLine); | ||
} | ||
|
||
if (showBottomLine) { | ||
containerStyle.push(styles.bottomLine); | ||
} | ||
|
||
return ( | ||
<View style={containerStyle}> | ||
<Text.Secondary numberOfLines={1}>{title}</Text.Secondary> | ||
<Switch | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'll need some help with this one. I added some padding to the <Switch> descendant component of <ListItemSwitch> in the most recent push to maybe help fix this, but I can't actually perform a visual check since I don't own anything that can run or emulate iOS (as far as I know). |
||
style={switchStyle} | ||
value={value} | ||
onChange={onChange} | ||
/> | ||
</View> | ||
); | ||
}; | ||
|
||
export default ListItemSwitch; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
import "react-native"; | ||
import React from "react"; | ||
import { create } from "react-test-renderer"; | ||
|
||
import ListItemSwitch from "../ListItemSwitch"; | ||
|
||
describe("<ListItemSwitch />", () => { | ||
const props = { | ||
title: "Germany", | ||
value: true, | ||
onChange: () => { | ||
// do nothing. | ||
}, | ||
}; | ||
const wrapper = create(<ListItemSwitch {...props} />); | ||
|
||
test("render", () => { | ||
expect(wrapper).toMatchSnapshot(); | ||
}); | ||
|
||
test("ListItem renders correctly with topLine", () => { | ||
const tree = create(<ListItemSwitch showTopLine {...props} />).toJSON(); | ||
expect(tree).toMatchSnapshot(); | ||
}); | ||
|
||
test("ListItem renders correctly with bottomLine", () => { | ||
const tree = create(<ListItemSwitch showBottomLine {...props} />).toJSON(); | ||
expect(tree).toMatchSnapshot(); | ||
}); | ||
|
||
test("ListItem renders correctly with both topLine and bottomLine", () => { | ||
const tree = create(<ListItemSwitch showBottomLine showTopLine {...props} />).toJSON(); | ||
expect(tree).toMatchSnapshot(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,197 @@ | ||
// Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
||
exports[`<ListItemSwitch /> ListItem renders correctly with both topLine and bottomLine 1`] = ` | ||
<View | ||
style={ | ||
Array [ | ||
Object { | ||
"alignItems": "center", | ||
"flex": 1, | ||
"flexDirection": "row", | ||
"justifyContent": "space-between", | ||
"paddingVertical": 18, | ||
}, | ||
Object { | ||
"borderTopColor": "#F0F0F0", | ||
"borderTopWidth": 1.6, | ||
}, | ||
Object { | ||
"borderBottomColor": "#F0F0F0", | ||
"borderBottomWidth": 1.6, | ||
}, | ||
] | ||
} | ||
> | ||
<Text.Secondary | ||
numberOfLines={1} | ||
> | ||
Germany | ||
</Text.Secondary> | ||
<RCTSwitch | ||
accessibilityRole="switch" | ||
collapsable={false} | ||
handlerTag={4} | ||
handlerType="NativeViewGestureHandler" | ||
onChange={[Function]} | ||
onGestureHandlerEvent={[Function]} | ||
onGestureHandlerStateChange={[Function]} | ||
onResponderTerminationRequest={[Function]} | ||
onStartShouldSetResponder={[Function]} | ||
style={ | ||
Array [ | ||
Object { | ||
"height": 31, | ||
"width": 51, | ||
}, | ||
Object { | ||
"height": 18, | ||
}, | ||
] | ||
} | ||
value={true} | ||
/> | ||
</View> | ||
`; | ||
|
||
exports[`<ListItemSwitch /> ListItem renders correctly with bottomLine 1`] = ` | ||
<View | ||
style={ | ||
Array [ | ||
Object { | ||
"alignItems": "center", | ||
"flex": 1, | ||
"flexDirection": "row", | ||
"justifyContent": "space-between", | ||
"paddingVertical": 18, | ||
}, | ||
Object { | ||
"borderBottomColor": "#F0F0F0", | ||
"borderBottomWidth": 1.6, | ||
}, | ||
] | ||
} | ||
> | ||
<Text.Secondary | ||
numberOfLines={1} | ||
> | ||
Germany | ||
</Text.Secondary> | ||
<RCTSwitch | ||
accessibilityRole="switch" | ||
collapsable={false} | ||
handlerTag={3} | ||
handlerType="NativeViewGestureHandler" | ||
onChange={[Function]} | ||
onGestureHandlerEvent={[Function]} | ||
onGestureHandlerStateChange={[Function]} | ||
onResponderTerminationRequest={[Function]} | ||
onStartShouldSetResponder={[Function]} | ||
style={ | ||
Array [ | ||
Object { | ||
"height": 31, | ||
"width": 51, | ||
}, | ||
Object { | ||
"height": 18, | ||
}, | ||
] | ||
} | ||
value={true} | ||
/> | ||
</View> | ||
`; | ||
|
||
exports[`<ListItemSwitch /> ListItem renders correctly with topLine 1`] = ` | ||
<View | ||
style={ | ||
Array [ | ||
Object { | ||
"alignItems": "center", | ||
"flex": 1, | ||
"flexDirection": "row", | ||
"justifyContent": "space-between", | ||
"paddingVertical": 18, | ||
}, | ||
Object { | ||
"borderTopColor": "#F0F0F0", | ||
"borderTopWidth": 1.6, | ||
}, | ||
] | ||
} | ||
> | ||
<Text.Secondary | ||
numberOfLines={1} | ||
> | ||
Germany | ||
</Text.Secondary> | ||
<RCTSwitch | ||
accessibilityRole="switch" | ||
collapsable={false} | ||
handlerTag={2} | ||
handlerType="NativeViewGestureHandler" | ||
onChange={[Function]} | ||
onGestureHandlerEvent={[Function]} | ||
onGestureHandlerStateChange={[Function]} | ||
onResponderTerminationRequest={[Function]} | ||
onStartShouldSetResponder={[Function]} | ||
style={ | ||
Array [ | ||
Object { | ||
"height": 31, | ||
"width": 51, | ||
}, | ||
Object { | ||
"height": 18, | ||
}, | ||
] | ||
} | ||
value={true} | ||
/> | ||
</View> | ||
`; | ||
|
||
exports[`<ListItemSwitch /> render 1`] = ` | ||
<View | ||
style={ | ||
Array [ | ||
Object { | ||
"alignItems": "center", | ||
"flex": 1, | ||
"flexDirection": "row", | ||
"justifyContent": "space-between", | ||
"paddingVertical": 18, | ||
}, | ||
] | ||
} | ||
> | ||
<Text.Secondary | ||
numberOfLines={1} | ||
> | ||
Germany | ||
</Text.Secondary> | ||
<RCTSwitch | ||
accessibilityRole="switch" | ||
collapsable={false} | ||
handlerTag={1} | ||
handlerType="NativeViewGestureHandler" | ||
onChange={[Function]} | ||
onGestureHandlerEvent={[Function]} | ||
onGestureHandlerStateChange={[Function]} | ||
onResponderTerminationRequest={[Function]} | ||
onStartShouldSetResponder={[Function]} | ||
style={ | ||
Array [ | ||
Object { | ||
"height": 31, | ||
"width": 51, | ||
}, | ||
Object { | ||
"height": 18, | ||
}, | ||
] | ||
} | ||
value={true} | ||
/> | ||
</View> | ||
`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { default } from "./ListItemSwitch"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
// import React from "react"; | ||
// import { storiesOf } from "@storybook/react-native"; | ||
// import { text } from "@storybook/addon-knobs"; | ||
|
||
// import ListItemSwitch from ".."; | ||
|
||
// storiesOf("ListItemSwitch", module).add("switch list item", () => ( | ||
// <ListItemSwitch | ||
// title={text("Title", "Germany")} | ||
// value={true} | ||
// onChange={() => { | ||
// // do nothing. | ||
// }} | ||
// /> | ||
// )); |
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.
switch style can be deleted. Instead, remove
paddingVertical: 18,
from the container and create a text style like this :And then :
This should fix your alignments issues ;)
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.
Oh yeah that worked. Comparing ListItem and its styles to ListItemSwitch and its styles now to try and figure out why it worked. Could we make the same change to ListItem (move the padding to the container component's <Text> child) to achieve the same result? And moreover, why does moving the padding inward help center the switch? Is it because the container style's
alignItems: center
, and when you add padding to the container it changes the center slightly (that doesn't make a whole lot of sense, but I'm not sure what's going on)?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.
When the components inside a parent component are almost the same size, then applying the padding to the parent is usually the best way to go. However, when you have different size component inside one parent which has the padding, then you can have some kind of "overflow" like in ListItemSwitch. In my experience, the best way to visualize/debug that is to put different background color to the different components, it helps understanding how to apply constraints in a "lego/tetrix" way. So yeap, I think it's thanks to the
alignItems: center
that child components are aligned now (I did a quick debugging yesterday so not sure 100%)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.
Oh okay, that makes sense. I'm guessing then you want to apply the padding to the component with the smallest size in the target axis? I should probably review the box model. Also changing the colors totally helped visualize that and enabled me to actually play around with it meaningfully instead of pure guess-and-check. Thanks!