feat: add side navigation #154
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| flex: 1; | ||
| line-height: 1.5; | ||
| margin-inline-start: 4px; | ||
| padding-block: 0.75rem; |
There was a problem hiding this comment.
als we block en inline beide zetten kunnen we beter de padding shorthand gebruiken
There was a problem hiding this comment.
Als de stylelint in te stellen is om non-logical-shorthands zoals padding alleen toe te staan met 1 length, dan is het okay.
Anders is het handiger om padding volledig te blokkeren.
| display: flex; | ||
| flex: 1; | ||
| line-height: 1.5; | ||
| margin-inline-start: 4px; |
There was a problem hiding this comment.
voelt als een magic number, zou het moeten corresponderen met de witruimte binnen het icon voor 'Verberg'? die lijkt 2px te zijn?
| {props.label} | ||
| </Link> | ||
| {props.children && ( | ||
| <Button appearance="subtle-button" className="kernteam-menu-list__icon-button" onClick={() => setOpen(!open)}> |
There was a problem hiding this comment.
hier kunnen we het beste de expanded state exposen:
| <Button appearance="subtle-button" className="kernteam-menu-list__icon-button" onClick={() => setOpen(!open)}> | |
| <Button appearance="subtle-button" aria-expanded={open} className="kernteam-menu-list__icon-button" onClick={() => setOpen(!open)}> |
| > | ||
| {open ? ( | ||
| <> | ||
| <IconArrowBarToLeft /> |
There was a problem hiding this comment.
laten we het icon visueel verberg en als presentational markeren
There was a problem hiding this comment.
Dat kan met de Icon component van Utrecht
| ); | ||
| }; | ||
|
|
||
| export const MenuList = ({ children, ...restProps }: PropsWithChildren) => { |
There was a problem hiding this comment.
hier NavList ipv MenuList gebruiken om verwarring met de menu role of het <menu> HTML element te voorkomen?
| children?: MenuListItemProps[]; | ||
| } | ||
|
|
||
| export const MenuListItem = (props: MenuListItemProps) => { |
There was a problem hiding this comment.
same als hierboven maar NavListItem
|
Paar side notes: