feat: Nutrition Game #70 #114
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import * as React from 'react'; | ||
| import TextField from '@mui/material/TextField'; | ||
| import Autocomplete from '@mui/material/Autocomplete'; | ||
|
|
||
| export default function AdditionalNutriments({options, setNutriments, setAdditionalNutriments}) { | ||
|
|
||
| const [inputValue, setInputValue] = React.useState('') | ||
|
|
||
| return ( | ||
| <Autocomplete | ||
| value={null} | ||
| disablePortal | ||
| id="nutrition-input" | ||
| options={options} | ||
| sx={{ width: 245, marginLeft: 2, marginTop: 2 }} | ||
| onChange={(event) => { | ||
| const nutrIndex = event.target.dataset.optionIndex | ||
| if (nutrIndex) { | ||
| setNutriments(prev => [...prev, options[nutrIndex]]) | ||
| setAdditionalNutriments(prev => prev.filter(elem => elem !== options[nutrIndex])) | ||
| setInputValue("") | ||
| } | ||
| }} | ||
| inputValue={inputValue} | ||
| onInputChange={(event, newInputValue) => { | ||
| setInputValue(newInputValue); | ||
| }} | ||
| renderInput={(params) => <TextField {...params} label="Add nutriment" />} | ||
| /> | ||
| ); | ||
| } | ||
|
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,24 @@ | ||||||
| import * as React from "react"; | ||||||
| import NutritionTable from "./table"; | ||||||
| import ProductNutriments from "./productCard"; | ||||||
| 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: { xs: 'center', md:"flex-start" }, | ||||||
| justifyContent: "center", | ||||||
| padding: 4 | ||||||
| }}> | ||||||
| <ProductNutriments /> | ||||||
| <NutritionTable /> | ||||||
| </Box> | ||||||
| ); | ||||||
| } | ||||||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,76 @@ | ||||||
| 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 Typography from '@mui/material/Typography' | ||||||
| import offService from '../../off' | ||||||
| import { useEffect } from "react"; | ||||||
Shmigolk marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
|
||||||
| export default function ProductNutriments({}) { | ||||||
|
|
||||||
| const { getNutritionToFillUrl } = offService | ||||||
|
There was a problem hiding this comment. It works, but writing
Suggested change
|
||||||
|
|
||||||
| const [ page, setPage ] = React.useState(1) | ||||||
| const [ index, setIndex ] = React.useState(0) | ||||||
| const [ products, setProducts ] = React.useState([]) | ||||||
|
|
||||||
| useEffect(() => { | ||||||
| console.log('fetched') | ||||||
| const productListUrl = getNutritionToFillUrl({page}) | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| fetch(productListUrl) | ||||||
| .then(res => res.json()) | ||||||
| .then(data => setProducts(data.products)) | ||||||
|
There was a problem hiding this comment. maybe set the |
||||||
| }, [page]) | ||||||
|
|
||||||
| const pictureURL = products[index] ? products[index].image_nutrition_url : "https://static.openfoodfacts.org/images/image-placeholder.png" | ||||||
|
|
||||||
| function clickHandler(){ | ||||||
| setIndex(prev => prev < 22 ? ++prev : 0) | ||||||
| } | ||||||
|
|
||||||
| return ( | ||||||
|
|
||||||
| <Box flexGrow={1} | ||||||
| flexShrink={1} | ||||||
| display={'flex'} | ||||||
| flexDirection={'column'} | ||||||
| sx={{ | ||||||
| maxWidth: '380px' | ||||||
| }}> | ||||||
| <Typography variant="h5" component="h5" sx={{alignSelf: 'center'}}> | ||||||
| PRODUCT.BRAND | ||||||
| </Typography> | ||||||
| <Typography variant="p" component="p" sx={{alignSelf: 'center'}}> | ||||||
| PRODUCT DESCRIPTION IF NEEDED | ||||||
| </Typography> | ||||||
| <Zoom wrapStyle={{ height: "100%" }}> | ||||||
| <img | ||||||
| // TODO: use getFullSizeImage when the zoom is activated | ||||||
| // src={getFullSizeImage(question.source_image_url)} | ||||||
| src={pictureURL} | ||||||
| alt="" | ||||||
| style={{ maxWidth: "100%", maxHeight: "100%" }} | ||||||
| /> | ||||||
| </Zoom> | ||||||
| <Box display={"flex"} flexDirection={"row"} Gap={'.5em'}> | ||||||
| <Button | ||||||
| onClick={clickHandler} | ||||||
| color="secondary" | ||||||
| variant="contained" | ||||||
| size="large" | ||||||
| sx={{ display: "flex", flexDirection: "column", flexGrow: 1, width: '47%' }} | ||||||
| > | ||||||
| SKIP | ||||||
| </Button> | ||||||
| <Button | ||||||
| onClick={clickHandler} | ||||||
| color="success" | ||||||
| variant="contained" | ||||||
| size="large" | ||||||
| sx={{ display: "flex", flexDirection: "column", flexGrow: 1, width: '47%' }} | ||||||
| > | ||||||
| VALIDATE | ||||||
| </Button> | ||||||
| </Box> | ||||||
| </Box>) | ||||||
| } | ||||||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,149 @@ | ||
| 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 "./unitSelect"; | ||
| import { Box } from "@mui/material"; | ||
| import Checkbox from "@mui/material/Checkbox"; | ||
| import AdditionalNutriments from "./additionalNutritions"; | ||
| import DeleteOutlineIcon from '@mui/icons-material/DeleteOutline'; | ||
|
|
||
| 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 | ||
| } | ||
| ]) | ||
|
|
||
| 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 | ||
| } | ||
| )) | ||
| } | ||
|
|
||
| function deleteItem(nutrition) { | ||
| setNutriments(prev => prev.filter(elem => elem !== nutrition)) | ||
| setAdditionalNutriments(prev => [...prev, nutrition]) | ||
| } | ||
|
|
||
| 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)])
} |
||
| <DeleteOutlineIcon sx={{cursor: 'pointer', color: 'red'}} onClick={() => deleteItem(nutrition)}/>)); | ||
| }); | ||
|
|
||
| return ( | ||
| <Box> | ||
| <TableContainer sx={{ margin: 0, maxWidth: "1000px", width: "340px" }}> | ||
| <Table aria-label="simple table"> | ||
| <TableHead> | ||
| <TableRow> | ||
| <TableCell sx={{ | ||
| maxWidth: "8em", | ||
| fontSize: "large", | ||
| fontWeight: "bold" | ||
| }}>nutrition.table.value</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> | ||
| ); | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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?)