feat(menu): add menu-button with menu-items #124
Conversation
src/scripts/components/menu/menu.tsx
Outdated
| interface MenuItems { | ||
| id: number; | ||
| name: string; | ||
| link: string | null; |
There was a problem hiding this comment.
Should be link?: string so that it can be a string or undefined. In this case it has to be a string or null, which should't work with {id: 3, name: 'Change language'}
src/scripts/components/menu/menu.tsx
Outdated
| import React, {FunctionComponent, useState} from 'react'; | ||
| import styles from './menu.styl'; | ||
|
|
||
| interface MenuItems { |
src/scripts/components/menu/menu.tsx
Outdated
| {id: 4, name: 'Share Content'}, | ||
| {id: 5, name: 'Export Data'}, | ||
| {id: 6, name: 'More Information'} | ||
| ]; |
There was a problem hiding this comment.
We should tell typescript which type this variable is: You can do this with as MenuItem[] before the ;
src/scripts/components/menu/menu.tsx
Outdated
|
|
||
| const [isOpen, setIsOpen] = useState(false); | ||
|
|
||
| const onButtonClickHandler = () => { |
There was a problem hiding this comment.
Easier: () => setIsOpen(!isOpen)
src/scripts/components/menu/menu.tsx
Outdated
| {menuItems.map(menuItem => ( | ||
| <li className={styles.menuListItem} key={menuItem.id}> | ||
| {menuItem.link ? ( | ||
| <a href="{link}">{menuItem.name}</a> |
There was a problem hiding this comment.
{link} without the double quotes
src/scripts/components/menu/menu.tsx
Outdated
| import styles from './menu.styl'; | ||
|
|
||
| interface MenuItems { | ||
| id: number; |
There was a problem hiding this comment.
It's always better to use a string for the id because it's easier to understand when you read it.
| @@ -0,0 +1,27 @@ | |||
| .menuContainer | |||
| position: relative | |||
There was a problem hiding this comment.
I guess it would be better to position the container as absolute and the children as relative? Not sure though, I have to see it in the dev tools :)
No description provided.