diff --git a/.changeset/fix-mui-nested-buttons.md b/.changeset/fix-mui-nested-buttons.md new file mode 100644 index 0000000000000..19247a2171954 --- /dev/null +++ b/.changeset/fix-mui-nested-buttons.md @@ -0,0 +1,5 @@ +--- +"@refinedev/mui": patch +--- + +fix(mui): prevent nested interactive elements in navigation buttons diff --git a/packages/inferencer/src/inferencers/mui/__snapshots__/index.test.tsx.snap b/packages/inferencer/src/inferencers/mui/__snapshots__/index.test.tsx.snap index b840c574fe135..7b7eaec42a940 100644 --- a/packages/inferencer/src/inferencers/mui/__snapshots__/index.test.tsx.snap +++ b/packages/inferencer/src/inferencers/mui/__snapshots__/index.test.tsx.snap @@ -1687,50 +1687,42 @@ exports[`MuiInferencer > should match the snapshot 1`] = ` tabindex="-1" > - + + - + +
+ + - + +
+ + + + Posts + + + + Create
@@ -1718,50 +1713,40 @@ exports[`MuiListInferencer > should match the snapshot 1`] = ` tabindex="-1" > - + + - + + @@ -1997,50 +1982,40 @@ exports[`MuiListInferencer > should match the snapshot 1`] = ` tabindex="-1" > - + + - + + diff --git a/packages/inferencer/src/inferencers/mui/__tests__/__snapshots__/show.test.tsx.snap b/packages/inferencer/src/inferencers/mui/__tests__/__snapshots__/show.test.tsx.snap index 43a3d93517e95..9eac00eb44fce 100644 --- a/packages/inferencer/src/inferencers/mui/__tests__/__snapshots__/show.test.tsx.snap +++ b/packages/inferencer/src/inferencers/mui/__tests__/__snapshots__/show.test.tsx.snap @@ -95,32 +95,27 @@ exports[`MuiShowInferencer > should match the snapshot 1`] = ` class="MuiBox-root css-10egq61" > - + + + + Posts - + {hideText ? ( + + ) : ( + children ?? label + )} + ); }; diff --git a/packages/mui/src/components/buttons/create/index.tsx b/packages/mui/src/components/buttons/create/index.tsx index 65b72471067e0..b1d631b410ae5 100644 --- a/packages/mui/src/components/buttons/create/index.tsx +++ b/packages/mui/src/components/buttons/create/index.tsx @@ -42,9 +42,11 @@ export const CreateButton: React.FC = ({ const { sx, ...restProps } = props; return ( - ) => { if (isDisabled) { e.preventDefault(); @@ -55,24 +57,19 @@ export const CreateButton: React.FC = ({ onClick(e); } }} - style={{ textDecoration: "none" }} + startIcon={!hideText && } + title={title} + variant="contained" + sx={{ minWidth: 0, textDecoration: "none", ...sx }} + data-testid={RefineButtonTestIds.CreateButton} + className={RefineButtonClassNames.CreateButton} + {...restProps} > - - + {hideText ? ( + + ) : ( + children ?? label + )} + ); }; diff --git a/packages/mui/src/components/buttons/edit/index.tsx b/packages/mui/src/components/buttons/edit/index.tsx index a188eb3c04a18..11595ddabc766 100644 --- a/packages/mui/src/components/buttons/edit/index.tsx +++ b/packages/mui/src/components/buttons/edit/index.tsx @@ -43,7 +43,9 @@ export const EditButton: React.FC = ({ const { sx, ...restProps } = rest; return ( - ) => { @@ -56,27 +58,22 @@ export const EditButton: React.FC = ({ onClick(e); } }} - style={{ textDecoration: "none" }} + startIcon={ + !hideText && ( + + ) + } + title={title} + sx={{ minWidth: 0, textDecoration: "none", ...sx }} + data-testid={RefineButtonTestIds.EditButton} + className={RefineButtonClassNames.EditButton} + {...restProps} > - - + {hideText ? ( + + ) : ( + children ?? label + )} + ); }; diff --git a/packages/mui/src/components/buttons/list/index.tsx b/packages/mui/src/components/buttons/list/index.tsx index f350193ae4660..1fcaab2d0dc47 100644 --- a/packages/mui/src/components/buttons/list/index.tsx +++ b/packages/mui/src/components/buttons/list/index.tsx @@ -41,9 +41,11 @@ export const ListButton: React.FC = ({ const { sx, ...restProps } = rest; return ( - ) => { if (isDisabled) { e.preventDefault(); @@ -54,23 +56,18 @@ export const ListButton: React.FC = ({ onClick(e); } }} - style={{ textDecoration: "none" }} + startIcon={!hideText && } + title={title} + sx={{ minWidth: 0, textDecoration: "none", ...sx }} + data-testid={RefineButtonTestIds.ListButton} + className={RefineButtonClassNames.ListButton} + {...restProps} > - - + {hideText ? ( + + ) : ( + children ?? label + )} + ); }; diff --git a/packages/mui/src/components/buttons/show/index.tsx b/packages/mui/src/components/buttons/show/index.tsx index ebc80cfe4a202..4a76918382408 100644 --- a/packages/mui/src/components/buttons/show/index.tsx +++ b/packages/mui/src/components/buttons/show/index.tsx @@ -43,9 +43,11 @@ export const ShowButton: React.FC = ({ const { sx, ...restProps } = rest; return ( - ) => { if (isDisabled) { e.preventDefault(); @@ -56,23 +58,18 @@ export const ShowButton: React.FC = ({ onClick(e); } }} - style={{ textDecoration: "none" }} + startIcon={!hideText && } + title={title} + sx={{ minWidth: 0, textDecoration: "none", ...sx }} + data-testid={RefineButtonTestIds.ShowButton} + className={RefineButtonClassNames.ShowButton} + {...restProps} > - - + {hideText ? ( + + ) : ( + children ?? label + )} + ); }; diff --git a/packages/mui/src/components/crud/show/index.spec.tsx b/packages/mui/src/components/crud/show/index.spec.tsx index 816ab752894ba..09d9e5fb907f6 100644 --- a/packages/mui/src/components/crud/show/index.spec.tsx +++ b/packages/mui/src/components/crud/show/index.spec.tsx @@ -95,10 +95,10 @@ describe("Show", () => { ); await waitFor(() => - expect(getByText("Edit").closest("button")).not.toBeDisabled(), + expect(getByText("Edit").closest("button, a")).not.toBeDisabled(), ); await waitFor(() => - expect(getAllByText("Posts")[1].closest("button")).not.toBeDisabled(), + expect(getAllByText("Posts")[1].closest("button, a")).not.toBeDisabled(), ); await waitFor(() => diff --git a/packages/mui/test/vitest.setup.ts b/packages/mui/test/vitest.setup.ts index a57b504aff175..3b563cbc4ae6b 100644 --- a/packages/mui/test/vitest.setup.ts +++ b/packages/mui/test/vitest.setup.ts @@ -1,7 +1,39 @@ import "@testing-library/jest-dom/vitest"; import "@testing-library/react"; import { configure } from "@testing-library/dom"; -import { vi } from "vitest"; +import { vi, expect } from "vitest"; + +// Override toBeDisabled to also handle aria-disabled for non-form elements. +// MUI renders navigation buttons as elements with component={LinkComponent}, +// which use aria-disabled="true" instead of native disabled attribute. +expect.extend({ + toBeDisabled(received) { + if ( + !(received instanceof HTMLElement) && + !(received instanceof SVGElement) + ) { + return { + pass: this.isNot, + message: () => + `received value must be an HTMLElement or an SVGElement.\nReceived has value: ${received}`, + }; + } + const element = received as HTMLElement; + const isNativelyDisabled = element.hasAttribute("disabled"); + const isAriaDisabled = element.getAttribute("aria-disabled") === "true"; + + return { + pass: isNativelyDisabled || isAriaDisabled, + message: () => { + const clone = element.cloneNode(false) as HTMLElement; + if (this.isNot) { + return `expected element not to be disabled:\n\n${clone.outerHTML}`; + } + return `expected element to be disabled:\n\n${clone.outerHTML}`; + }, + }; + }, +}); // Mock CSS imports vi.mock("*.css", () => ({})); diff --git a/packages/ui-tests/src/tests/buttons/clone.tsx b/packages/ui-tests/src/tests/buttons/clone.tsx index adb22ee114496..09d3f9df30773 100644 --- a/packages/ui-tests/src/tests/buttons/clone.tsx +++ b/packages/ui-tests/src/tests/buttons/clone.tsx @@ -32,7 +32,7 @@ export const buttonCloneTests = ( expect(container).toBeTruthy(); - expect(getByText("Clone").closest("button")).not.toBeDisabled(); + expect(getByText("Clone").closest("button, a")).not.toBeDisabled(); }); it("should be disabled by prop", async () => { @@ -45,9 +45,9 @@ export const buttonCloneTests = ( }, ); - expect(getByText("Clone").closest("button")).toBeDisabled(); + expect(getByText("Clone").closest("button, a")).toBeDisabled(); - fireEvent.click(getByText("Clone").closest("button") as Element); + fireEvent.click(getByText("Clone").closest("button, a") as Element); expect(mockOnClick).not.toHaveBeenCalled(); }); @@ -119,12 +119,14 @@ export const buttonCloneTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("Clone").closest("button")).toBeDisabled(), + expect(getByText("Clone").closest("button, a")).toBeDisabled(), ); waitFor(() => expect( - getByText("Clone").closest("button")?.getAttribute("title"), + getByText("Clone") + .closest("button, a") + ?.getAttribute("title"), ).toBe("Access Denied"), ); }); @@ -155,7 +157,9 @@ export const buttonCloneTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("Clone").closest("button")).not.toBeDisabled(), + expect( + getByText("Clone").closest("button, a"), + ).not.toBeDisabled(), ); }); @@ -171,7 +175,7 @@ export const buttonCloneTests = ( }, ); - const button = getByText("Clone").closest("button"); + const button = getByText("Clone").closest("button, a"); expect(button).toBeDisabled(); }); }); @@ -222,7 +226,7 @@ export const buttonCloneTests = ( expect(container).toBeTruthy(); - expect(getByText("Clone").closest("button")).not.toBeDisabled(); + expect(getByText("Clone").closest("button, a")).not.toBeDisabled(); }); }); }); @@ -257,7 +261,9 @@ export const buttonCloneTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("Clone").closest("button")).not.toBeDisabled(), + expect( + getByText("Clone").closest("button, a"), + ).not.toBeDisabled(), ); }); }); @@ -321,12 +327,14 @@ export const buttonCloneTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("Clone").closest("button")).toBeDisabled(), + expect(getByText("Clone").closest("button, a")).toBeDisabled(), ); waitFor(() => expect( - getByText("Clone").closest("button")?.getAttribute("title"), + getByText("Clone") + .closest("button, a") + ?.getAttribute("title"), ).toBe("Access Denied"), ); }); diff --git a/packages/ui-tests/src/tests/buttons/create.tsx b/packages/ui-tests/src/tests/buttons/create.tsx index d60b5898a176e..412a604decf3b 100644 --- a/packages/ui-tests/src/tests/buttons/create.tsx +++ b/packages/ui-tests/src/tests/buttons/create.tsx @@ -32,7 +32,7 @@ export const buttonCreateTests = ( expect(container).toBeTruthy(); - expect(getByText("Create").closest("button")).not.toBeDisabled(); + expect(getByText("Create").closest("button, a")).not.toBeDisabled(); }); it("should be disabled by prop", async () => { @@ -45,9 +45,9 @@ export const buttonCreateTests = ( }, ); - expect(getByText("Create").closest("button")).toBeDisabled(); + expect(getByText("Create").closest("button, a")).toBeDisabled(); - fireEvent.click(getByText("Create").closest("button") as Element); + fireEvent.click(getByText("Create").closest("button, a") as Element); expect(mockOnClick).not.toHaveBeenCalled(); }); @@ -62,7 +62,7 @@ export const buttonCreateTests = ( }, ); - const button = getByText("Create").closest("button"); + const button = getByText("Create").closest("button, a"); await act(async () => { fireEvent.click(button!); }); @@ -136,12 +136,14 @@ export const buttonCreateTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("Create").closest("button")).toBeDisabled(), + expect(getByText("Create").closest("button, a")).toBeDisabled(), ); waitFor(() => expect( - getByText("Create").closest("button")?.getAttribute("title"), + getByText("Create") + .closest("button, a") + ?.getAttribute("title"), ).toBe("Access Denied"), ); }); @@ -173,7 +175,7 @@ export const buttonCreateTests = ( await waitFor(() => expect( - getByText("Create").closest("button"), + getByText("Create").closest("button, a"), ).not.toBeDisabled(), ); }); @@ -190,7 +192,7 @@ export const buttonCreateTests = ( }, ); - const button = getByText("Create").closest("button"); + const button = getByText("Create").closest("button, a"); expect(button).toBeDisabled(); }); }); @@ -241,7 +243,7 @@ export const buttonCreateTests = ( expect(container).toBeTruthy(); - expect(getByText("Create").closest("button")).not.toBeDisabled(); + expect(getByText("Create").closest("button, a")).not.toBeDisabled(); }); }); }); @@ -277,7 +279,7 @@ export const buttonCreateTests = ( await waitFor(() => expect( - getByText("Create").closest("button"), + getByText("Create").closest("button, a"), ).not.toBeDisabled(), ); }); @@ -342,12 +344,14 @@ export const buttonCreateTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("Create").closest("button")).toBeDisabled(), + expect(getByText("Create").closest("button, a")).toBeDisabled(), ); waitFor(() => expect( - getByText("Create").closest("button")?.getAttribute("title"), + getByText("Create") + .closest("button, a") + ?.getAttribute("title"), ).toBe("Access Denied"), ); }); diff --git a/packages/ui-tests/src/tests/buttons/edit.tsx b/packages/ui-tests/src/tests/buttons/edit.tsx index d0df50534481c..3fe98d381fb68 100644 --- a/packages/ui-tests/src/tests/buttons/edit.tsx +++ b/packages/ui-tests/src/tests/buttons/edit.tsx @@ -25,7 +25,7 @@ export const buttonEditTests = ( expect(container).toBeTruthy(); - expect(getByText("Edit").closest("button")).not.toBeDisabled(); + expect(getByText("Edit").closest("button, a")).not.toBeDisabled(); }); it("should be disabled by prop", async () => { @@ -38,9 +38,9 @@ export const buttonEditTests = ( }, ); - expect(getByText("Edit").closest("button")).toBeDisabled(); + expect(getByText("Edit").closest("button, a")).toBeDisabled(); - fireEvent.click(getByText("Edit").closest("button") as Element); + fireEvent.click(getByText("Edit").closest("button, a") as Element); expect(mockOnClick).not.toHaveBeenCalled(); }); @@ -109,12 +109,12 @@ export const buttonEditTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("Edit").closest("button")).toBeDisabled(), + expect(getByText("Edit").closest("button, a")).toBeDisabled(), ); waitFor(() => expect( - getByText("Edit").closest("button")?.getAttribute("title"), + getByText("Edit").closest("button, a")?.getAttribute("title"), ).toBe("Access Denied"), ); }); @@ -145,7 +145,9 @@ export const buttonEditTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("Edit").closest("button")).not.toBeDisabled(), + expect( + getByText("Edit").closest("button, a"), + ).not.toBeDisabled(), ); }); @@ -161,7 +163,7 @@ export const buttonEditTests = ( }, ); - const button = getByText("Edit").closest("button"); + const button = getByText("Edit").closest("button, a"); expect(button).toBeDisabled(); }); }); @@ -212,7 +214,7 @@ export const buttonEditTests = ( expect(container).toBeTruthy(); - expect(getByText("Edit").closest("button")).not.toBeDisabled(); + expect(getByText("Edit").closest("button, a")).not.toBeDisabled(); }); }); }); @@ -247,7 +249,9 @@ export const buttonEditTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("Edit").closest("button")).not.toBeDisabled(), + expect( + getByText("Edit").closest("button, a"), + ).not.toBeDisabled(), ); }); }); @@ -309,12 +313,12 @@ export const buttonEditTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("Edit").closest("button")).toBeDisabled(), + expect(getByText("Edit").closest("button, a")).toBeDisabled(), ); waitFor(() => expect( - getByText("Edit").closest("button")?.getAttribute("title"), + getByText("Edit").closest("button, a")?.getAttribute("title"), ).toBe("Access Denied"), ); }); diff --git a/packages/ui-tests/src/tests/buttons/list.tsx b/packages/ui-tests/src/tests/buttons/list.tsx index 1f9ec26c69b67..e1d79afd0f2d1 100644 --- a/packages/ui-tests/src/tests/buttons/list.tsx +++ b/packages/ui-tests/src/tests/buttons/list.tsx @@ -28,7 +28,7 @@ export const buttonListTests = ( expect(container).toBeTruthy(); - expect(getByText("List").closest("button")).not.toBeDisabled(); + expect(getByText("List").closest("button, a")).not.toBeDisabled(); }); it("should have the correct test-id", async () => { @@ -51,9 +51,9 @@ export const buttonListTests = ( }, ); - expect(getByText("List").closest("button")).toBeDisabled(); + expect(getByText("List").closest("button, a")).toBeDisabled(); - fireEvent.click(getByText("List").closest("button") as Element); + fireEvent.click(getByText("List").closest("button, a") as Element); expect(mockOnClick).not.toHaveBeenCalled(); }); @@ -166,12 +166,12 @@ export const buttonListTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("List").closest("button")).toBeDisabled(), + expect(getByText("List").closest("button, a")).toBeDisabled(), ); waitFor(() => expect( - getByText("List").closest("button")?.getAttribute("title"), + getByText("List").closest("button, a")?.getAttribute("title"), ).toBe("Access Denied"), ); }); @@ -202,7 +202,9 @@ export const buttonListTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("List").closest("button")).not.toBeDisabled(), + expect( + getByText("List").closest("button, a"), + ).not.toBeDisabled(), ); }); @@ -218,7 +220,7 @@ export const buttonListTests = ( }, ); - const button = getByText("List").closest("button"); + const button = getByText("List").closest("button, a"); expect(button).toBeDisabled(); }); }); @@ -269,7 +271,7 @@ export const buttonListTests = ( expect(container).toBeTruthy(); - expect(getByText("List").closest("button")).not.toBeDisabled(); + expect(getByText("List").closest("button, a")).not.toBeDisabled(); }); }); }); @@ -304,7 +306,9 @@ export const buttonListTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("List").closest("button")).not.toBeDisabled(), + expect( + getByText("List").closest("button, a"), + ).not.toBeDisabled(), ); }); }); @@ -366,12 +370,12 @@ export const buttonListTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("List").closest("button")).toBeDisabled(), + expect(getByText("List").closest("button, a")).toBeDisabled(), ); waitFor(() => expect( - getByText("List").closest("button")?.getAttribute("title"), + getByText("List").closest("button, a")?.getAttribute("title"), ).toBe("Access Denied"), ); }); diff --git a/packages/ui-tests/src/tests/buttons/show.tsx b/packages/ui-tests/src/tests/buttons/show.tsx index e4f947f714dac..a07f96b86b5ae 100644 --- a/packages/ui-tests/src/tests/buttons/show.tsx +++ b/packages/ui-tests/src/tests/buttons/show.tsx @@ -32,7 +32,7 @@ export const buttonShowTests = ( expect(container).toBeTruthy(); - expect(getByText("Show").closest("button")).not.toBeDisabled(); + expect(getByText("Show").closest("button, a")).not.toBeDisabled(); }); it("should be disabled by prop", async () => { @@ -45,9 +45,9 @@ export const buttonShowTests = ( }, ); - expect(getByText("Show").closest("button")).toBeDisabled(); + expect(getByText("Show").closest("button, a")).toBeDisabled(); - fireEvent.click(getByText("Show").closest("button") as Element); + fireEvent.click(getByText("Show").closest("button, a") as Element); expect(mockOnClick).not.toHaveBeenCalled(); }); @@ -116,12 +116,12 @@ export const buttonShowTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("Show").closest("button")).toBeDisabled(), + expect(getByText("Show").closest("button, a")).toBeDisabled(), ); waitFor(() => expect( - getByText("Show").closest("button")?.getAttribute("title"), + getByText("Show").closest("button, a")?.getAttribute("title"), ).toBe("Access Denied"), ); }); @@ -152,7 +152,9 @@ export const buttonShowTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("Show").closest("button")).not.toBeDisabled(), + expect( + getByText("Show").closest("button, a"), + ).not.toBeDisabled(), ); }); @@ -168,7 +170,7 @@ export const buttonShowTests = ( }, ); - const button = getByText("Show").closest("button"); + const button = getByText("Show").closest("button, a"); expect(button).toBeDisabled(); }); }); @@ -219,7 +221,7 @@ export const buttonShowTests = ( expect(container).toBeTruthy(); - expect(getByText("Show").closest("button")).not.toBeDisabled(); + expect(getByText("Show").closest("button, a")).not.toBeDisabled(); }); }); }); @@ -254,7 +256,9 @@ export const buttonShowTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("Show").closest("button")).not.toBeDisabled(), + expect( + getByText("Show").closest("button, a"), + ).not.toBeDisabled(), ); }); }); @@ -316,12 +320,12 @@ export const buttonShowTests = ( expect(container).toBeTruthy(); await waitFor(() => - expect(getByText("Show").closest("button")).toBeDisabled(), + expect(getByText("Show").closest("button, a")).toBeDisabled(), ); waitFor(() => expect( - getByText("Show").closest("button")?.getAttribute("title"), + getByText("Show").closest("button, a")?.getAttribute("title"), ).toBe("Access Denied"), ); });