Fix disabled menu items triggering click events (#4409)

jpmorganchase · Nov 15, 2024 · 1a29b4e · 1a29b4e
1 parent d2ffaa4
commit 1a29b4e
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 4 deletions.
diff --git a/.changeset/seven-bugs-tickle.md b/.changeset/seven-bugs-tickle.md
@@ -0,0 +1,5 @@
+---
+"@salt-ds/core": patch
+---
+
+Fixed disabled MenuItems triggering onClick.
diff --git a/packages/core/src/__tests__/__e2e__/menu/Menu.cy.tsx b/packages/core/src/__tests__/__e2e__/menu/Menu.cy.tsx
@@ -166,6 +166,9 @@ describe("Given a Menu", () => {
   });
 
   it("should support disabled items", () => {
+    const alertStub = cy.stub().as("alertStub");
+    cy.on("window:alert", alertStub);
+
     cy.mount(<IconWithGroups />);
     cy.findByRole("button", { name: "Open Menu" }).realClick();
     cy.findByRole("menuitem", { name: "Paste" }).should(
@@ -175,6 +178,7 @@ describe("Given a Menu", () => {
     cy.findByRole("menuitem", { name: "Paste" }).realClick();
     cy.findByRole("menu").should("exist");
     cy.findByRole("menuitem", { name: "Paste" }).should("not.be.focused");
+    cy.get("@alertStub").should("not.have.been.called");
   });
 
   it("should focus items on hover", () => {

diff --git a/packages/core/src/menu/MenuItem.tsx b/packages/core/src/menu/MenuItem.tsx
@@ -81,9 +81,11 @@ export const MenuItem = forwardRef<HTMLDivElement, MenuItemProps>(
             }
           },
           onClick(event: MouseEvent<HTMLDivElement>) {
-            onClick?.(event);
-            if (!triggersSubmenu && !disabled) {
-              tree?.events.emit("click");
+            if (!disabled) {
+              onClick?.(event);
+              if (!triggersSubmenu) {
+                tree?.events.emit("click");
+              }
             }
           },
           onFocus(event: FocusEvent<HTMLDivElement>) {

diff --git a/packages/core/stories/menu/menu.stories.tsx b/packages/core/stories/menu/menu.stories.tsx
@@ -263,7 +263,12 @@ export const IconWithGroups: StoryFn<typeof Menu> = (args) => {
             <CopyIcon aria-hidden />
             Copy
           </MenuItem>
-          <MenuItem disabled>
+          <MenuItem
+            disabled
+            onClick={() => {
+              alert("Paste");
+            }}
+          >
             <PasteIcon aria-hidden />
             Paste
           </MenuItem>