From a6c234d3767316dcd5db00b53c5fbb04ee099c07 Mon Sep 17 00:00:00 2001 From: Arno V Date: Sat, 27 Apr 2024 10:39:31 -0400 Subject: [PATCH 1/4] feat: refactor Menu to extract trigger prop This is a breaking change, but we are in alpha so no major version bump --- .../src/Components/Header.stories.tsx | 8 ++- .../src/Components/Menu.stories.tsx | 31 ++++++++++- .../src/Styles/DarkMode.stories.tsx | 8 ++- .../src/components/Button/ButtonTypes.d.ts | 5 ++ .../src/components/Menu/Menu.tsx | 55 ++++++++++++------- .../src/components/Menu/MenuItem.tsx | 9 +-- .../src/components/Menu/MenuTypes.d.ts | 16 +++--- .../components/Menu/__tests__/Menu.test.tsx | 42 +++++++++++--- .../src/components/private/BaseButton.tsx | 13 ++++- 9 files changed, 137 insertions(+), 50 deletions(-) diff --git a/packages/documentation/src/Components/Header.stories.tsx b/packages/documentation/src/Components/Header.stories.tsx index 165329f6..f78427d2 100644 --- a/packages/documentation/src/Components/Header.stories.tsx +++ b/packages/documentation/src/Components/Header.stories.tsx @@ -143,7 +143,13 @@ const CommonTemplate = () => {
- }> + + + + } + > } /> } /> } /> diff --git a/packages/documentation/src/Components/Menu.stories.tsx b/packages/documentation/src/Components/Menu.stories.tsx index f848dfa9..00ba7ae5 100644 --- 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 = (args) => ( - } spacing={{ r: 2 }} {...args}> + + + + } + spacing={{ r: 2 }} + {...args} + > @@ -78,7 +87,15 @@ export const WithIcons: Story = (args) => ( - } spacing={{ r: 2 }} {...args}> + + + + } + spacing={{ r: 2 }} + {...args} + > } /> } /> } /> @@ -123,7 +140,15 @@ export const WithMessageBox: Story = (args) => { - } spacing={{ r: 2 }} {...args}> + + + + } + spacing={{ r: 2 }} + {...args} + > } /> } /> } /> diff --git a/packages/documentation/src/Styles/DarkMode.stories.tsx b/packages/documentation/src/Styles/DarkMode.stories.tsx index 17145313..40568626 100644 --- a/packages/documentation/src/Styles/DarkMode.stories.tsx +++ b/packages/documentation/src/Styles/DarkMode.stories.tsx @@ -125,7 +125,13 @@ const CommonTemplate = () => {
- }> + + + + } + > } /> } /> } /> diff --git a/packages/ui-components/src/components/Button/ButtonTypes.d.ts b/packages/ui-components/src/components/Button/ButtonTypes.d.ts index 71c259d6..5b12cacf 100644 --- 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 index da736562..92bc7caf 100644 --- a/packages/ui-components/src/components/Menu/Menu.tsx +++ b/packages/ui-components/src/components/Menu/Menu.tsx @@ -20,9 +20,14 @@ 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"; @@ -32,9 +37,9 @@ export const MenuComponent = forwardRef< >( ( { + trigger, children, - label, - icon, + label = "Open menu", defaultPlacement = "bottom-start", onOpenChange, spacing, @@ -92,6 +97,30 @@ export const MenuComponent = forwardRef< const { getReferenceProps, getFloatingProps, getItemProps } = useInteractions([click, role, dismiss, listNavigation, typeahead]); + let isKnownButton = false; + if ( + trigger?.type?.displayName === "Button" || + trigger?.type?.displayName === "ButtonIcon" + ) { + isKnownButton = true; + } + + const triggerElement = React.cloneElement(trigger as React.ReactElement, { + mode, + focusMode, + spacing, + noInternalClick: isKnownButton, + "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 +150,7 @@ export const MenuComponent = forwardRef< return ( - - {label} - {icon} - + {triggerElement} {icon} - + {label && {label}} + ); }); diff --git a/packages/ui-components/src/components/Menu/MenuTypes.d.ts b/packages/ui-components/src/components/Menu/MenuTypes.d.ts index 1f21978d..32f5be73 100644 --- a/packages/ui-components/src/components/Menu/MenuTypes.d.ts +++ b/packages/ui-components/src/components/Menu/MenuTypes.d.ts @@ -1,7 +1,11 @@ import type { Placement } from "@floating-ui/react"; import type { SpacingProps } from "@versini/ui-private/dist/utilities"; -export interface MenuProps extends SpacingProps { +export type MenuProps = { + /** + * The component to use to open the menu, e.g. a ButtonIcon, a Button, etc. + */ + trigger: any; /** * The children to render. */ @@ -16,10 +20,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 +33,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 +49,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; diff --git a/packages/ui-components/src/components/Menu/__tests__/Menu.test.tsx b/packages/ui-components/src/components/Menu/__tests__/Menu.test.tsx index 2e65176a..6f20275c 100644 --- a/packages/ui-components/src/components/Menu/__tests__/Menu.test.tsx +++ b/packages/ui-components/src/components/Menu/__tests__/Menu.test.tsx @@ -1,7 +1,8 @@ import { render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import { IconSettings } from "@versini/ui-icons"; -import { Menu, MenuItem, MenuSeparator } from "../.."; +import { Button, ButtonIcon, Menu, MenuItem, MenuSeparator } from "../.."; const MENU_TRIGGER_LABEL = "Click Me"; const FIRST_MENU_ITEM = "Menu 1"; @@ -11,7 +12,25 @@ const FOURTH_MENU_ITEM = "Menu 4"; const FIFTH_MENU_ITEM = "Menu 5"; const SimpleMenu = ({ ...props }) => ( - + Click Me} {...props}> + + + + + + + +); + +const SimpleMenuIcon = ({ ...props }) => ( + + + + } + {...props} + > @@ -37,9 +56,14 @@ describe("Menu (exceptions)", () => { }); describe("Menu rendering", () => { - it("should render a menu with a trigger", () => { + it("should render a menu with a trigger", async () => { render(); - const trigger = screen.getByLabelText("Open menu"); + const trigger = await screen.findByLabelText("Open menu"); + expect(trigger).toBeInTheDocument(); + }); + it("should render a menu with a icon-only trigger", async () => { + render(); + const trigger = await screen.findByLabelText("Open menu"); expect(trigger).toBeInTheDocument(); }); }); @@ -133,7 +157,11 @@ describe("Menu behaviors", () => { it("should trigger the Menu onOpenChange callback when opened and closed", async () => { const onOpenChange = vi.fn(); const { user } = renderWithUserEvent( - + Click Me} + label={MENU_TRIGGER_LABEL} + onOpenChange={onOpenChange} + > @@ -158,7 +186,7 @@ describe("Menu behaviors", () => { it("should trigger the MenuItem onClick callback when a menuitem is selected", async () => { const onClick = vi.fn(); const { user } = renderWithUserEvent( - + Click Me} label={MENU_TRIGGER_LABEL}> @@ -180,7 +208,7 @@ describe("Menu behaviors", () => { it("should trigger the MenuItem onFocus callback when a menuitem is selected", async () => { const onFocus = vi.fn(); const { user } = renderWithUserEvent( - + Click Me} label={MENU_TRIGGER_LABEL}> diff --git a/packages/ui-components/src/components/private/BaseButton.tsx b/packages/ui-components/src/components/private/BaseButton.tsx index 2a229a43..51379f38 100644 --- a/packages/ui-components/src/components/private/BaseButton.tsx +++ b/packages/ui-components/src/components/private/BaseButton.tsx @@ -18,25 +18,32 @@ import type { ButtonProps } from "../Button/ButtonTypes"; */ const internalClick = ( e: React.MouseEvent, + noInternalClick: boolean, onClick?: (e: React.MouseEvent) => void, ) => { - if (!document.activeElement || document.activeElement !== e.currentTarget) { + if ( + !noInternalClick && + (!document.activeElement || document.activeElement !== e.currentTarget) + ) { typeof e?.currentTarget?.focus === "function" && e.currentTarget.focus(); } + typeof onClick === "function" && onClick(e); }; export const BaseButton = React.forwardRef( (buttonProps: any, ref) => { - const { onClick, ...otherProps } = buttonProps; + const { onClick, noInternalClick = false, ...otherProps } = buttonProps; return (