diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e615f53a..dcf11f339 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,16 @@ silently discarded and never reached the DOM. These wrappers now use RAC's callback form from `ButtonProps`. This widens the accepted type, so string `className` values keep working unchanged. +#### Render-callback `style` support in react-aria-components wrappers (CORE-2710) + +`NavBarMenuItem`, `NavBarPopover`, and `TreeCheckbox` merged the caller's `style` into their +own CSS-variable object with a spread. React-aria-components types `style` as +`CSSProperties | ((renderProps) => CSSProperties)`, and spreading a function into an object +literal copies nothing, so a render-callback `style` was silently discarded — with no type +error to catch it. These wrappers now use RAC's `composeRenderProps` and merge inside a +callback, so both forms reach the DOM. The object form is unchanged, including the caller's +ability to override the wrapper's CSS variables. + ### Changed - BREAKING CHANGES #### Button Component Migration (CORE-1999) diff --git a/src/components/NavBarMenuButtons.spec.tsx b/src/components/NavBarMenuButtons.spec.tsx index 9133ba884..72c8ce745 100644 --- a/src/components/NavBarMenuButtons.spec.tsx +++ b/src/components/NavBarMenuButtons.spec.tsx @@ -8,6 +8,7 @@ import { NavBarPopoverButton, } from "./NavBarMenuButtons"; import { NavBarButton } from "./NavBarButton"; +import type { CSSPropertiesWithVariables } from "../types"; describe("NavBarPopoverButton", () => { it("matches snapshot", () => { @@ -73,36 +74,103 @@ describe("NavBarMenuItem", () => { expect(item?.className).toContain("navbar-menu-item"); expect(item?.className).toContain("caller-item"); }); + + it("merges a render-callback style", () => { + render( + + ({ color: "rgb(255, 0, 0)" })}> + Menu item + + , + ); + + const item = document.querySelector(".navbar-menu-item") as HTMLElement; + expect(item.style.color).toBe("rgb(255, 0, 0)"); + expect(item.style.getPropertyValue("--navbar-menu-item-hover-bg")).toBeTruthy(); + }); + + it("lets a render-callback style override the wrapper variables", () => { + render( + + ({ "--navbar-menu-item-hover-bg": "rebeccapurple" }) as CSSPropertiesWithVariables} + > + Menu item + + , + ); + + const item = document.querySelector(".navbar-menu-item") as HTMLElement; + expect(item.style.getPropertyValue("--navbar-menu-item-hover-bg")).toBe("rebeccapurple"); + }); + + it("keeps merging an object style, caller last", () => { + render( + + + Menu item + + , + ); + + const item = document.querySelector(".navbar-menu-item") as HTMLElement; + expect(item.style.color).toBe("rgb(0, 0, 255)"); + expect(item.style.getPropertyValue("--navbar-menu-item-hover-bg")).toBe("rebeccapurple"); + expect(item.style.getPropertyValue("--navbar-menu-item-border-color")).toBeTruthy(); + }); }); describe("NavBarPopover", () => { - it("composes a render-callback className", () => { + const renderPopover = (popoverProps: React.ComponentProps) => { render( - "caller-popover"}> + Popover content , ); - const popover = document.querySelector(".navbar-popover"); - expect(popover?.className).toContain("navbar-popover"); - expect(popover?.className).toContain("caller-popover"); + return document.querySelector(".navbar-popover") as HTMLElement; + }; + + it("composes a render-callback className", () => { + const popover = renderPopover({ className: () => "caller-popover" }); + + expect(popover.className).toContain("navbar-popover"); + expect(popover.className).toContain("caller-popover"); }); it("keeps composing a string className", () => { - render( - - - - Popover content - - , - ); + const popover = renderPopover({ className: "caller-popover" }); + + expect(popover.className).toContain("navbar-popover"); + expect(popover.className).toContain("caller-popover"); + }); + + it("merges a render-callback style", () => { + const popover = renderPopover({ style: () => ({ color: "rgb(255, 0, 0)" }) }); + + expect(popover.style.color).toBe("rgb(255, 0, 0)"); + expect(popover.style.getPropertyValue("--navbar-popover-border-color")).toBeTruthy(); + }); + + it("lets a render-callback style override the wrapper variables", () => { + const popover = renderPopover({ + style: () => ({ "--navbar-popover-border-color": "rebeccapurple" }) as CSSPropertiesWithVariables, + }); + + expect(popover.style.getPropertyValue("--navbar-popover-border-color")).toBe("rebeccapurple"); + }); + + it("keeps merging an object style, caller last", () => { + const popover = renderPopover({ + style: { color: "rgb(0, 0, 255)", "--navbar-popover-border-color": "rebeccapurple" } as CSSPropertiesWithVariables, + }); - const popover = document.querySelector(".navbar-popover"); - expect(popover?.className).toContain("navbar-popover"); - expect(popover?.className).toContain("caller-popover"); + expect(popover.style.color).toBe("rgb(0, 0, 255)"); + expect(popover.style.getPropertyValue("--navbar-popover-border-color")).toBe("rebeccapurple"); }); }); diff --git a/src/components/NavBarMenuButtons.tsx b/src/components/NavBarMenuButtons.tsx index 0c2bc16be..44aab02f9 100644 --- a/src/components/NavBarMenuButtons.tsx +++ b/src/components/NavBarMenuButtons.tsx @@ -19,11 +19,17 @@ export const NavBarMenuItem = React.forwardRef< HTMLDivElement, React.ComponentProps >(({ className, style, ...props }, ref) => { - const menuItemStyle: CSSPropertiesWithVariables = { - '--navbar-menu-item-hover-bg': colors.palette.neutralLighter, - '--navbar-menu-item-border-color': colors.palette.neutralBright, - ...style - }; + // composeRenderProps normalises the object and render-callback forms of style so a + // caller-supplied callback is merged rather than dropped. The caller still spreads last + // and can override the CSS variables set here. + const menuItemStyle = composeRenderProps( + style, + (resolvedStyle): CSSPropertiesWithVariables => ({ + '--navbar-menu-item-hover-bg': colors.palette.neutralLighter, + '--navbar-menu-item-border-color': colors.palette.neutralBright, + ...resolvedStyle + }) + ); return ( (({ className, style, ...props }, ref) => { - const popoverStyle: CSSPropertiesWithVariables = { - '--navbar-popover-border-color': colors.palette.darkGreen, - ...style - }; + const popoverStyle = composeRenderProps( + style, + (resolvedStyle): CSSPropertiesWithVariables => ({ + '--navbar-popover-border-color': colors.palette.darkGreen, + ...resolvedStyle + }) + ); return ( { it('matches snapshot', () => { @@ -61,4 +62,42 @@ describe('TreeCheckbox', () => { expect(label?.className).toContain('checkbox-label'); expect(label?.className).toContain('caller-class'); }); + + it('merges a render-callback style', () => { + render( + ({ color: 'rgb(255, 0, 0)' })}>Click Me + ); + + const label = document.querySelector('.checkbox-label') as HTMLElement; + expect(label.style.color).toBe('rgb(255, 0, 0)'); + expect(label.style.getPropertyValue('--checkbox-size')).toBe('1.6rem'); + }); + + it('lets a render-callback style override the wrapper variables', () => { + render( + ({ '--checkbox-size': '9rem' }) as CSSPropertiesWithVariables} + > + Click Me + + ); + + const label = document.querySelector('.checkbox-label') as HTMLElement; + expect(label.style.getPropertyValue('--checkbox-size')).toBe('9rem'); + }); + + it('keeps merging an object style, caller last', () => { + render( + + Click Me + + ); + + const label = document.querySelector('.checkbox-label') as HTMLElement; + expect(label.style.color).toBe('rgb(0, 0, 255)'); + expect(label.style.getPropertyValue('--checkbox-size')).toBe('9rem'); + expect(label.style.getPropertyValue('--checkbox-font-weight')).toBe('400'); + }); }); diff --git a/src/components/Tree/TreeCheckbox.tsx b/src/components/Tree/TreeCheckbox.tsx index 391dba92e..d77b5f968 100644 --- a/src/components/Tree/TreeCheckbox.tsx +++ b/src/components/Tree/TreeCheckbox.tsx @@ -11,6 +11,7 @@ import { } from "../Checkbox/sharedCheckboxStyles"; import { checkedMixIcon } from "../svgs/checkmarksvgs"; import { colors } from '../../theme'; +import { CSSPropertiesWithVariables } from "../../types"; import classNames from "classnames"; import "../Checkbox/Checkbox.css"; @@ -42,24 +43,29 @@ export const TreeCheckbox = ({ resolved )); - // Build style with CSS variables - const checkboxStyle = { - '--checkbox-font-weight': bold ? 700 : 400, - '--checkbox-color': variantStyles.color, - '--checkbox-disabled-color': colors.palette.neutralLight, - '--checkbox-size': `${size}rem`, - '--checkbox-bg-unchecked': colors.palette.white, - '--checkbox-bg': variantStyles.backgroundColor, - '--checkbox-border-unchecked': variantStyles.unCheckedBorder, - '--checkbox-border-checked': variantStyles.checkedBorder, - '--checkbox-checkmark': variantStyles.backgroundImage === 'none' ? 'none' : `url('${variantStyles.backgroundImage}')`, - '--checkbox-opacity': isDisabled ? '0.4' : '1', - '--checkbox-checked-opacity': isDisabled ? '0' : '1', - '--checkbox-disabled-border': `1px solid ${colors.palette.pale}`, - '--checkbox-indeterminate-bg': colors.palette.mediumBlue, - '--checkbox-indeterminate-icon': `url('${checkedMixIcon}')`, - ...style, - } as unknown as RACCheckboxProps['style']; // --vars are not in the type + // Build style with CSS variables. composeRenderProps normalises the object and + // render-callback forms of style so a caller-supplied callback is merged rather than + // dropped. The caller still spreads last and can override the variables set here. + const checkboxStyle = composeRenderProps( + style, + (resolvedStyle): CSSPropertiesWithVariables => ({ + '--checkbox-font-weight': bold ? 700 : 400, + '--checkbox-color': variantStyles.color, + '--checkbox-disabled-color': colors.palette.neutralLight, + '--checkbox-size': `${size}rem`, + '--checkbox-bg-unchecked': colors.palette.white, + '--checkbox-bg': variantStyles.backgroundColor, + '--checkbox-border-unchecked': variantStyles.unCheckedBorder, + '--checkbox-border-checked': variantStyles.checkedBorder, + '--checkbox-checkmark': variantStyles.backgroundImage === 'none' ? 'none' : `url('${variantStyles.backgroundImage}')`, + '--checkbox-opacity': isDisabled ? '0.4' : '1', + '--checkbox-checked-opacity': isDisabled ? '0' : '1', + '--checkbox-disabled-border': `1px solid ${colors.palette.pale}`, + '--checkbox-indeterminate-bg': colors.palette.mediumBlue, + '--checkbox-indeterminate-icon': `url('${checkedMixIcon}')`, + ...resolvedStyle, + }) + ); return (