fix: menu item classNames not work (#4156)

* fix: menu item classNames not work * feat: changeset * docs: update * feat: merge classes utility added * Update .changeset/brave-trains-wave.md --------- Co-authored-by: WK Wong <[email protected]> Co-authored-by: Junior Garcia <[email protected]>
nextui-org · Nov 28, 2024 · d37007c · d37007c
1 parent c8f2ec8
commit d37007c
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 3 deletions.
diff --git a/.changeset/brave-trains-wave.md b/.changeset/brave-trains-wave.md
@@ -0,0 +1,6 @@
+---
+"@nextui-org/menu": patch
+"@nextui-org/theme": patch
+---
+
+Fix menu item classNames not work (#4119)
diff --git a/apps/docs/content/docs/components/dropdown.mdx b/apps/docs/content/docs/components/dropdown.mdx
@@ -386,7 +386,7 @@ you to customize each item individually.
 | isReadOnly       | `boolean`                                                                                     | Whether the dropdown item press events should be ignored.                                                                                                                        | `false` |
 | hideSelectedIcon | `boolean`                                                                                     | Whether to hide the check icon when the item is selected.                                                                                                                        | `false` |
 | closeOnSelect    | `boolean`                                                                                     | Whether the dropdown menu should be closed when the item is selected.                                                                                                            | `true`  |
-| classNames       | `Record<"base"｜ "wrapper"｜ "title"｜ "description"｜ "shortcut" ｜ "selectedIcon", string>` | Allows to set custom class names for the dropdown item slots.                                                                                                                    | -       |
+| classNames       | `Record<"base"｜ "wrapper"｜ "title"｜ "description"｜ "shortcut" ｜ "selectedIcon", string>` | Allows to set custom class names for the dropdown item slots, which will override the menu `itemClasses`.                                                                                                                    | -       |
 
 ### DropdownItem Events
 

diff --git a/packages/components/menu/__tests__/menu.test.tsx b/packages/components/menu/__tests__/menu.test.tsx
@@ -344,6 +344,47 @@ describe("Menu", () => {
     expect(onClick).toHaveBeenCalledTimes(1);
   });
 
+  it("should menuItem classNames work", () => {
+    const wrapper = render(
+      <Menu>
+        <MenuItem classNames={{title: "test"}}>New file</MenuItem>
+      </Menu>,
+    );
+    const menuItem = wrapper.getByText("New file");
+
+    expect(menuItem.classList.contains("test")).toBeTruthy();
+  });
+
+  it("should menuItem classNames override menu itemClasses", () => {
+    const wrapper = render(
+      <Menu itemClasses={{title: "test"}}>
+        <MenuItem classNames={{title: "test2"}}>New file</MenuItem>
+      </Menu>,
+    );
+    const menuItem = wrapper.getByText("New file");
+
+    expect(menuItem.classList.contains("test2")).toBeTruthy();
+  });
+  it("should merge menu item classNames with itemClasses", () => {
+    const wrapper = render(
+      <Menu itemClasses={{title: "test"}}>
+        <MenuItem classNames={{title: "test2"}}>New file</MenuItem>
+        <MenuItem>Delete file</MenuItem>
+      </Menu>,
+    );
+
+    const menuItemWithBoth = wrapper.getByText("New file");
+    const menuItemWithDefault = wrapper.getByText("Delete file");
+
+    // Check first MenuItem has both classes
+    expect(menuItemWithBoth.classList.contains("test2")).toBeTruthy();
+    expect(menuItemWithBoth.classList.contains("test")).toBeTruthy();
+
+    // Check second MenuItem only has the default class
+    expect(menuItemWithDefault.classList.contains("test")).toBeTruthy();
+    expect(menuItemWithDefault.classList.contains("test2")).toBeFalsy();
+  });
+
   it("should truncate the text if the child is not a string", () => {
     const wrapper = render(
       <Menu>

diff --git a/packages/components/menu/src/menu.tsx b/packages/components/menu/src/menu.tsx
@@ -1,5 +1,6 @@
 import {forwardRef} from "@nextui-org/system";
 import {ForwardedRef, ReactElement, Ref} from "react";
+import {mergeClasses} from "@nextui-org/theme";
 
 import {UseMenuProps, useMenu} from "./use-menu";
 import MenuSection from "./menu-section";
@@ -48,10 +49,12 @@ function Menu<T extends object>(props: Props<T>, ref: ForwardedRef<HTMLUListElem
           ...item.props,
         };
 
+        const mergedItemClasses = mergeClasses(itemClasses, itemProps?.classNames);
+
         if (item.type === "section") {
-          return <MenuSection key={item.key} {...itemProps} itemClasses={itemClasses} />;
+          return <MenuSection key={item.key} {...itemProps} itemClasses={mergedItemClasses} />;
         }
-        let menuItem = <MenuItem key={item.key} {...itemProps} classNames={itemClasses} />;
+        let menuItem = <MenuItem key={item.key} {...itemProps} classNames={mergedItemClasses} />;
 
         if (item.wrapper) {
           menuItem = item.wrapper(menuItem);

diff --git a/packages/core/theme/src/utils/index.ts b/packages/core/theme/src/utils/index.ts
@@ -11,4 +11,5 @@ export {
 export type {SlotsToClasses} from "./types";
 export {colorVariants} from "./variants";
 export {COMMON_UNITS, twMergeConfig} from "./tw-merge-config";
+export {mergeClasses} from "./merge-classes";
 export {cn} from "./cn";
diff --git a/packages/core/theme/src/utils/merge-classes.ts b/packages/core/theme/src/utils/merge-classes.ts
@@ -0,0 +1,26 @@
+import type {SlotsToClasses} from "./types";
+
+import {clsx} from "@nextui-org/shared-utils";
+
+/**
+ * Merges two sets of class names for each slot in a component.
+ * @param itemClasses - Base classes for each slot
+ * @param itemPropsClasses - Additional classes from props for each slot
+ * @returns A merged object containing the combined classes for each slot
+ */
+export const mergeClasses = <T extends SlotsToClasses<string>, P extends SlotsToClasses<string>>(
+  itemClasses?: T,
+  itemPropsClasses?: P,
+): T => {
+  if (!itemClasses && !itemPropsClasses) return {} as T;
+
+  const keys = new Set([...Object.keys(itemClasses || {}), ...Object.keys(itemPropsClasses || {})]);
+
+  return Array.from(keys).reduce(
+    (acc, key) => ({
+      ...acc,
+      [key]: clsx(itemClasses?.[key], itemPropsClasses?.[key]),
+    }),
+    {} as T,
+  );
+};