feat: refactor Menu to extract trigger prop (#536)

This is a breaking change, but we are in alpha so no major version bump  ## Summary by CodeRabbit - **New Features** - Updated the Menu component to use a ButtonIcon as a trigger across various components. - Added a new property `noInternalClick` to control internal click behavior in buttons for Safari compatibility. - **Refactor** - Refactored the Menu component to handle triggers and labels more efficiently. - Replaced the ButtonIcon with a standard button in the MenuItem component, adjusting props accordingly. - **Documentation** - Updated story files to reflect changes in Menu and Button components.
versini-org · Apr 27, 2024 · 1818315 · 1818315
1 parent a1f4eae
commit 1818315
Show file tree

Hide file tree

Showing 13 changed files with 276 additions and 54 deletions.
diff --git a/examples/with-vite/src/components/CustomCard.tsx b/examples/with-vite/src/components/CustomCard.tsx
@@ -1,4 +1,10 @@
-import { Card, Menu, MenuItem, Panel } from "@versini/ui-components";
+import {
+	ButtonIcon,
+	Card,
+	Menu,
+	MenuItem,
+	Panel,
+} from "@versini/ui-components";
 import {
 	IconChart,
 	IconHistory,
@@ -27,7 +33,13 @@ export const CustomCard = () => {
 							<h2 className="m-0 mb-1">Kitchen Sink</h2>
 						</FlexgridItem>
 						<FlexgridItem>
-							<Menu icon={<IconSettings />}>
+							<Menu
+								trigger={
+									<ButtonIcon>
+										<IconSettings />
+									</ButtonIcon>
+								}
+							>
 								<MenuItem
 									label="Profile"
 									icon={<IconProfile />}

diff --git a/examples/with-webpack/src/components/CustomCard.tsx b/examples/with-webpack/src/components/CustomCard.tsx
@@ -1,4 +1,10 @@
-import { Card, Menu, MenuItem, Panel } from "@versini/ui-components";
+import {
+	ButtonIcon,
+	Card,
+	Menu,
+	MenuItem,
+	Panel,
+} from "@versini/ui-components";
 import {
 	IconChart,
 	IconHistory,
@@ -27,7 +33,13 @@ export const CustomCard = () => {
 							<h2 className="m-0 mb-1">Kitchen Sink</h2>
 						</FlexgridItem>
 						<FlexgridItem>
-							<Menu icon={<IconSettings />}>
+							<Menu
+								trigger={
+									<ButtonIcon>
+										<IconSettings />
+									</ButtonIcon>
+								}
+							>
 								<MenuItem
 									label="Profile"
 									icon={<IconProfile />}

diff --git a/packages/documentation/src/Components/Header.stories.tsx b/packages/documentation/src/Components/Header.stories.tsx
@@ -143,7 +143,13 @@ const CommonTemplate = () => {
 				</Anchor>
 			</div>
 			<div className="mb-2 flex flex-wrap justify-end gap-1">
-				<Menu icon={<IconSettings />}>
+				<Menu
+					trigger={
+						<ButtonIcon>
+							<IconSettings />
+						</ButtonIcon>
+					}
+				>
 					<MenuItem label="Profile" icon={<IconProfile />} />
 					<MenuItem label="Statistics" icon={<IconChart />} />
 					<MenuItem label="History" icon={<IconHistory />} />

diff --git a/packages/documentation/src/Components/Menu.stories.tsx b/packages/documentation/src/Components/Menu.stories.tsx
@@ -1,6 +1,7 @@
 import type { Story } from "@ladle/react";
 import {
 	Button,
+	ButtonIcon,
 	Menu,
 	MenuItem,
 	MenuSeparator,
@@ -61,7 +62,15 @@ export const Basic: Story<any> = (args) => (
 		<Button size="small" noBorder spacing={{ r: 2 }}>
 			Button
 		</Button>
-		<Menu icon={<IconSettings />} spacing={{ r: 2 }} {...args}>
+		<Menu
+			trigger={
+				<ButtonIcon>
+					<IconSettings />
+				</ButtonIcon>
+			}
+			spacing={{ r: 2 }}
+			{...args}
+		>
 			<MenuItem label="Profile" />
 			<MenuItem label="Statistics" />
 			<MenuItem label="History" />
@@ -78,7 +87,15 @@ export const WithIcons: Story<any> = (args) => (
 		<Button size="small" spacing={{ r: 2 }}>
 			Button
 		</Button>
-		<Menu icon={<IconSettings />} spacing={{ r: 2 }} {...args}>
+		<Menu
+			trigger={
+				<ButtonIcon>
+					<IconSettings />
+				</ButtonIcon>
+			}
+			spacing={{ r: 2 }}
+			{...args}
+		>
 			<MenuItem label="Profile" icon={<IconProfile />} />
 			<MenuItem label="Statistics" icon={<IconChart />} />
 			<MenuItem label="History" icon={<IconHistory />} />
@@ -123,7 +140,15 @@ export const WithMessageBox: Story<any> = (args) => {
 				<Button size="small" spacing={{ r: 2 }}>
 					Button
 				</Button>
-				<Menu icon={<IconSettings />} spacing={{ r: 2 }} {...args}>
+				<Menu
+					trigger={
+						<ButtonIcon>
+							<IconSettings />
+						</ButtonIcon>
+					}
+					spacing={{ r: 2 }}
+					{...args}
+				>
 					<MenuItem label="Profile" icon={<IconProfile />} />
 					<MenuItem label="Statistics" icon={<IconChart />} />
 					<MenuItem label="History" icon={<IconHistory />} />

diff --git a/packages/documentation/src/Styles/DarkMode.stories.tsx b/packages/documentation/src/Styles/DarkMode.stories.tsx
@@ -125,7 +125,13 @@ const CommonTemplate = () => {
 				</Anchor>
 			</div>
 			<div className="mb-2 flex flex-wrap justify-end gap-1">
-				<Menu icon={<IconSettings />}>
+				<Menu
+					trigger={
+						<ButtonIcon>
+							<IconSettings />
+						</ButtonIcon>
+					}
+				>
 					<MenuItem label="Profile" icon={<IconProfile />} />
 					<MenuItem label="Statistics" icon={<IconChart />} />
 					<MenuItem label="History" icon={<IconHistory />} />

diff --git a/packages/ui-components/src/components/Button/ButtonTypes.d.ts b/packages/ui-components/src/components/Button/ButtonTypes.d.ts
@@ -24,6 +24,11 @@ export type CommonButtonProps = {
 	 * @default false
 	 */
 	noBorder?: boolean;
+	/**
+	 * Whether or not to render the Button with a hack for Safari click.
+	 * @default false
+	 */
+	noInternalClick?: boolean;
 	/**
 	 * Whether or not to render the Button with styles or not.
 	 * @default false

diff --git a/packages/ui-components/src/components/Menu/Menu.tsx b/packages/ui-components/src/components/Menu/Menu.tsx
@@ -20,21 +20,27 @@ import {
 	useRole,
 	useTypeahead,
 } from "@floating-ui/react";
-import { forwardRef, useContext, useEffect, useRef, useState } from "react";
+import React, {
+	forwardRef,
+	useContext,
+	useEffect,
+	useRef,
+	useState,
+} from "react";
 
-import { ButtonIcon } from "..";
 import { MenuContext } from "./MenuContext";
 import type { MenuProps } from "./MenuTypes";
+import { getDisplayName } from "./utilities";
 
 export const MenuComponent = forwardRef<
 	HTMLButtonElement,
 	MenuProps & React.HTMLProps<HTMLButtonElement>
 >(
 	(
 		{
+			trigger,
 			children,
-			label,
-			icon,
+			label = "Open menu",
 			defaultPlacement = "bottom-start",
 			onOpenChange,
 			spacing,
@@ -92,6 +98,26 @@ export const MenuComponent = forwardRef<
 		const { getReferenceProps, getFloatingProps, getItemProps } =
 			useInteractions([click, role, dismiss, listNavigation, typeahead]);
 
+		const noInternalClick =
+			getDisplayName(trigger) === "Button" ||
+			getDisplayName(trigger) === "ButtonIcon";
+
+		const triggerElement = React.cloneElement(trigger as React.ReactElement, {
+			mode,
+			focusMode,
+			spacing,
+			noInternalClick,
+			"aria-label": label,
+			"data-open": isOpen ? "" : undefined,
+			"data-focus-inside": hasFocusInside ? "" : undefined,
+			ref: useMergeRefs([refs.setReference, item.ref, userRef]),
+			...getReferenceProps(
+				parent.getItemProps({
+					...props,
+				}),
+			),
+		});
+
 		// Event emitter allows you to communicate across tree components.
 		// This effect closes all menus when an item gets clicked anywhere
 		// in the tree.
@@ -121,23 +147,7 @@ export const MenuComponent = forwardRef<
 
 		return (
 			<FloatingNode id={nodeId}>
-				<ButtonIcon
-					mode={mode}
-					focusMode={focusMode}
-					spacing={spacing}
-					label={label || "Open menu"}
-					ref={useMergeRefs([refs.setReference, item.ref, userRef])}
-					data-open={isOpen ? "" : undefined}
-					data-focus-inside={hasFocusInside ? "" : undefined}
-					{...getReferenceProps(
-						parent.getItemProps({
-							...props,
-						}),
-					)}
-				>
-					{label}
-					{icon}
-				</ButtonIcon>
+				{triggerElement}
 
 				<MenuContext.Provider
 					value={{

diff --git a/packages/ui-components/src/components/Menu/MenuItem.tsx b/packages/ui-components/src/components/Menu/MenuItem.tsx
@@ -2,7 +2,6 @@ import { useFloatingTree, useListItem, useMergeRefs } from "@floating-ui/react";
 import clsx from "clsx";
 import * as React from "react";
 
-import { ButtonIcon } from "..";
 import { MenuContext } from "./MenuContext";
 import type { MenuItemProps, MenuSeparatorProps } from "./MenuTypes";
 
@@ -15,11 +14,9 @@ export const MenuItem = React.forwardRef<
 	const tree = useFloatingTree();
 
 	return (
-		<ButtonIcon
+		<button
 			{...props} // this needs to be first to allow override
-			raw
 			ref={useMergeRefs([item.ref, forwardedRef])}
-			type="button"
 			role="menuitem"
 			className="m-0 flex w-full rounded-md border border-transparent bg-none px-3 py-2 text-left text-base outline-none focus:border focus:border-border-medium focus:bg-surface-lighter focus:underline disabled:cursor-not-allowed disabled:text-copy-medium sm:py-1"
 			tabIndex={0}
@@ -34,10 +31,10 @@ export const MenuItem = React.forwardRef<
 					menu.setHasFocusInside(true);
 				},
 			})}
-			labelRight={label}
 		>
 			{icon}
-		</ButtonIcon>
+			{label && <span className="pl-2">{label}</span>}
+		</button>
 	);
 });
 

diff --git a/packages/ui-components/src/components/Menu/MenuTypes.d.ts b/packages/ui-components/src/components/Menu/MenuTypes.d.ts
@@ -1,7 +1,12 @@
 import type { Placement } from "@floating-ui/react";
 import type { SpacingProps } from "@versini/ui-private/dist/utilities";
+import React from "react";
 
-export interface MenuProps extends SpacingProps {
+export type MenuProps = {
+	/**
+	 * The component to use to open the menu, e.g. a ButtonIcon, a Button, etc.
+	 */
+	trigger: React.ReactNode;
 	/**
 	 * The children to render.
 	 */
@@ -16,10 +21,6 @@ export interface MenuProps extends SpacingProps {
 	 * of the focus ring around the Button.
 	 */
 	focusMode?: "dark" | "light" | "system" | "alt-system";
-	/**
-	 * A React component of type Icon to be placed on the left of the label.
-	 */
-	icon?: React.ReactNode;
 	/**
 	 * The type of Button trigger. This will change the color of the Button.
 	 */
@@ -33,9 +34,9 @@ export interface MenuProps extends SpacingProps {
 	 * @param open whether or not the menu is open
 	 */
 	onOpenChange?: (open: boolean) => void;
-}
+} & SpacingProps;
 
-export interface MenuItemProps {
+export type MenuItemProps = {
 	/**
 	 * The label to use for the menu item.
 	 */
@@ -49,6 +50,6 @@ export interface MenuItemProps {
 	 * A React component of type Icon to be placed on the left of the label.
 	 */
 	icon?: React.ReactNode;
-}
+};
 
 export type MenuSeparatorProps = React.HTMLAttributes<HTMLDivElement>;