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

Imperial Units Feature #365

Merged
merged 19 commits into from
Mar 10, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
ad539b5
cross-platform jest TZ fix and grammar error for gafa guide
ThomasLatham Dec 21, 2022
c3abd32
changing projectId back to original
ThomasLatham Dec 21, 2022
4d28a21
reverting to 'a ethical' for single-purpose PR
ThomasLatham Dec 22, 2022
9e2f2d0
switch list item component, units setting UI and redux, related trans…
ThomasLatham Dec 26, 2022
2df6b40
removed unused import
ThomasLatham Dec 26, 2022
dbccc70
Merge branch 'imperial-system'
ThomasLatham Dec 26, 2022
0af7c7e
Merge branch 'main' of https://github.com/NMF-earth/nmf-app
ThomasLatham Dec 26, 2022
59252fd
Merge branch 'main' of https://github.com/NMF-earth/nmf-app
ThomasLatham Jan 31, 2023
d03df83
applying unit preference state to emission adding
ThomasLatham Jan 31, 2023
c64bc6c
extracted displayUnits and corresponding value to own functions; some…
ThomasLatham Feb 14, 2023
d687255
updated snapshots for the change to ListItemSwitch's switch padding
ThomasLatham Feb 14, 2023
00551ad
Merge branch 'main' of https://github.com/NMF-earth/nmf-app into impe…
ThomasLatham Feb 14, 2023
f390d9f
adding tests for new unit-conversion functions
ThomasLatham Feb 14, 2023
efd101a
unit preference for EmissionListItem and MonthlyBudget slider; snapsh…
ThomasLatham Feb 14, 2023
6015200
snapshots actually updated
ThomasLatham Feb 14, 2023
f885e3f
adding translations for units
ThomasLatham Feb 15, 2023
33482d9
updating calculation tests for translations
ThomasLatham Feb 15, 2023
3533ca7
implementing unit preference in all the nooks and crannies; updated s…
ThomasLatham Feb 15, 2023
0abca19
switch center fix; units calc functions legibility; metric in snapshots
ThomasLatham Feb 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions app/components/ListItemSwitch/ListItemSwitch.styles.ts
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: {
Copy link
Member

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 :

  text {
    paddingVertical: 18,
  },

And then :

      <Text.Secondary style={styles.text} numberOfLines={1}>
        {title}
      </Text.Secondary>`

This should fix your alignments issues ;)

Copy link
Contributor Author

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)?

Copy link
Member

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%)

Copy link
Contributor Author

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!

height: 18
},
topLine: {
borderTopColor: Colors.grey10,
borderTopWidth: 1.6,
},
bottomLine: {
borderBottomColor: Colors.grey10,
borderBottomWidth: 1.6,
},
});
40 changes: 40 additions & 0 deletions app/components/ListItemSwitch/ListItemSwitch.tsx
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The switch needs to be center
Screenshot 2023-02-03 at 10 33 12

Copy link
Contributor Author

@ThomasLatham ThomasLatham Feb 14, 2023

Choose a reason for hiding this comment

The 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).

It seems to be working in Android for me.
units_toggle_center_android

style={switchStyle}
value={value}
onChange={onChange}
/>
</View>
);
};

export default ListItemSwitch;
35 changes: 35 additions & 0 deletions app/components/ListItemSwitch/__tests__/ListItemSwitch.test.tsx
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>
`;
1 change: 1 addition & 0 deletions app/components/ListItemSwitch/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default } from "./ListItemSwitch";
15 changes: 15 additions & 0 deletions app/components/ListItemSwitch/stories/ListItemSwitch.story.tsx
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.
// }}
// />
// ));
2 changes: 2 additions & 0 deletions app/components/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import TextInput from "./TextInput";
import StickersImage from "./StickersImage";
import SelectableListItem from "./SelectableListItem";
import ListItem from "./ListItem";
import ListItemSwitch from "./ListItemSwitch"
import HTMLImage from "./HTMLImage";
import InfoButton from "./InfoButton";
import NoEmission from "./NoEmission";
Expand All @@ -21,6 +22,7 @@ export {
EmissionListItemProps,
HTMLImage,
ListItem,
ListItemSwitch,
SelectableListItem,
StickersImage,
SocialMedia,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Object {
"changeLanguage": [Function],
"incrementTimesStarted": [Function],
"toggleNotifications": [Function],
"toggleUnits": [Function],
"updateLocation": [Function],
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,12 @@ describe("userPreferences actions should", () => {
};
expect(userPreferences.actions.incrementTimesStarted()).toEqual(expectedAction);
});

it("be able to toggle units", () => {
const expectedAction = {
type: userPreferences.actions.toggleUnits.toString(),
payload: false,
};
expect(userPreferences.actions.toggleUnits(false)).toEqual(expectedAction);
})
});
Loading