Replace Navbar with Sidebar #1167
Conversation
✅ Deploy Preview for harness-design ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for harness-xd-review ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| return ( | ||
| <form {...props}> | ||
| {props.logo} | ||
| <Sidebar.Group className="py-0 -mt-2 px-2"> |
There was a problem hiding this comment.
| <Sidebar.Group className="py-0 -mt-2 px-2"> | |
| <Sidebar.Group className="py-0"> |
-
it seems we can get rid of
-mt-2which breaks the height of the form parent, if we need to get closer to the logo we can just change its current height -
We also do not need to specify
px-2, because initially Sidebar.Group has already set the necessary value for it.
| )} | ||
| </Sidebar.Header> | ||
| <Sidebar.Content> | ||
| <Sidebar.Group className="px-4 pt-5"> |
There was a problem hiding this comment.
| <Sidebar.Group className="px-4 pt-5"> | |
| <Sidebar.Group className="px-2 pt-5"> |
| </Sidebar.Group> | ||
|
|
||
| {!!recentMenuItems.length && ( | ||
| <Sidebar.Group title="Recent" className="border-t px-4 pt-3"> |
There was a problem hiding this comment.
| <Sidebar.Group title="Recent" className="border-t px-4 pt-3"> | |
| <Sidebar.Group title="Recent" className="border-t px-2 pt-3"> |
same
| </Sidebar.Group> | ||
| )} | ||
|
|
||
| <Sidebar.Group className="border-t px-4 pt-5"> |
There was a problem hiding this comment.
| <Sidebar.Group className="border-t px-4 pt-5"> | |
| <Sidebar.Group className="border-t px-2 pt-5"> |
same
| <Sidebar.MenuItem> | ||
| <Sidebar.MenuButton | ||
| asChild | ||
| className="cursor-pointer" |
There was a problem hiding this comment.
| className="cursor-pointer" |
I think we can set this class as the default for this component, as it is interactive in nature, so we don't have to pass it every time.
| <Icon | ||
| name="account" | ||
| size={12} | ||
| className="text-icons-4 transition-colors hover:text-primary !w-[12px] !h-[auto]" |
There was a problem hiding this comment.
| className="text-icons-4 transition-colors hover:text-primary !w-[12px] !h-[auto]" | |
| className="text-icons-4 transition-colors hover:text-primary" |
I would prefer to get rid of the [&>svg]:size-4 class initially in the parent SidebarMenuButton component, as this approach does not work in our case, since we pass the svg through the Icon component to which we specify the required sizes, and in the current situation we have to overwrite each time the value set in the parent, so it does not seem to make any sense
| <NavLink className="block" to={item.to || ''} end> | ||
| {({ isActive }) => ( | ||
| <Sidebar.MenuItem | ||
| className={cn('hover:bg-background-4 rounded text-foreground-3 transition-colors hover:text-primary', { |
There was a problem hiding this comment.
| className={cn('hover:bg-background-4 rounded text-foreground-3 transition-colors hover:text-primary', { | |
| className={cn('hover:bg-background-4 rounded text-foreground-3 transition-colors hover:text-primary', { |
extra space
| <Icon | ||
| name={iconName!} | ||
| size={10} | ||
| className={cn('!w-[12px] !h-[auto]', { |
There was a problem hiding this comment.
| className={cn('!w-[12px] !h-[auto]', { | |
| className={cn( { |
similar story
Replaces the current Navbar with latest Sidebar from Shadcn (https://ui.shadcn.com/blocks/sidebar#sidebar-02)
Sidebar.Changes.mov