From 7f8038d2b3f27fdfceb2517d23a7acdb609fc1c8 Mon Sep 17 00:00:00 2001
From: Felix Thape <felix.thape@gmail.com>
Date: Tue, 31 Dec 2024 15:06:06 +0100
Subject: [PATCH] feat: add menu hover effects and update styling

---
 landing-page/components/Header.tsx            | 70 ++++++++--------
 lib/components/user-input/Menu.tsx            | 81 +++++++++++++------
 lib/components/user-input/MultiSelect.tsx     | 38 +++++----
 tasks/components/UserMenu.tsx                 | 32 +++-----
 .../layout/property/PropertyList.tsx          |  2 +-
 5 files changed, 118 insertions(+), 105 deletions(-)

diff --git a/landing-page/components/Header.tsx b/landing-page/components/Header.tsx
index 057a17dfe..22ad707da 100644
--- a/landing-page/components/Header.tsx
+++ b/landing-page/components/Header.tsx
@@ -1,7 +1,7 @@
 import { Span } from '@helpwave/common/components/Span'
 import { Helpwave } from '@helpwave/common/icons/Helpwave'
 import { tw } from '@helpwave/common/twind'
-import { Menu as MenuIcon, X } from 'lucide-react'
+import { ChevronDown, Menu as MenuIcon, X } from 'lucide-react'
 import { useState } from 'react'
 import Link from 'next/link'
 import { Menu, MenuItem } from '@helpwave/common/components/user-input/Menu'
@@ -9,6 +9,7 @@ import type { Languages } from '@helpwave/common/hooks/useLanguage'
 import { useTranslation } from '@helpwave/common/hooks/useTranslation'
 import { MarkdownInterpreter } from '@helpwave/common/components/MarkdownInterpreter'
 import { Chip } from '@helpwave/common/components/ChipList'
+import { tx } from '@twind/core'
 
 const homeURL = '/'
 
@@ -107,11 +108,12 @@ const Header = () => {
 
   return (
     <>
-      <div className={tw('absolute flex flex-row justify-center top-0 w-screen z-[50] bg-hw-grayscale-50 mobile:px-6 tablet:px-12 desktop:px-24')}>
+      <div
+        className={tw('absolute flex flex-row justify-center top-0 w-screen z-[50] bg-hw-grayscale-50 mobile:px-6 tablet:px-12 desktop:px-24')}>
         <nav className={tw('flex pt-2 items-center justify-between w-full max-w-[1200px]')}>
           <Link href={homeURL} className={tw('flex flex-row gap-x-1 items-center text-2xl')}>
-            <Helpwave />
-            <MarkdownInterpreter text={'\\helpwave'} />
+            <Helpwave/>
+            <MarkdownInterpreter text={'\\helpwave'}/>
           </Link>
           <div className={tw('mobile:hidden w-full')}>
             <div className={tw('flex flex-wrap items-center justify-end gap-x-6')}>
@@ -122,42 +124,33 @@ const Header = () => {
               }) => (
                 <div key={name}>
                   {subpage === undefined ? (
-                    <Link href={url}>
-                      <Span className={navigationItemStyle}>
-                        {translation[name]}
-                      </Span>
+                    <Link href={url} className={navigationItemStyle}>
+                      {translation[name]}
                     </Link>
                   ) : (
-                    <Menu<HTMLDivElement>
+                    <Menu<HTMLButtonElement>
                       alignment="tl"
                       trigger={(onClick, ref) => (
-                        <div ref={ref} onClick={onClick} className={tw('cursor-pointer select-none')}>
-                          <Span className={navigationItemStyle}>
-                            {translation[name]}
-                          </Span>
-                        </div>
+                        <button ref={ref} onClick={onClick} className={tx('flex flex-row gap-x-1/2 items-center', navigationItemStyle)}>
+                          {translation[name]}
+                          <ChevronDown size={24}/>
+                        </button>
                       )}
                       showOnHover={true}
                     >
-                      {subpage.map(({
-                        name: subPageName,
-                        url: subPageUrl,
-                        external: subPageExternal,
-                      }) =>
+                      {subpage.map(({ name: subPageName, url: subPageUrl, external: subPageExternal }) =>
                         (
-                        <Link key={subPageName} className={tw('cursor-pointer')} href={subPageExternal ? subPageUrl : url + subPageUrl}>
-                          <MenuItem alignment="left">
-                            <Span className={navigationItemStyle}>
+                          <Link key={subPageName} href={subPageExternal ? subPageUrl : url + subPageUrl}>
+                            <MenuItem alignment="left" role="none" className={tw(navigationItemStyle)}>
                               {translation[subPageName]}
-                            </Span>
-                          </MenuItem>
-                        </Link>
+                            </MenuItem>
+                          </Link>
                         ))}
                     </Menu>
                   )}
                 </div>
               ))}
-              <Link href="mailto:contact@helpwave.de">
+              <Link href="mailto:contact@helpwave.de" role="link">
                 <Chip
                   variant="fullyRounded"
                   color="black"
@@ -169,8 +162,8 @@ const Header = () => {
             </div>
           </div>
           <button onClick={() => setNavbarOpen(true)} className={tw('desktop:hidden tablet:hidden content-end')}
-            aria-controls="navbar" aria-expanded="false">
-            <MenuIcon size={32} />
+                  aria-controls="navbar" aria-expanded="false">
+            <MenuIcon size={32}/>
           </button>
         </nav>
       </div>
@@ -179,7 +172,7 @@ const Header = () => {
         <div className={tw('absolute w-screen h-screen z-[100] bg-hw-grayscale-50')}>
           <div className={tw('text-center content-center fixed left-1/2 top-1/2 -translate-x-1/2 -translate-y-1/2')}>
             <button onClick={() => setNavbarOpen(false)} className={tw('mb-5')}>
-              <X size={64} />
+              <X size={64}/>
             </button>
 
             <div className={tw('w-full p-2')}>
@@ -204,10 +197,11 @@ const Header = () => {
                   </Link>
                 ) : (
                   <Menu<HTMLDivElement> alignment="tl" trigger={(onClick, ref) => (
-                    <div ref={ref} onClick={onClick} className={tw('cursor-pointer select-none')}>
+                    <div ref={ref} onClick={onClick} className={tw('cursor-pointer select-none flex flex-row gap-x-2 items-center')}>
                       <Span type="heading">
                         {translation[name]}
                       </Span>
+                      <ChevronDown size={32}/>
                     </div>
                   )}>
                     {subpage.map(({
@@ -216,14 +210,14 @@ const Header = () => {
                       external: subPageExternal
                     }) =>
                       (
-                      <Link key={subPageName} className={tw('cursor-pointer')} onClick={() => setNavbarOpen(false)}
-                        href={subPageExternal ? subPageUrl : url + subPageUrl}>
-                        <MenuItem alignment="left">
-                          <Span type="heading">
-                            {translation[subPageName]}
-                          </Span>
-                        </MenuItem>
-                      </Link>
+                        <Link key={subPageName} className={tw('cursor-pointer')} onClick={() => setNavbarOpen(false)}
+                              href={subPageExternal ? subPageUrl : url + subPageUrl}>
+                          <MenuItem alignment="left">
+                            <Span type="heading">
+                              {translation[subPageName]}
+                            </Span>
+                          </MenuItem>
+                        </Link>
                       ))}
                   </Menu>
                 )}
diff --git a/lib/components/user-input/Menu.tsx b/lib/components/user-input/Menu.tsx
index ca5e38c31..f64295264 100644
--- a/lib/components/user-input/Menu.tsx
+++ b/lib/components/user-input/Menu.tsx
@@ -1,38 +1,70 @@
-import { useRef, useState, type PropsWithChildren, type ReactNode, type RefObject, useEffect } from 'react'
-import { tw, tx } from '../../twind'
+import {
+  useRef,
+  useState,
+  useEffect
+} from 'react'
+import type {
+  AriaAttributes,
+  PropsWithChildren,
+  ReactNode,
+  RefObject, HTMLAttributes
+} from 'react'
+import { apply } from '@twind/core'
+import { tx } from '../../twind'
 import { useOutsideClick } from '../../hooks/useOutsideClick'
 
 type MenuProps<T> = PropsWithChildren<{
+  /**
+   * The builder for the trigger element. Assign the ref and onClick to your element that should be used as a trigger.
+   *
+   * Accessibility: ensure your trigger element is either a button or has the role="button" in addition to a tabIndex={0} or value > 0
+   */
   trigger: (onClick: () => void, ref: RefObject<T>) => ReactNode,
   /**
    * @default 'tl'
    */
   alignment?: 'tl' | 'tr' | 'bl' | 'br' | '_l' | '_r' | 't_' | 'b_',
   showOnHover?: boolean,
-  menuClassName?: string
+  menuClassName?: string,
+  containerClassName?: string
 }>
 
-export type MenuItemProps = {
+export type MenuItemProps = AriaAttributes & Pick<HTMLAttributes<HTMLDivElement>, 'role'> & {
   onClick?: () => void,
   alignment?: 'left' | 'right',
+  isDisabled?: boolean,
   className?: string
 }
+
 const MenuItem = ({
   children,
   onClick,
   alignment = 'left',
-  className
-}: PropsWithChildren<MenuItemProps>) => (
-  <div
-    className={tx('block px-3 py-1 hover:bg-slate-100', {
-      'text-right': alignment === 'right',
-      'text-left': alignment === 'left',
-    }, className)}
-    onClick={onClick}
-  >
-    {children}
-  </div>
-)
+  isDisabled = false,
+  className = '',
+  role = 'menuitem',
+  ...restProps
+}: PropsWithChildren<MenuItemProps>) => {
+  const isClickable = onClick !== undefined
+
+  return (
+    <div
+      className={tx(apply('block px-3 py-1 w-full text-sm leading-6 font-semibold'), {
+        '@(text-right)': alignment === 'right',
+        '@(text-left)': alignment === 'left',
+        '@(cursor-pointer)': isClickable && !isDisabled,
+        '@(cursor-not-allowed)': isClickable && isDisabled,
+        '@(text-hw-grayscale-700 hover:text-hw-grayscale-800 hover:bg-hw-grayscale-50)': !isDisabled,
+        '@(text-hw-grayscale-300)': isDisabled,
+      }, className)}
+      onClick={onClick}
+      role={role}
+      {...restProps}
+    >
+      {children}
+    </div>
+  )
+}
 
 // TODO: it is quite annoying that the type for the ref has to be specified manually, is there some solution around this?
 /**
@@ -44,6 +76,7 @@ const Menu = <T extends HTMLElement>({
   alignment = 'tl',
   showOnHover = false,
   menuClassName = '',
+  containerClassName = '',
 }: MenuProps<T>) => {
   const [open, setOpen] = useState(false)
   const [timer, setTimer] = useState<NodeJS.Timeout>()
@@ -76,19 +109,21 @@ const Menu = <T extends HTMLElement>({
 
   return (
     <div
-      className={tw('relative')}
+      className={tx(apply('relative'), containerClassName)}
       onMouseOver={handleHover}
       onMouseLeave={handleLeaveHover}
     >
       {trigger(() => setOpen(!open), triggerRef)}
       {open ? (
         <div ref={menuRef} onClick={e => e.stopPropagation()}
-             className={tx('absolute top-full mt-1 py-2 w-60 rounded-lg bg-white ring-1 ring-slate-900/5 text-sm leading-6 font-semibold text-slate-700 shadow-md z-[1]', {
-               '    top-[8px]': alignment[0] === 't',
-               ' bottom-[8px]': alignment[0] === 'b',
-               '  left-[-8px]': alignment[1] === 'l',
-               ' right-[-8px]': alignment[1] === 'r',
-             }, menuClassName)}>
+             className={tx(apply('absolute top-full mt-1 py-2 w-60 rounded-lg bg-white ring-1 ring-slate-900/5 shadow-md z-[1]'), {
+               '@(top-[8px])': alignment[0] === 't',
+               '@(bottom-[8px])': alignment[0] === 'b',
+               '@(left-[-8px])': alignment[1] === 'l',
+               '@(right-[-8px])': alignment[1] === 'r',
+             }, menuClassName)}
+             role="menu"
+        >
           {children}
         </div>
       ) : null}
diff --git a/lib/components/user-input/MultiSelect.tsx b/lib/components/user-input/MultiSelect.tsx
index ddc218c00..c3dfabc99 100644
--- a/lib/components/user-input/MultiSelect.tsx
+++ b/lib/components/user-input/MultiSelect.tsx
@@ -100,7 +100,8 @@ export const MultiSelect = <T, >({
   return (
     <div className={tx(className)}>
       {label && (
-        <Label {...label} htmlFor={label.name} className={tx(' mb-1', label.className)} labelType={label.labelType ?? 'labelBig'}/>
+        <Label {...label} htmlFor={label.name} className={tx(' mb-1', label.className)}
+               labelType={label.labelType ?? 'labelBig'}/>
       )}
       <Menu<HTMLDivElement>
         alignment="t_"
@@ -129,25 +130,22 @@ export const MultiSelect = <T, >({
           </div>
         )}
         {filteredOptions.map((option, index) => (
-          <MenuItem key={`item${index}`}>
-            <div
-              className={tx('px-4 py-2 overflow-hidden whitespace-nowrap text-ellipsis flex flex-row gap-x-2',
-                option.className, {
-                  'text-gray-300 cursor-not-allowed': !!option.disabled,
-                  'hover:bg-gray-100 cursor-pointer': !option.disabled,
-                })}
-              onClick={() => {
-                if (!option.disabled) {
-                  onChange(options.map(value => value.value === option.value ? ({
-                    ...option,
-                    selected: !value.selected
-                  }) : value))
-                }
-              }}
-            >
-              <Checkbox checked={option.selected} disabled={option.disabled}/>
-              {option.label}
-            </div>
+          <MenuItem
+            key={`item${index}`}
+            className={tx('px-4 py-2 overflow-hidden whitespace-nowrap text-ellipsis flex flex-row gap-x-2', option.className)}
+            onClick={() => {
+              if (!option.disabled) {
+                onChange(options.map(value => value.value === option.value ? ({
+                  ...option,
+                  selected: !value.selected
+                }) : value))
+              }
+            }}
+            role="menuitemcheckbox"
+            isDisabled={option.disabled}
+          >
+            <Checkbox checked={option.selected} disabled={option.disabled}/>
+            {option.label}
           </MenuItem>
         ))}
       </Menu>
diff --git a/tasks/components/UserMenu.tsx b/tasks/components/UserMenu.tsx
index 0df027c6a..01fd0a72b 100644
--- a/tasks/components/UserMenu.tsx
+++ b/tasks/components/UserMenu.tsx
@@ -1,6 +1,5 @@
 import { useState } from 'react'
 import Link from 'next/link'
-import { useRouter } from 'next/router'
 import { tw } from '@helpwave/common/twind'
 import { Menu, MenuItem } from '@helpwave/common/components/user-input/Menu'
 import { LanguageModal } from '@helpwave/common/components/modals/LanguageModal'
@@ -52,7 +51,6 @@ export const UserMenu = ({
   const translation = useTranslation(defaultUserMenuTranslations, overwriteTranslation)
   const [isLanguageModalOpen, setLanguageModalOpen] = useState(false)
   const { user, signOut } = useAuth()
-  const router = useRouter()
 
   if (!user) return null
 
@@ -75,27 +73,15 @@ export const UserMenu = ({
           <Avatar avatarUrl={user.avatarUrl} alt={user.email} size="small"/>
         </div>
       )}>
-        <Link href={settingsURL} target="_blank"><MenuItem alignment="left">{translation.profile}</MenuItem></Link>
-        <div className="cursor-pointer" onClick={() => setLanguageModalOpen(true)}><MenuItem
-          alignment="left">{translation.language}</MenuItem></div>
-        <div className={tw('cursor-pointer')} onClick={() => router.push('/templates')}>
-          <MenuItem alignment="left">{translation.taskTemplates}</MenuItem>
-        </div>
-        <div className={tw('cursor-pointer')} onClick={() => router.push('/properties')}>
-          <MenuItem alignment="left">{translation.properties}</MenuItem>
-        </div>
-        <div className={tw('cursor-pointer')} onClick={() => router.push('/organizations')}>
-          <MenuItem alignment="left">{translation.organizations}</MenuItem>
-        </div>
-        <div className={tw('cursor-pointer')} onClick={() => router.push('/invitations')}>
-          <MenuItem alignment="left">{translation.invitations}</MenuItem>
-        </div>
-        <div
-          className={tw('cursor-pointer text-hw-negative-400 hover:text-hw-negative-500')}
-          onClick={() => signOut()}
-        >
-          <MenuItem alignment="left">{translation.signOut}</MenuItem>
-        </div>
+        <Link href={settingsURL} target="_blank"><MenuItem role="none">{translation.profile}</MenuItem></Link>
+        <MenuItem onClick={() => setLanguageModalOpen(true)}>{translation.language}</MenuItem>
+        <Link href="/templates"><MenuItem role="none">{translation.taskTemplates}</MenuItem></Link>
+        <Link href="/properties"><MenuItem role="none">{translation.properties}</MenuItem></Link>
+        <Link href="/organizations"><MenuItem role="none">{translation.organizations}</MenuItem></Link>
+        <Link href="/invitations"><MenuItem role="none">{translation.invitations}</MenuItem></Link>
+        <MenuItem className={tw('text-hw-negative-400 hover:text-hw-negative-500')} onClick={() => signOut()}>
+          {translation.signOut}
+        </MenuItem>
       </Menu>
     </div>
   )
diff --git a/tasks/components/layout/property/PropertyList.tsx b/tasks/components/layout/property/PropertyList.tsx
index 4010a015c..22271a998 100644
--- a/tasks/components/layout/property/PropertyList.tsx
+++ b/tasks/components/layout/property/PropertyList.tsx
@@ -127,7 +127,7 @@ export const PropertyList = ({
                       addOrUpdatePropertyMutation.mutate({ previous: attachedProperty, update: attachedProperty, fieldType: property.fieldType })
                       updateViewRulesMutation.mutate({ subjectId, appendToAlwaysInclude: [property.id] })
                     }}
-                    className={tw('rounded-md cursor-pointer')}
+                    className={tw('rounded-md')}
                   >
                     {property.name}
                   </MenuItem>