Issue #19 | Recreate Nav bar using MUI #47
Conversation
|
Visit the preview URL for this PR (updated for commit 0c994bf): https://tcl-70-smart-shopping-list--pr47-gl-km-mui-nav-bar-3geyvllv.web.app (expires Fri, 05 Apr 2024 20:30:33 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 7bb526f3a6607712c7f73ffd6c0dfff4c62b86d9 |
MiliMade
requested review from
lyleschemmerling,
chungleee,
RachaelSkye,
djtaylor8 and
amandaguan-ag
Accidentally added to PR
Accidentally added to PR
Accidentally added to PR
src/components/ListItem.jsx
Outdated
| @@ -69,29 +72,51 @@ export function ListItem({ | |||
| }, [isChecked, itemId, listPath]); | |||
|
|
|||
| const urgency = calculateUrgency(daysUntilNextPurchase, dateLastPurchased); | |||
| const label = { inputProps: { 'aria-label': `${name}` } }; | |||
| const color = | |||
There was a problem hiding this comment.
nested ternarys, even with identation, are problematic. This would be better with an if or a switch
edit: this doesn't look used?
There was a problem hiding this comment.
Hi Lyle! Oops, I was looking at the wrong code. But we fixed this in the other branch for our other PR (Issue#20)! Should fix the issue mentioned in the comment directly below
src/components/ListItem.jsx
Outdated
| <Chip | ||
| label={urgency} | ||
| sx={{ | ||
| backgroundColor: |
src/views/NavBar.jsx
Outdated
| import { NavLink } from 'react-router-dom'; | ||
| import SvgIcon from '@mui/material/SvgIcon'; | ||
|
|
||
| function NavBar() { |
There was a problem hiding this comment.
this is really more of a component than it is a view so I think this should live in ../components
src/views/NavBar.jsx
Outdated
| vertical: 'top', | ||
| horizontal: 'left', | ||
| }} | ||
| open={Boolean(anchorElNav)} |
There was a problem hiding this comment.
See the MDN article for Boolean
Warning: You should rarely find yourself using Boolean as a constructor.
| open={Boolean(anchorElNav)} | |
| open={!!anchorElNav} |
Description
This code adds a NavBar component that uses MUI's app bar component to create a responsive nav bar.
Related Issue
closes #42
Acceptance Criteria
Use the Appbar in MUI to display a Navbar
Implement a hamburger dropdown if Appbar doesnt already do so
Type of Changes
Updates
Before
After
Testing Steps / QA Criteria
git pull origin gl-km-mui-nav-barand check that branch out withgit checkout gl-km-mui-nav-barnpm start