feat(Menu): set new default for menu trigger (#1965)

- use primary button with trailing icon - update stories to reflect change - also update button handling to prevent UI breaking on long text - align menu item hover whether using keyboard or mouse
chanzuckerberg · May 24, 2024 · 24815c2 · 24815c2
1 parent 109a0e5
commit 24815c2
Show file tree

Hide file tree

Showing 7 changed files with 85 additions and 58 deletions.
diff --git a/src/components/Button/Button-v2.module.css b/src/components/Button/Button-v2.module.css
@@ -21,6 +21,10 @@
   align-items: center;
   justify-content: center;
 
+  white-space: nowrap;
+  overflow: hidden;
+  text-overflow: ellipsis;
+
   width: 100%;
 
   .button--sm & {
@@ -33,6 +37,10 @@
   .button--lg & {
     gap: 0.5rem;
   }
+
+  & svg {
+    flex-shrink: 0;
+  }
 }
 
 .button__text.button--is-loading {

diff --git a/src/components/Menu/Menu-v2.module.css b/src/components/Menu/Menu-v2.module.css
@@ -25,6 +25,7 @@
 }
 
 .menu__item {
+  display: block;
   text-decoration: none;
   color: inherit;
 }

diff --git a/src/components/Menu/Menu-v2.stories.tsx b/src/components/Menu/Menu-v2.stories.tsx
@@ -62,6 +62,9 @@ export const Default: StoryObj<MenuProps> = {
   },
 };
 
+/**
+ * Be careful when using a lot of text for a menu trigger. The UI will force the text out of the button container. Use brief text for menu triggers.
+ */
 export const WithLongButtonText: StoryObj<MenuProps> = {
   args: {
     children: (

diff --git a/src/components/Menu/Menu-v2.tsx b/src/components/Menu/Menu-v2.tsx
@@ -8,8 +8,8 @@ import { usePopper } from 'react-popper';
 
 import type { ExtractProps } from '../../util/utility-types';
 
-import Button from '../Button';
-import { IconV2 as Icon, type IconNameV2 as IconName } from '../Icon';
+import { ButtonV2 as Button } from '../Button';
+import { type IconNameV2 as IconName } from '../Icon';
 
 import {
   PopoverContainerV2 as PopoverContainer,
@@ -54,7 +54,7 @@ export type MenuButtonProps = {
   /**
    * The button contents placed left of the chevron icon.
    */
-  children: ReactNode;
+  children: string;
   /**
    * Allow custom classes to be applied to the menu button.
    */
@@ -126,14 +126,14 @@ const MenuButton = ({
 
   return (
     <HeadlessMenu.Button as={React.Fragment} ref={setReferenceElement}>
-      <Button className={buttonClassNames} status="neutral" {...other}>
+      <Button
+        className={buttonClassNames}
+        icon={icon}
+        iconLayout="right"
+        rank="primary"
+        {...other}
+      >
         {children}
-        <Icon
-          className={styles['menu__button--with-chevron']}
-          name={icon}
-          purpose="decorative"
-          size="1.25rem"
-        />
       </Button>
     </HeadlessMenu.Button>
   );

diff --git a/src/components/Menu/__snapshots__/Menu-v2.test.tsx.snap b/src/components/Menu/__snapshots__/Menu-v2.test.tsx.snap
@@ -9,26 +9,30 @@ exports[`<Menu /> Default story renders snapshot 1`] = `
     aria-controls="headlessui-menu-items-:r1:"
     aria-expanded="true"
     aria-haspopup="menu"
-    class="button button--lg button--secondary button--neutral menu__button"
+    class="button button--layout-right button--primary button--lg button--variant-default menu__button"
     data-headlessui-state="open"
     id="headlessui-menu-button-:r0:"
     type="button"
   >
-    Documentation Links
-    <svg
-      aria-hidden="true"
-      class="icon menu__button--with-chevron"
-      fill="currentColor"
-      height="1.25rem"
-      style="--icon-size: 1.25rem;"
-      viewBox="0 0 24 24"
-      width="1.25rem"
-      xmlns="http://www.w3.org/2000/svg"
+    <span
+      class="button__text"
     >
-      <path
-        d="M15.88 9.29L12 13.17 8.12 9.29c-.39-.39-1.02-.39-1.41 0-.39.39-.39 1.02 0 1.41l4.59 4.59c.39.39 1.02.39 1.41 0l4.59-4.59c.39-.39.39-1.02 0-1.41-.39-.38-1.03-.39-1.42 0z"
-      />
-    </svg>
+      Documentation Links
+      <svg
+        aria-hidden="true"
+        class="icon"
+        fill="currentColor"
+        height="1.5rem"
+        style="--icon-size: 1.5rem;"
+        viewBox="0 0 24 24"
+        width="1.5rem"
+        xmlns="http://www.w3.org/2000/svg"
+      >
+        <path
+          d="M15.88 9.29L12 13.17 8.12 9.29c-.39-.39-1.02-.39-1.41 0-.39.39-.39 1.02 0 1.41l4.59 4.59c.39.39 1.02.39 1.41 0l4.59-4.59c.39-.39.39-1.02 0-1.41-.39-.38-1.03-.39-1.42 0z"
+        />
+      </svg>
+    </span>
   </button>
 </div>
 `;
@@ -125,26 +129,30 @@ exports[`<Menu /> WithLongButtonText story renders snapshot 1`] = `
     aria-controls="headlessui-menu-items-:r7:"
     aria-expanded="true"
     aria-haspopup="menu"
-    class="button button--lg button--secondary button--neutral menu__button"
+    class="button button--layout-right button--primary button--lg button--variant-default menu__button"
     data-headlessui-state="open"
     id="headlessui-menu-button-:r6:"
     type="button"
   >
-    Long Trigger Button Text to Demonstrate Popover Matching
-    <svg
-      aria-hidden="true"
-      class="icon menu__button--with-chevron"
-      fill="currentColor"
-      height="1.25rem"
-      style="--icon-size: 1.25rem;"
-      viewBox="0 0 24 24"
-      width="1.25rem"
-      xmlns="http://www.w3.org/2000/svg"
+    <span
+      class="button__text"
     >
-      <path
-        d="M15.88 9.29L12 13.17 8.12 9.29c-.39-.39-1.02-.39-1.41 0-.39.39-.39 1.02 0 1.41l4.59 4.59c.39.39 1.02.39 1.41 0l4.59-4.59c.39-.39.39-1.02 0-1.41-.39-.38-1.03-.39-1.42 0z"
-      />
-    </svg>
+      Long Trigger Button Text to Demonstrate Popover Matching
+      <svg
+        aria-hidden="true"
+        class="icon"
+        fill="currentColor"
+        height="1.5rem"
+        style="--icon-size: 1.5rem;"
+        viewBox="0 0 24 24"
+        width="1.5rem"
+        xmlns="http://www.w3.org/2000/svg"
+      >
+        <path
+          d="M15.88 9.29L12 13.17 8.12 9.29c-.39-.39-1.02-.39-1.41 0-.39.39-.39 1.02 0 1.41l4.59 4.59c.39.39 1.02.39 1.41 0l4.59-4.59c.39-.39.39-1.02 0-1.41-.39-.38-1.03-.39-1.42 0z"
+        />
+      </svg>
+    </span>
   </button>
 </div>
 `;
@@ -158,26 +166,30 @@ exports[`<Menu /> WithShortButtonText story renders snapshot 1`] = `
     aria-controls="headlessui-menu-items-:rd:"
     aria-expanded="true"
     aria-haspopup="menu"
-    class="button button--lg button--secondary button--neutral menu__button"
+    class="button button--layout-right button--primary button--lg button--variant-default menu__button"
     data-headlessui-state="open"
     id="headlessui-menu-button-:rc:"
     type="button"
   >
-    Menu
-    <svg
-      aria-hidden="true"
-      class="icon menu__button--with-chevron"
-      fill="currentColor"
-      height="1.25rem"
-      style="--icon-size: 1.25rem;"
-      viewBox="0 0 24 24"
-      width="1.25rem"
-      xmlns="http://www.w3.org/2000/svg"
+    <span
+      class="button__text"
     >
-      <path
-        d="M15.88 9.29L12 13.17 8.12 9.29c-.39-.39-1.02-.39-1.41 0-.39.39-.39 1.02 0 1.41l4.59 4.59c.39.39 1.02.39 1.41 0l4.59-4.59c.39-.39.39-1.02 0-1.41-.39-.38-1.03-.39-1.42 0z"
-      />
-    </svg>
+      Menu
+      <svg
+        aria-hidden="true"
+        class="icon"
+        fill="currentColor"
+        height="1.5rem"
+        style="--icon-size: 1.5rem;"
+        viewBox="0 0 24 24"
+        width="1.5rem"
+        xmlns="http://www.w3.org/2000/svg"
+      >
+        <path
+          d="M15.88 9.29L12 13.17 8.12 9.29c-.39-.39-1.02-.39-1.41 0-.39.39-.39 1.02 0 1.41l4.59 4.59c.39.39 1.02.39 1.41 0l4.59-4.59c.39-.39.39-1.02 0-1.41-.39-.38-1.03-.39-1.42 0z"
+        />
+      </svg>
+    </span>
   </button>
 </div>
 `;
diff --git a/src/components/PopoverContainer/PopoverContainer-v2.module.css b/src/components/PopoverContainer/PopoverContainer-v2.module.css
@@ -15,6 +15,10 @@
 
   box-shadow: var(--eds-box-shadow-md);
   background-color: var(--eds-theme-color-background-utility-container);
+
+  &:focus-visible {
+    outline: none;
+  }
 }
 
 .popover-container > *[role=none] + *[role=none] {

diff --git a/src/components/PopoverListItem/PopoverListItem-v2.module.css b/src/components/PopoverListItem/PopoverListItem-v2.module.css
@@ -27,9 +27,8 @@
   }
 }
 
-.popover-list-item--focused, 
-.popover-list-item:focus-visible {
-  box-shadow: 0 0 0 2px var(--eds-theme-color-border-utility-focus);
+.popover-list-item--focused {
+  background-color: var(--eds-theme-color-background-utility-default-no-emphasis-hover);
 }
 
 .popover-list-item--disabled {
-Original file line number
+Diff line change
@@ Expand Up / @@ -62,6 +62,9 @@ export const Default: StoryObj<MenuProps> = { @@
       },
     };
+    /**
+     * Be careful when using a lot of text for a menu trigger. The UI will force the text out of the button container. Use brief text for menu triggers.
+     */
     export const WithLongButtonText: StoryObj<MenuProps> = {
       args: {
         children: (
@@ Expand Down @@