Feat/left menu #30
Feat/left menu #30
Conversation
| import { Separator } from "../ui/separator"; | ||
|
|
||
| type MenuItems = { | ||
| route: string; |
There was a problem hiding this comment.
I believe it would be nice to have a onClick here and I don't think the route is necessary
| const items: MenuItems[] = [ | ||
| { | ||
| route: "", | ||
| text: "Dashboard", |
There was a problem hiding this comment.
can you do a setup for i18n and put the strings there?
There was a problem hiding this comment.
it's necessary do this is the actual sprint?
It is not better to do when we have more time?
There was a problem hiding this comment.
it's easier to start using it from the beginning.
| const LateralMenu = (): JSX.Element => { | ||
| return ( | ||
| <NavigationMenu | ||
| className="sticky bg-gray-700 z-50 pt-6 flex-col" |
There was a problem hiding this comment.
why are you changing the z-index?
There was a problem hiding this comment.
because this component will be always be presented on top of others components
There was a problem hiding this comment.
i don't see this as necessary, it will just create bugs
There was a problem hiding this comment.
This is not our case, this side menu will not cover any other components
rfgvieira
force-pushed
the
feat/left-menu
branch
2 times, most recently
from
886643b to
904cbdd
Compare
| @@ -7,6 +7,12 @@ Currently, two official plugins are available: | |||
| - [@vitejs/plugin-react](https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-react/README.md) uses [Babel](https://babeljs.io/) for Fast Refresh | |||
| - [@vitejs/plugin-react-swc](https://github.com/vitejs/vite-plugin-react-swc) uses [SWC](https://swc.rs/) for Fast Refresh | |||
|
|
|||
| ## Add component | |||
| To add a new component to projet use the following command on terminal: | |||
There was a problem hiding this comment.
Nit: It would be nice to put a link to the docs https://ui.shadcn.com/docs/cli
dashboard/package.json
Outdated
| "vite": "^5.3.1", | ||
| "@vitejs/plugin-react": "^4.3.1" |
There was a problem hiding this comment.
Why you moved those from devDependencies to dependencies?
There was a problem hiding this comment.
Because when running pnpm build command there is a warning to move this two dependencies to dependencies
| const items: MenuItems[] = [ | ||
| { | ||
| route: "", | ||
| text: "Dashboard", |
There was a problem hiding this comment.
it's easier to start using it from the beginning.
rfgvieira
force-pushed
the
feat/left-menu
branch
3 times, most recently
from
73ab1a5 to
e1923ce
Compare
rfgvieira
force-pushed
the
feat/left-menu
branch
from
e1923ce to
18ad762
Compare
There was a problem hiding this comment.
IMO: this change may have its own commit
| </NavigationMenuLink> | ||
| ); | ||
|
|
||
| const LateralMenu = (): JSX.Element => { |
| onClick: () => void; | ||
| text: string; | ||
| icon: JSX.Element; | ||
| selected: boolean; |
There was a problem hiding this comment.
I think LateralMenu should control who is selected and not the item
There was a problem hiding this comment.
I plan to change this when we have setup Routes file
| }, | ||
| { | ||
| onClick: emptyFunc, | ||
| text: "Tree Monitor", |
There was a problem hiding this comment.
Nit: It is more common to use label in this case, text is more like a larger chunk of words
|
I thought we were not supposed to style to look like the figma design and use the base styles of the library as much as possible |
rfgvieira
force-pushed
the
feat/left-menu
branch
2 times, most recently
from
4ef873a to
9a0a51d
Compare
rfgvieira
force-pushed
the
feat/left-menu
branch
from
9a0a51d to
bb0f90d
Compare
Description
Related Issues
Visual reference