Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
100 changes: 84 additions & 16 deletions src/components/NavBarMenuButtons.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
NavBarPopoverButton,
} from "./NavBarMenuButtons";
import { NavBarButton } from "./NavBarButton";
import type { CSSPropertiesWithVariables } from "../types";

describe("NavBarPopoverButton", () => {
it("matches snapshot", () => {
Expand Down Expand Up @@ -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(
<Menu aria-label="Test menu">
<NavBarMenuItem style={() => ({ color: "rgb(255, 0, 0)" })}>
Menu item
</NavBarMenuItem>
</Menu>,
);

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(
<Menu aria-label="Test menu">
<NavBarMenuItem
style={() => ({ "--navbar-menu-item-hover-bg": "rebeccapurple" }) as CSSPropertiesWithVariables}
>
Menu item
</NavBarMenuItem>
</Menu>,
);

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 aria-label="Test menu">
<NavBarMenuItem
style={{ color: "rgb(0, 0, 255)", "--navbar-menu-item-hover-bg": "rebeccapurple" } as CSSPropertiesWithVariables}
>
Menu item
</NavBarMenuItem>
</Menu>,
);

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<typeof NavBarPopover>) => {
render(
<DialogTrigger defaultOpen>
<NavBarButton label="Test menu" />
<NavBarPopover className={() => "caller-popover"}>
<NavBarPopover {...popoverProps}>
<Dialog aria-label="Test dialog">Popover content</Dialog>
</NavBarPopover>
</DialogTrigger>,
);

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(
<DialogTrigger defaultOpen>
<NavBarButton label="Test menu" />
<NavBarPopover className="caller-popover">
<Dialog aria-label="Test dialog">Popover content</Dialog>
</NavBarPopover>
</DialogTrigger>,
);
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");
});
});
27 changes: 18 additions & 9 deletions src/components/NavBarMenuButtons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,17 @@ export const NavBarMenuItem = React.forwardRef<
HTMLDivElement,
React.ComponentProps<typeof MenuItem>
>(({ 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 (
<MenuItem
Expand All @@ -48,10 +54,13 @@ export const NavBarPopover = React.forwardRef<
HTMLDivElement,
PopoverProps
>(({ 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 (
<Popover
Expand Down
39 changes: 39 additions & 0 deletions src/components/Tree/TreeCheckbox.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { render } from '@testing-library/react';
import { TreeCheckbox } from './TreeCheckbox';
import renderer from 'react-test-renderer';
import type { CSSPropertiesWithVariables } from '../../types';

describe('TreeCheckbox', () => {
it('matches snapshot', () => {
Expand Down Expand Up @@ -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(
<TreeCheckbox style={() => ({ color: 'rgb(255, 0, 0)' })}>Click Me</TreeCheckbox>
);

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(
<TreeCheckbox
style={() => ({ '--checkbox-size': '9rem' }) as CSSPropertiesWithVariables}
>
Click Me
</TreeCheckbox>
);

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(
<TreeCheckbox
style={{ color: 'rgb(0, 0, 255)', '--checkbox-size': '9rem' } as CSSPropertiesWithVariables}
>
Click Me
</TreeCheckbox>
);

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');
});
});
42 changes: 24 additions & 18 deletions src/components/Tree/TreeCheckbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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 (
<RACCheckbox
Expand Down
Loading