feat: Nutrition Game #70 #114
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import * as React from 'react'; | ||
| import TextField from '@mui/material/TextField'; | ||
| import Autocomplete from '@mui/material/Autocomplete'; | ||
|
|
||
| export default function AdditionalNutriments({options, setNutriments, setAdditionalNutriments}) { | ||
| return ( | ||
| <Autocomplete | ||
| disablePortal | ||
| id="combo-box-demo" | ||
| options={options} | ||
| sx={{ width: 300 }} | ||
| onChange={(event) => { | ||
| const nutrIndex = event.target.dataset.optionIndex | ||
| if (nutrIndex) { | ||
| setNutriments(prev => [...prev, options[nutrIndex]]) | ||
| setAdditionalNutriments(prev => prev.filter(elem => elem !== options[nutrIndex])) | ||
| console.log(options) | ||
Shmigolk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| }} | ||
| renderInput={(params) => <TextField {...params} label="Nutrition" />} | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| // Top 100 films as rated by IMDb users. http://www.imdb.com/chart/top | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +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"; | ||||||
|
There was a problem hiding this comment.
Suggested change
imported but not used. Normally when you do Here is what I get when I run
|
||||||
|
|
||||||
| export default function Nutrition() { | ||||||
| return ( | ||||||
| <Box display={"flex"} | ||||||
|
There was a problem hiding this comment.
Suggested change
Some points which will be useful for tech interview about React (JSX syntax to be precise):
|
||||||
| flexDirection={{ xs: "column", md: "row" }} | ||||||
| gap={2} | ||||||
| sx={{ | ||||||
| width: 1, | ||||||
| height: 1, | ||||||
| alignItems: "center", | ||||||
| justifyContent: "center", | ||||||
| padding: 4 | ||||||
| }}> | ||||||
| <ImageTable /> | ||||||
| <NutritionTable /> | ||||||
| </Box> | ||||||
| ); | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import * as React from 'react'; | ||
| import Box from '@mui/material/Box'; | ||
| import Zoom from "react-medium-image-zoom"; | ||
| import Button from "@mui/material/Button"; | ||
| import DeleteIcon from "@mui/icons-material/Delete"; | ||
|
|
||
| export default function ImageTable() { | ||
| return ( | ||
| <Box flexGrow={1} | ||
| flexShrink={1} | ||
| sx={{ | ||
| maxWidth: '380px', | ||
|
|
||
| }}> | ||
| <Zoom wrapStyle={{ height: "100%" }}> | ||
| <img | ||
| // TODO: use getFullSizeImage when the zoom is activated | ||
|
There was a problem hiding this comment. it would be nice to have an issue linked to this TODO, that way we don't forget about it :) |
||
| // src={getFullSizeImage(question.source_image_url)} | ||
| src={"https://static.openfoodfacts.org/images/image-placeholder.png"} | ||
| alt="" | ||
|
There was a problem hiding this comment. if possible please add the alt description |
||
| style={{ maxWidth: "100%", maxHeight: "100%" }} | ||
| /> | ||
| </Zoom> | ||
| <Box display={"flex"} flexDirection={"row"} Gap={'.5em'}> | ||
| <Button | ||
| color="secondary" | ||
| variant="contained" | ||
| size="large" | ||
| sx={{ display: "flex", flexDirection: "column", flexGrow: 1, width: '47%' }} | ||
| > | ||
| SKIP | ||
| </Button> | ||
| <Button | ||
| color="success" | ||
| variant="contained" | ||
| size="large" | ||
| sx={{ display: "flex", flexDirection: "column", flexGrow: 1, width: '47%' }} | ||
| > | ||
| VALIDATE | ||
| </Button> | ||
| </Box> | ||
| </Box>) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import * as React from 'react'; | ||
| import InputLabel from '@mui/material/InputLabel'; | ||
| import MenuItem from '@mui/material/MenuItem'; | ||
| import FormControl from '@mui/material/FormControl'; | ||
| import Select, { SelectChangeEvent } from '@mui/material/Select'; | ||
|
|
||
| export default function SelectAutoWidth({}) { | ||
| const [age, setAge] = React.useState(''); | ||
|
|
||
| const handleChange = (event: SelectChangeEvent) => { | ||
| setAge(event.target.value); | ||
| }; | ||
|
|
||
| return ( | ||
| <div> | ||
| <FormControl sx={{ m: 1, minWidth: 80, margin: '0' }}> | ||
| <InputLabel id="demo-simple-select-autowidth-label">Unit</InputLabel> | ||
|
There was a problem hiding this comment. these ids could be improved, what about something like 'dropdown-unit-label' ?... better to use something as specific as we can or we could end up having multiple similar ids |
||
| <Select | ||
| labelId="demo-simple-select-autowidth-label" | ||
| id="demo-simple-select-autowidth" | ||
| value={age} | ||
| onChange={handleChange} | ||
| autoWidth | ||
| label="Age" | ||
| > | ||
| <MenuItem value=""> | ||
| <em>None</em> | ||
| </MenuItem> | ||
| <MenuItem value={10}>g</MenuItem> | ||
|
There was a problem hiding this comment. think it would be nice to somewhere something that gives you an idea of what these values (10, 21, 22) actually mean. What about using an object outside of this component, something like |
||
| <MenuItem value={21}>mg</MenuItem> | ||
| <MenuItem value={22}>kg</MenuItem> | ||
| </Select> | ||
| </FormControl> | ||
| </div> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| import * as React from "react"; | ||
| import Table from "@mui/material/Table"; | ||
| import TableBody from "@mui/material/TableBody"; | ||
| import TableCell from "@mui/material/TableCell"; | ||
| import TableContainer from "@mui/material/TableContainer"; | ||
| import TableHead from "@mui/material/TableHead"; | ||
| import TableRow from "@mui/material/TableRow"; | ||
| import TextField from "@mui/material/TextField"; | ||
| import SelectAutoWidth from "./selectComp"; | ||
| import { Box } from "@mui/material"; | ||
| import Checkbox from "@mui/material/Checkbox"; | ||
| import AdditionalNutriments from "./additionalNutritions"; | ||
|
|
||
| function createData( | ||
| label, property, unit | ||
| ) { | ||
| return { label, property, unit }; | ||
| } | ||
|
|
||
| export default function NutritionTable() { | ||
|
|
||
| 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 | ||
| } | ||
|
There was a problem hiding this comment. Maybe this values could live outside of the component .... |
||
| ]) | ||
|
|
||
| const [additionalNutriments, setAdditionalNutriments] = 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 | ||
| } | ||
| ]) | ||
|
|
||
| function onchangeHandler(e) { | ||
| const {value, name} = e.target | ||
| setNutriments(prevState => prevState.map( | ||
| nutr => { | ||
|
There was a problem hiding this comment. 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 |
||
| 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" | ||
| type={'number'} | ||
| label={nutrition.label} | ||
| variant="outlined" | ||
| sx={{ width: "10rem" }} | ||
| value={nutrition.value} | ||
| name={nutrition.label} | ||
| onChange={onchangeHandler} | ||
| /> , <SelectAutoWidth /></Box>, | ||
|
There was a problem hiding this comment. I'd advice against this kind of implicit closing elements ( There was a problem hiding this comment. plase see the coment below There was a problem hiding this comment. I assume it's a mistake. The @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: 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 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={{}} />)); | ||
|
There was a problem hiding this comment. I removed checkbox and leaved delete icon. Still working on it |
||
| }); | ||
|
|
||
| return ( | ||
| <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> | ||
|
|
||
| </TableRow> | ||
| </TableHead> | ||
| <TableBody> | ||
| {rows.map((row) => ( | ||
|
There was a problem hiding this comment. 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> | ||
| <AdditionalNutriments | ||
| options = {additionalNutriments} | ||
| setNutriments={setNutriments} | ||
| setAdditionalNutriments={setAdditionalNutriments} | ||
|
There was a problem hiding this comment. You are trying to keep up to date 2 different arrays which is error pron. As soon as you forget an edge case, the two arrays will not be sync and it's hard to debug I propose to keep the const missingNutriments = offNutriments.filter(nutrimentKey =>
data.every(element) => element.off_nutriment_id !== nutrimentKey)
)with This would replace |
||
| /> | ||
| </Box> | ||
| ); | ||
| } | ||
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.
why is it useful to have this package? I see that it is kind of used in
pages/nutrition/index.jsxbut seems to be importing a non usedflexbox(what would be the advantage of using that flexbox compared to css defined flexbox?)