diff --git a/.changeset/steady-button-tooltips.md b/.changeset/steady-button-tooltips.md new file mode 100644 index 000000000..04f9bb116 --- /dev/null +++ b/.changeset/steady-button-tooltips.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/kumo": patch +--- + +Keep Button and LinkButton tooltip triggers stable when disabled or loading state changes. diff --git a/packages/kumo/src/components/button/button.test.tsx b/packages/kumo/src/components/button/button.test.tsx index ed2cf490d..1e9689df3 100644 --- a/packages/kumo/src/components/button/button.test.tsx +++ b/packages/kumo/src/components/button/button.test.tsx @@ -116,8 +116,12 @@ describe("Button", () => { it("title prop wraps in Tooltip and removes native title attribute", () => { render(); const button = screen.getByRole("button", { name: "Save" }); + const trigger = button.parentElement; // title is intercepted by Tooltip wrapper, not set as native attribute expect(button.getAttribute("title")).toBeNull(); + expect(button.hasAttribute("data-base-ui-tooltip-trigger")).toBe(false); + expect(trigger?.tagName).toBe("SPAN"); + expect(trigger?.hasAttribute("data-base-ui-tooltip-trigger")).toBe(true); }); it("uses title as the accessible name when there are no children", () => { @@ -178,6 +182,37 @@ describe("Button", () => { expect(trigger?.hasAttribute("disabled")).toBe(false); }); + it.each(["disabled", "loading"] as const)( + "keeps the tooltip trigger mounted when %s changes", + (state) => { + const { container, rerender } = render( + , + ); + const trigger = container.querySelector("[data-base-ui-tooltip-trigger]"); + const button = screen.getByRole("button"); + const activeState = + state === "disabled" ? { disabled: true } : { loading: true }; + + expect(trigger).toBeTruthy(); + + rerender( + , + ); + expect(container.querySelector("[data-base-ui-tooltip-trigger]")).toBe( + trigger, + ); + expect(screen.getByRole("button")).toBe(button); + + rerender(); + expect(container.querySelector("[data-base-ui-tooltip-trigger]")).toBe( + trigger, + ); + expect(screen.getByRole("button")).toBe(button); + }, + ); + it("keeps emphasized variant rings color-matched when pressed or focused", () => { for (const variant of ["primary", "destructive"] as const) { const className = buttonVariants({ variant }); @@ -249,9 +284,12 @@ describe("LinkButton", () => { , ); const link = screen.getByRole("link", { name: "Home" }); + const trigger = link.parentElement; // title is intercepted by the Kumo Tooltip wrapper, not set as a native attribute expect(link.getAttribute("title")).toBeNull(); - expect(link.hasAttribute("data-base-ui-tooltip-trigger")).toBe(true); + expect(link.hasAttribute("data-base-ui-tooltip-trigger")).toBe(false); + expect(trigger?.tagName).toBe("SPAN"); + expect(trigger?.hasAttribute("data-base-ui-tooltip-trigger")).toBe(true); }); describe("disabled", () => { @@ -322,5 +360,57 @@ describe("LinkButton", () => { expect(trigger?.hasAttribute("data-base-ui-tooltip-trigger")).toBe(true); expect(trigger?.hasAttribute("disabled")).toBe(false); }); + + it("uses title as the accessible name when disabled without children", () => { + render( + , + ); + + expect(screen.getByRole("button", { name: "Go home" })).toBeTruthy(); + }); + + it("keeps the tooltip trigger mounted when disabled changes", () => { + const { container, rerender } = render( + + Home + , + ); + const trigger = container.querySelector("[data-base-ui-tooltip-trigger]"); + + expect(trigger).toBeTruthy(); + expect( + container.querySelectorAll("[data-base-ui-tooltip-trigger]"), + ).toHaveLength(1); + + rerender( + + Home + , + ); + expect(container.querySelector("[data-base-ui-tooltip-trigger]")).toBe( + trigger, + ); + expect( + container.querySelectorAll("[data-base-ui-tooltip-trigger]"), + ).toHaveLength(1); + + rerender( + + Home + , + ); + expect(container.querySelector("[data-base-ui-tooltip-trigger]")).toBe( + trigger, + ); + expect( + container.querySelectorAll("[data-base-ui-tooltip-trigger]"), + ).toHaveLength(1); + }); }); }); diff --git a/packages/kumo/src/components/button/button.tsx b/packages/kumo/src/components/button/button.tsx index a8e50b53d..49c368d13 100644 --- a/packages/kumo/src/components/button/button.tsx +++ b/packages/kumo/src/components/button/button.tsx @@ -399,7 +399,7 @@ export const Button = React.forwardRef( ); - if (title && (disabled || loading)) { + if (title) { return ( }> {button} @@ -407,10 +407,6 @@ export const Button = React.forwardRef( ); } - if (title) { - return ; - } - return button; }, ); @@ -471,31 +467,33 @@ export const LinkButton = React.forwardRef( ) => { const LinkComponent = useLinkComponent(); const emphasisStyle = getEmphasisStyle(variant); + const titleLabel = getTitleLabel(title); const externalProps = external ? { target: "_blank", rel: "noopener noreferrer" } : {}; - if (disabled) { + const linkButton = disabled ? ( // ref is intentionally not forwarded: it's typed for the anchor, but the disabled state renders a button - return ( - - ); - } - - const link = ( + + ) : ( ( ); if (title) { - return ; + return ( + }> + {linkButton} + + ); } - return link; + return linkButton; }, );