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.

Sign up for GitHub

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

feat: Nutrition Game #70 #114

Draft
wants to merge 33 commits into
base: master
Choose a base branch
from
+787 −2
Draft

feat: Nutrition Game #70 #114

Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
c7043b9
bug: shortCut issue solved
Shmigolk Jul 27, 2022
9c92737
UI: add settings icon and moved links in NavBar to the right
Shmigolk Jul 27, 2022
d0149e2
UI: align pictures and add gap
Shmigolk Jul 27, 2022
4529d8b
UI: add settings icon and moved links in NavBar to the right
Shmigolk Jul 27, 2022
0df05c0
UI: align pictures and add gap
Shmigolk Jul 28, 2022
c89e1f2
UI: create table component
Shmigolk Jul 29, 2022
3a5bd0d
UI: add media queueueueueueury
Shmigolk Jul 29, 2022
9412b2c
fix: add controlled input
Shmigolk Jul 31, 2022
d36a119
fix: add controlled input
Shmigolk Jul 31, 2022
ce50230
Merge remote-tracking branch 'upstream/master' into nutrition
Shmigolk Aug 1, 2022
93d4b4e
feature: change the code of additional nutr
Shmigolk Aug 1, 2022
fe21b02
feature: implement adding nutriments to the table
Shmigolk Aug 1, 2022
a674dfd
feature: implement adding nutriments to the table and removing them f…
Shmigolk Aug 1, 2022
630f0ef
feature: add delete icons and implement deleting nutriments from the …
Shmigolk Aug 1, 2022
257bd35
feature: add Title and the description of the product (hardcoded)
Shmigolk Aug 1, 2022
cb1e486
feature: get data from API. Display picture
Shmigolk Aug 2, 2022
ecd4f07
feature: make clickHandler shorter
Shmigolk Aug 2, 2022
fb90329
feature: move nutriments arraus out of the component. Refresh the tab…
Shmigolk Aug 3, 2022
b952bc4
feature: replace useEffect with React.useEffect
Shmigolk Aug 3, 2022
7d36892
feature: restructure Table component. Make TableRowComp
Shmigolk Aug 9, 2022
b179515
feature: now delete icon toggle display property
Shmigolk Aug 9, 2022
cb6e2c2
feature: now delete icon toggle display property
Shmigolk Aug 9, 2022
089d9df
Merge remote-tracking branch 'origin/master' into nutrition
alexfauquette Aug 9, 2022
79d4643
remove unused import
alexfauquette Aug 9, 2022
9b873e3
feature: now i18n works into the table
Shmigolk Aug 11, 2022
10f5d56
Merge branch 'master' into nutrition
Shmigolk Aug 11, 2022
0add0f0
feature: add keys to common json and en json
Shmigolk Aug 11, 2022
93e7719
Merge remote-tracking branch 'origin/nutrition' into nutrition
Shmigolk Aug 11, 2022
79764f1
feature: display correct properties in select elements
Shmigolk Aug 12, 2022
ae0bffe
feature: on change is working
Shmigolk Aug 12, 2022
30be061
UI: fixed width problem
Shmigolk Aug 13, 2022
7214c83
UI: change the order of elements of productCard and add temporary bor…
Shmigolk Aug 13, 2022
d19a758
bug: fix onChange handler bug
Shmigolk Aug 13, 2022
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
Prev Previous commit
Next Next commit
fix: add controlled input
@Shmigolk
Shmigolk committed Jul 31, 2022
commit 9412b2c224a9921fc4da81c70871eb0755a95397
3 changes: 2 additions & 1 deletion src/i18n/en.json
View file
Original file line number Diff line number Diff line change
@@ -56,7 +56,8 @@
"insights": "Insights",
"nutritions": "Nutrition facts",
"settings": "Settings",
"eco-score": "Eco-score"
"eco-score": "Eco-score",
"nutrition": "nutrition"
},
"insights": {
"insights": "Insights",
50 changes: 50 additions & 0 deletions src/pages/nutrition/additionalNutritions.jsx
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import * as React from "react";
import TextField from "@mui/material/TextField";
import Autocomplete from "@mui/material/Autocomplete";

const options = [
"Option 1",
"Option 2",
"Option 3",
"Option 4",
"Option 5",
"Option 6",
"Option 7",
"Option 8"
];

export default function ControllableStates() {
const [data, setData] = React.useState([]);
const [inputValue, setInputValue] = React.useState("");

return (
<div>
<ol>
{data.map((obj) => (
<li key={obj.label}>{obj.label}</li>
))}
</ol>
<br />
<Autocomplete
value={null}
onChange={(event, newValue) => {
if (newValue !== "" && data.every((obj) => obj.label !== newValue)) {
// if it's a new one
setData((previouseData) => [...previouseData, { label: newValue }]);
}
// Reset the input content on selection
setInputValue("");
}}
inputValue={inputValue}
onInputChange={(event, newInputValue) => {
setInputValue(newInputValue);
}}
options={options}
sx={{ width: 300 }}
renderInput={(params) => (
<TextField {...params} label="Add nutriment" />
)}
/>
</div>
);
}
29 changes: 19 additions & 10 deletions src/pages/nutrition/index.jsx
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
import * as React from "react";
import NutritionTable from './table'
import ImageTable from './picture'
import { Box } from "@mui/material"
import { flexbox } from '@material-ui/system'
import NutritionTable from "./table";
import ImageTable from "./picture";
import { Box } from "@mui/material";
import { flexbox } from "@material-ui/system";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import { flexbox } from "@material-ui/system";

imported but not used. Normally when you do yarn start in the console, you should see some warnings about lines where you import or define variables you do not use. You should remove them to get a cleaner code.

Here is what I get when I run yarn start from your branch

image


export default function Nutrition(){
return(
<Box display={'flex'} flexDirection={{ xs: "column", md: 'row' }} sx={{width: 1, height: 1, alignItems: 'center', justifyContent: 'center', border: "5px solid red"}}>
<ImageTable/>
<NutritionTable/>
export default function Nutrition() {
return (
<Box display={"flex"}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<Box display={"flex"}
<Box display="flex"

Some points which will be useful for tech interview about React (JSX syntax to be precise):

  • By default props are provided as follow propName={propValue}
  • You do not need the {} for sting. FOr example propName={"some text"} is same as propName="some text"
  • A prop alone is equivalent to true. For example <Component isOpen /> is the same as <Component isOpen={true} />.

flexDirection={{ xs: "column", md: "row" }}
gap={2}
sx={{
width: 1,
height: 1,
alignItems: "center",
justifyContent: "center",
padding: 4
}}>
<ImageTable />
<NutritionTable />
</Box>
)
);
}
5 changes: 2 additions & 3 deletions src/pages/nutrition/picture.jsx
View file
Original file line number Diff line number Diff line change
@@ -9,9 +9,8 @@ export default function ImageTable() {
<Box flexGrow={1}
flexShrink={1}
sx={{
marginBottom: 4,
maxWidth: '300px',
border: '5px solid red'
maxWidth: '380px',

}}>
<Zoom wrapStyle={{ height: "100%" }}>
<img
177 changes: 96 additions & 81 deletions src/pages/nutrition/table.jsx
View file
Original file line number Diff line number Diff line change
@@ -9,98 +9,113 @@ import TextField from "@mui/material/TextField";
import SelectAutoWidth from "./selectComp";
import { Box } from "@mui/material";
import Checkbox from "@mui/material/Checkbox";
import ControllableStates from "./additionalNutritions";

function createData(
label, property, unit
) {
return { label, property, unit };
}

const nutritions = [{
off_nutriment_id: "energy_kj",
label: "Energie (kJ)",
value: "",
unit: 'null',
quantification: "=",
robotoffPrediction: null
},
{
off_nutriment_id: "energy_kcal",
label: "Protein",
value: "",
unit: 'null',
quantification: "<",
robotoffPrediction: null
},
{
off_nutriment_id: "energy_kcal",
label: "Shugar",
value: "",
unit: 'null',
quantification: "<",
robotoffPrediction: null
},
{
off_nutriment_id: "energy_kcal",
label: "Fat",
value: "",
unit: null,
quantification: "<",
robotoffPrediction: null
},
{
off_nutriment_id: "energy_kcal",
label: "Energie (kCal)",
value: "",
unit: null,
quantification: "<",
robotoffPrediction: null
}
]
export default function NutritionTable() {

const rows = nutritions.map(nutrition => {
return (
createData(<Box sx={{
display: "flex",
alignItems: "center",
flexDirection: "row",
}}><TextField id="outlined-basic"
label={nutrition.label}
variant="outlined"
sx={{width: '10rem',}}
/> , <SelectAutoWidth /></Box>,
<Checkbox sx={{}}/>))
})
const [nutriments, setNutriments] = React.useState([
{
off_nutriment_id: "energy_kj",
label: "Energie (kJ)",
value: "",
unit: "null",
quantification: "=",
robotoffPrediction: null
},
{
off_nutriment_id: "energy_kcal",
label: "Protein",
value: "",
unit: "null",
quantification: "<",
robotoffPrediction: null
},
{
off_nutriment_id: "energy_kcal",
label: "Shugar",
value: "",
unit: "null",
quantification: "<",
robotoffPrediction: null
},
{
off_nutriment_id: "energy_kcal",
label: "Fat",
value: "",
unit: null,
quantification: "<",
robotoffPrediction: null
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe this values could live outside of the component .... const NUTRIMENTS = [ ... ] and just used in the useState const [nutriments, setNutriments] = React.useState(NUTRIMENTS). That way the component is a bit more clean

])

function onchangeHandler(e) {
const {value, name} = e.target
console.log('value:', value, 'name:', name)
setNutriments(prevState => prevState.map(
nutr => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I prefer not to use abbreviations as they quickly become difficult to read and understand, specially for other developers. Plase use a whole word, more typing but better readability. In this cases I tend to use the suffix ..item, something like nutrimentItem that way I understand that it is the thing inside the loop

return name === nutr.label? {...nutr, value} : nutr
}
))
}

const rows = nutriments.map(nutrition => {
return (
createData(<Box sx={{
display: "flex",
alignItems: "center",
flexDirection: "row"
}}><TextField id="outlined-basic"
label={nutrition.label}
variant="outlined"
sx={{ width: "10rem" }}
value={nutrition.value}
name={nutrition.label}
onChange={onchangeHandler}
/> , <SelectAutoWidth /></Box>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd advice against this kind of implicit closing elements (</Box>) as the chance of forgetting or using it wrongly is relatively high

Copy link
Collaborator

Choose a reason for hiding this comment

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

plase see the coment below

Copy link
Member

Choose a reason for hiding this comment

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

I assume it's a mistake. The </Box> is planned to be before the coma But that's not important.

@Shmigolk I think you get confused by the MUI demo, because it's cells only contain strings/numbers, so they use

<TableCell>{row.thValueToDisplay}</TabCell>

In our case, we want to render inputs which is a bit more complex.

I propose you the following strategy:
create a component NutritionTableRow as follow:

const NutritionTableRow = ({ nutrimentData, updateValue, updateUnit, updateQuantificator }) => {
	const onNutrimentValueChange = (event) => {updateValue(event.target.value)}
	const onUnitChange = () => {...}
	const onQuantificatorChange = () => {...}
	
	const {label, value, quantification, unit} = nutrimentData
	
	return <TableRow>
		<TableCell>
			<TextField select value={quantification} onChange={onQuantificatorChange}>
				<MenuItem value="=">=</MenuItem>
				<MenuItem value=">">></MenuItem>
				<MenuItem value="<"><</MenuItem>
			</TextField>
		</TableCell>
		<TableCell>
			<TextField label={label} value={value} onChange={onNutrimentValueChange} />
		</TableCell>
		//  Same for the unit
	<TableRow>
}

Then you will be able to simplify your table content as follow:

	nutriments.map(nutriment => <NutritionTableRow nutrimentData={nutriment}  updateValue={updateUnit(nutriment.off_nutriment_id)}/>)

The trickiest part being updateValue which is a function returning a function.

const updateValue = (nutrimentId) => (newValue) => {
	// We get the index of the nutriment we want to update
	const indexToModify = nutriments.findIndex(nutriment => nutriment.off_nutriment_id === nutrimentId)
	
	// early return if not element correspond to this id
	if(indexToModify < 0){ return }
	
	// We update the value to the nutriment
	const newNutriment = {...nutriments[indexToModify], value: newValue }
	setNutriments([...nutriments.slice(0, indexToModify), newNutriment, ...nutriments.slice(indexToModify+1)])
}

<Checkbox sx={{}} />));
Copy link
Collaborator

Choose a reason for hiding this comment

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

is sx={{}} needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed checkbox and leaved delete icon. Still working on it

});

export default function NutritionTable() {
return (
<TableContainer sx={{margin: 0, maxWidth: "1000px", minWidth: '400px', width: '30%'}}>
<Table sx={{ border: "5px solid red"}}
aria-label="simple table">
<TableHead>
<TableRow>
<TableCell sx={{ maxWidth: '8em', fontSize: 'large', fontWeight: 'bold'}}>nutrition.table.value</TableCell>
<TableCell align="left" sx={{}}>isPresent</TableCell>
</TableRow>
</TableHead>
<TableBody>
{rows.map((row) => (
<TableRow
key={row.label}
sx={{ "&:last-child td, &:last-child th": { border: 0 } }}
>
<TableCell component="th"
scope="row"
sx={{border: '5px solid red', width: 10}}
>
{row.label}
</TableCell>
<TableCell align="left" sx={{border: '5px solid red'}}>{row.property}</TableCell>
<Box>
<TableContainer sx={{ margin: 0, maxWidth: "1000px", width: "380px" }}>
<Table aria-label="simple table">
<TableHead>
<TableRow>
<TableCell sx={{
maxWidth: "8em",
fontSize: "large",
fontWeight: "bold"
}}>nutrition.table.value</TableCell>
<TableCell align="left"
sx={{}}>isPresent</TableCell>
Copy link
Collaborator

Choose a reason for hiding this comment

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

is sx={{}} needed?

</TableRow>
))}
</TableBody>
</Table>
</TableContainer>
</TableHead>
<TableBody>
{rows.map((row) => (
Copy link
Collaborator

Choose a reason for hiding this comment

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

what about using the map directly?, I mean, instead of first map to createData() and using that result

<TableRow
key={row.label}
sx={{ "&:last-child td, &:last-child th": { border: 0 } }}
>
<TableCell component="th"
scope="row"
sx={{ width: 10 }}
>
{row.label}
</TableCell>
<TableCell align="left"
sx={{ width: "1rem" }}>{row.property}</TableCell>
</TableRow>
))}
</TableBody>
</Table>
</TableContainer>
<ControllableStates />
</Box>
);
}