From 64071747e874a0dc55c8601dc20e8bda00315909 Mon Sep 17 00:00:00 2001 From: Jordan Wong <42422209+JorWo@users.noreply.github.com> Date: Fri, 9 Aug 2024 16:51:31 -1000 Subject: [PATCH] Improve Navbar for smaller screens (#50) --- .../layout/navbar/mobile-navbar.tsx | 59 ------- .../navbar/{nav-links.ts => navbar-links.ts} | 2 +- .../layout/navbar/navbar-menu-icon.tsx | 21 +++ .../components/layout/navbar/navbar-menu.tsx | 39 +++++ ui/src/components/layout/navbar/navbar.tsx | 21 +-- ui/src/components/ui/sheet.tsx | 154 ++++++++---------- ...e-navbar.test.tsx => navbar-menu.test.tsx} | 18 +- 7 files changed, 146 insertions(+), 168 deletions(-) delete mode 100644 ui/src/components/layout/navbar/mobile-navbar.tsx rename ui/src/components/layout/navbar/{nav-links.ts => navbar-links.ts} (94%) create mode 100644 ui/src/components/layout/navbar/navbar-menu-icon.tsx create mode 100644 ui/src/components/layout/navbar/navbar-menu.tsx rename ui/tests/app/_components/navbar/{mobile-navbar.test.tsx => navbar-menu.test.tsx} (87%) diff --git a/ui/src/components/layout/navbar/mobile-navbar.tsx b/ui/src/components/layout/navbar/mobile-navbar.tsx deleted file mode 100644 index 333d5c08..00000000 --- a/ui/src/components/layout/navbar/mobile-navbar.tsx +++ /dev/null @@ -1,59 +0,0 @@ -'use client'; - -import { - Sheet, - SheetContent, - SheetTrigger, -} from '@/components/ui/sheet' -import User from '@/access/user'; -import Link from 'next/link'; -import { NavLinks } from './nav-links'; -import { useState } from 'react'; -import Role from '@/access/role'; - -const MobileNavbar = ({ - currentUser -} : { - currentUser: User -}) => { - const [isOpen, setIsOpen] = useState(false); - const handleClick = () => { - setIsOpen(!isOpen); - }; - - return ( - - -
- - - -
- - - - - - - ); -}; - - -export default MobileNavbar; diff --git a/ui/src/components/layout/navbar/nav-links.ts b/ui/src/components/layout/navbar/navbar-links.ts similarity index 94% rename from ui/src/components/layout/navbar/nav-links.ts rename to ui/src/components/layout/navbar/navbar-links.ts index 63fb9bdb..de2c535f 100644 --- a/ui/src/components/layout/navbar/nav-links.ts +++ b/ui/src/components/layout/navbar/navbar-links.ts @@ -1,6 +1,6 @@ import Role from '@/access/role'; -export const NavLinks = [ +export const NavbarLinks = [ { name: 'Admin', link: '/admin', diff --git a/ui/src/components/layout/navbar/navbar-menu-icon.tsx b/ui/src/components/layout/navbar/navbar-menu-icon.tsx new file mode 100644 index 00000000..ceb694c2 --- /dev/null +++ b/ui/src/components/layout/navbar/navbar-menu-icon.tsx @@ -0,0 +1,21 @@ +const NavbarMenuIcon = ({ open }: { open: boolean }) => ( +
+ + + +
+); + +export default NavbarMenuIcon; diff --git a/ui/src/components/layout/navbar/navbar-menu.tsx b/ui/src/components/layout/navbar/navbar-menu.tsx new file mode 100644 index 00000000..8066d4b7 --- /dev/null +++ b/ui/src/components/layout/navbar/navbar-menu.tsx @@ -0,0 +1,39 @@ +'use client'; + +import { Sheet, SheetContent, SheetTrigger } from '@/components/ui/sheet'; +import User from '@/access/user'; +import Link from 'next/link'; +import { NavbarLinks } from './navbar-links'; +import { useState } from 'react'; +import Role from '@/access/role'; +import NavbarMenuIcon from './navbar-menu-icon'; + +const NavbarMenu = ({ currentUser }: { currentUser: User }) => { + const [open, setOpen] = useState(false); + const handleClick = () => { + setOpen(!open); + }; + + return ( + + + + + + + + + ); +}; + +export default NavbarMenu; diff --git a/ui/src/components/layout/navbar/navbar.tsx b/ui/src/components/layout/navbar/navbar.tsx index a386ee02..0c1b4579 100644 --- a/ui/src/components/layout/navbar/navbar.tsx +++ b/ui/src/components/layout/navbar/navbar.tsx @@ -2,9 +2,9 @@ import Link from 'next/link'; import Image from 'next/image'; import Role from '@/access/role'; import { getCurrentUser } from '@/access/authentication'; -import { NavLinks } from './nav-links'; +import { NavbarLinks } from './navbar-links'; import LoginButton from './login-button'; -import MobileNavbar from './mobile-navbar'; +import NavbarMenu from './navbar-menu'; import TimeoutModal from '@/components/modal/timeout-modal'; const Navbar = async () => { @@ -24,7 +24,7 @@ const Navbar = async () => { />
- + {
- {NavLinks - .filter((navLink) => - currentUser.roles.includes(Role.ADMIN) || currentUser.roles.includes(navLink.role)) - .map((navLink) => + {NavbarLinks + .filter((navbarLink) => + currentUser.roles.includes(Role.ADMIN) || + currentUser.roles.includes(navbarLink.role)) + .map((navbarLink) => - {navLink.name} + {navbarLink.name} )}
diff --git a/ui/src/components/ui/sheet.tsx b/ui/src/components/ui/sheet.tsx index da7b8731..721fd760 100644 --- a/ui/src/components/ui/sheet.tsx +++ b/ui/src/components/ui/sheet.tsx @@ -9,26 +9,20 @@ import { cn } from '@/components/ui/utils'; const Sheet = SheetPrimitive.Root; -const SheetClose = SheetPrimitive.Close +const SheetClose = SheetPrimitive.Close; -const SheetPortal = SheetPrimitive.Portal +const SheetPortal = SheetPrimitive.Portal; -interface SheetTriggerProps - extends React.ComponentPropsWithoutRef { - onClick?: () => void +interface SheetTriggerProps extends React.ComponentPropsWithoutRef { + onClick?: () => void; } -const SheetTrigger = React.forwardRef< - React.ElementRef, - SheetTriggerProps ->(({ onClick, className, children, ...props }, ref) => ( - - {children} - -)); +const SheetTrigger = React.forwardRef, SheetTriggerProps>( + ({ onClick, className, children, ...props }, ref) => ( + + {children} + + ) +); SheetTrigger.displayName = SheetPrimitive.Trigger.displayName; const SheetOverlay = React.forwardRef< @@ -44,11 +38,11 @@ const SheetOverlay = React.forwardRef< {...props} ref={ref} /> -)) -SheetOverlay.displayName = SheetPrimitive.Overlay.displayName +)); +SheetOverlay.displayName = SheetPrimitive.Overlay.displayName; const sheetVariants = cva( - `fixed z-50 gap-4 bg-white p-6 shadow-lg transition ease-in-out data-[state=open]:animate-in + `fixed z-40 gap-4 bg-white py-6 shadow-lg transition ease-in-out data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:duration-300 data-[state=open]:duration-500 dark:bg-slate-950`, { @@ -61,80 +55,62 @@ const sheetVariants = cva( left: `inset-y-0 left-0 h-full w-3/4 border-r data-[state=closed]:slide-out-to-left data-[state=open]:slide-in-from-left sm:max-w-sm`, right: `inset-y-0 right-0 h-full w-3/4 border-l data-[state=closed]:slide-out-to-right - data-[state=open]:slide-in-from-right sm:max-w-sm`, + data-[state=open]:slide-in-from-right sm:max-w-sm` } }, defaultVariants: { - side: 'right', - }, + side: 'right' + } } -) +); interface SheetContentProps extends React.ComponentPropsWithoutRef, - VariantProps { - onClickOutside?: () => void, - hasCloseButton?: boolean + VariantProps { + onClickOutside?: () => void; + hasCloseButton?: boolean; } -const SheetContent = React.forwardRef< - React.ElementRef, - SheetContentProps ->(({ side = 'right', hasCloseButton = false, onClickOutside, className, children, ...props }, ref) => ( - - - { - e.preventDefault(); - }} - > - {children} - - {hasCloseButton && ( - , SheetContentProps>( + ({ side = 'right', hasCloseButton = false, onClickOutside, className, children, ...props }, ref) => ( + + + { + e.preventDefault(); + }} + > + {children} + + {hasCloseButton && ( + - - Close - - )} - - -)) -SheetContent.displayName = SheetPrimitive.Content.displayName - -const SheetHeader = ({ - className, - ...props -}: React.HTMLAttributes) => ( -
-) -SheetHeader.displayName = 'SheetHeader' - -const SheetFooter = ({ - className, - ...props -}: React.HTMLAttributes) => ( -
-) -SheetFooter.displayName = 'SheetFooter' + dark:focus:ring-slate-300 dark:data-[state=open]:bg-slate-800`} + > + + Close + + )} + + + ) +); +SheetContent.displayName = SheetPrimitive.Content.displayName; + +const SheetHeader = ({ className, ...props }: React.HTMLAttributes) => ( +
+); +SheetHeader.displayName = 'SheetHeader'; + +const SheetFooter = ({ className, ...props }: React.HTMLAttributes) => ( +
+); +SheetFooter.displayName = 'SheetFooter'; const SheetTitle = React.forwardRef< React.ElementRef, @@ -145,8 +121,8 @@ const SheetTitle = React.forwardRef< className={cn('text-lg font-semibold text-slate-950 dark:text-slate-50', className)} {...props} /> -)) -SheetTitle.displayName = SheetPrimitive.Title.displayName +)); +SheetTitle.displayName = SheetPrimitive.Title.displayName; const SheetDescription = React.forwardRef< React.ElementRef, @@ -157,8 +133,8 @@ const SheetDescription = React.forwardRef< className={cn('text-sm text-slate-500 dark:text-slate-400', className)} {...props} /> -)) -SheetDescription.displayName = SheetPrimitive.Description.displayName +)); +SheetDescription.displayName = SheetPrimitive.Description.displayName; export { Sheet, @@ -170,5 +146,5 @@ export { SheetHeader, SheetFooter, SheetTitle, - SheetDescription, -} + SheetDescription +}; diff --git a/ui/tests/app/_components/navbar/mobile-navbar.test.tsx b/ui/tests/app/_components/navbar/navbar-menu.test.tsx similarity index 87% rename from ui/tests/app/_components/navbar/mobile-navbar.test.tsx rename to ui/tests/app/_components/navbar/navbar-menu.test.tsx index 35b81230..c8011b96 100644 --- a/ui/tests/app/_components/navbar/mobile-navbar.test.tsx +++ b/ui/tests/app/_components/navbar/navbar-menu.test.tsx @@ -1,19 +1,19 @@ import Role from '@/access/role'; import User, { AnonymousUser } from '@/access/user'; -import MobileNavbar from '@/components/layout/navbar/mobile-navbar'; +import NavbarMenu from '@/components/layout/navbar/navbar-menu'; import { fireEvent, render, screen } from '@testing-library/react'; const testUser: User = JSON.parse(process.env.TEST_USER_A as string); -describe('MobileNavbar', () => { - it('should render the MobileNavbar with the sheet closed', () => { - render(); +describe('NavbarMenu', () => { + it('should render the NavbarMenu with the sheet closed', () => { + render(); expect(screen.queryByRole('navigation')).not.toBeInTheDocument(); }); it('should open the drawer on click', () => { - render(); + render(); fireEvent.click(screen.getByRole('button', { name: 'Open navigation menu' })); expect(screen.getByRole('navigation')).toBeInTheDocument(); @@ -21,7 +21,7 @@ describe('MobileNavbar', () => { describe('User is logged-out', () => { it('should render the navbar with only the link to /about', () => { - render(); + render(); fireEvent.click(screen.getByRole('button', { name: 'Open navigation menu' })); expect(screen.getByRole('navigation')).toBeInTheDocument(); @@ -40,7 +40,7 @@ describe('MobileNavbar', () => { it('should render only /memberships, /about, /feedback for the average user', () => { testUser.roles.push(Role.UH); - render(); + render(); fireEvent.click(screen.getByRole('button', { name: 'Open navigation menu' })); expect(screen.getByRole('navigation')).toBeInTheDocument(); @@ -53,7 +53,7 @@ describe('MobileNavbar', () => { it('should render only /memberships, /groupings, /about, /feedback for an owner of a grouping', () => { testUser.roles.push(Role.OWNER, Role.UH); - render(); + render(); fireEvent.click(screen.getByRole('button', { name: 'Open navigation menu' })); expect(screen.getByRole('navigation')).toBeInTheDocument(); @@ -66,7 +66,7 @@ describe('MobileNavbar', () => { it('should render all links for an Admin', () => { testUser.roles.push(Role.ADMIN, Role.UH); - render(); + render(); fireEvent.click(screen.getByRole('button', { name: 'Open navigation menu' })); expect(screen.getByRole('navigation')).toBeInTheDocument();