Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Slot): allow customizing prop merging behavior with the new propMergers prop #3083

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
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
2 changes: 2 additions & 0 deletions .yarn/versions/63dd769c.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
releases:
"@radix-ui/react-slot": minor
76 changes: 75 additions & 1 deletion packages/react/slot/src/Slot.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import { render, screen, fireEvent } from '@testing-library/react';
import { Slot, Slottable } from '@radix-ui/react-slot';
import { PropMergers, Slot, Slottable } from '@radix-ui/react-slot';

describe('given a slotted Trigger', () => {
describe('with onClick on itself', () => {
Expand Down Expand Up @@ -108,6 +108,80 @@ describe('given a slotted Trigger', () => {
});
});

describe('given propMergers', () => {
describe("with propMergers' onClick defined to only call slot's onClick AND className not defined", () => {
let handleSlotClick: jest.Mock;
let handleChildClick: jest.Mock;

beforeEach(() => {
handleSlotClick = jest.fn();
handleChildClick = jest.fn();

const propMergers: PropMergers = {
onClick: (slotPropValue, childPropValue) => (event) => {
if (slotPropValue) {
slotPropValue(event);
return;
}
if (childPropValue) childPropValue(event);
},
};

render(
<Slot onClick={handleSlotClick} className="slot-class" propMergers={propMergers}>
<button onClick={handleChildClick} className="child-class">
Click me
</button>
</Slot>
);

fireEvent.click(screen.getByRole('button'));
});

it('should use the custom merging behavior for onClick', () => {
expect(handleSlotClick).toHaveBeenCalledTimes(1);
expect(handleChildClick).not.toHaveBeenCalled();
});

it('should join the classNames from child and slot', () => {
expect(screen.getByRole('button')).toHaveAttribute('class', 'slot-class child-class');
});
});

describe("with propMergers' className defined in propMergers to only use slot's className AND onClick not defined", () => {
let handleSlotClick: jest.Mock;
let handleChildClick: jest.Mock;

beforeEach(() => {
handleSlotClick = jest.fn();
handleChildClick = jest.fn();

const propMergers: PropMergers = {
className: (slotPropValue) => slotPropValue,
};

render(
<Slot onClick={handleSlotClick} className="slot-class" propMergers={propMergers}>
<button onClick={handleChildClick} className="child-class">
Click me
</button>
</Slot>
);

fireEvent.click(screen.getByRole('button'));
});

it("should only use slot's className", () => {
expect(screen.getByRole('button')).toHaveAttribute('class', 'slot-class');
});

it('should call the both onClick handlers from child and slot', () => {
expect(handleSlotClick).toHaveBeenCalledTimes(1);
expect(handleChildClick).toHaveBeenCalledTimes(1);
});
});
});

describe('given a Button with Slottable', () => {
describe('without asChild', () => {
it('should render a button with icon on the left/right', async () => {
Expand Down
67 changes: 45 additions & 22 deletions packages/react/slot/src/Slot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { composeRefs } from '@radix-ui/react-compose-refs';

interface SlotProps extends React.HTMLAttributes<HTMLElement> {
children?: React.ReactNode;
propMergers?: PropMergers;
}

const Slot = React.forwardRef<HTMLElement, SlotProps>((props, forwardedRef) => {
Expand Down Expand Up @@ -55,15 +56,16 @@ Slot.displayName = 'Slot';

interface SlotCloneProps {
children: React.ReactNode;
propMergers?: PropMergers;
}

const SlotClone = React.forwardRef<any, SlotCloneProps>((props, forwardedRef) => {
const { children, ...slotProps } = props;
const { children, propMergers, ...slotProps } = props;

if (React.isValidElement(children)) {
const childrenRef = getElementRef(children);
return React.cloneElement(children, {
...mergeProps(slotProps, children.props),
...mergeProps(slotProps, children.props, { ...defaultPropMergers, ...propMergers }),
// @ts-ignore
ref: forwardedRef ? composeRefs(forwardedRef, childrenRef) : childrenRef,
});
Expand All @@ -84,39 +86,60 @@ const Slottable = ({ children }: { children: React.ReactNode }) => {

/* ---------------------------------------------------------------------------------------------- */

type CustomizableMergingPropName = Exclude<keyof React.HTMLAttributes<HTMLElement>, 'children'>;

type PropMerger<PropName extends CustomizableMergingPropName> = (
slotPropValue: React.HTMLAttributes<HTMLElement>[PropName],
childPropValue: React.HTMLAttributes<HTMLElement>[PropName]
) => React.HTMLAttributes<HTMLElement>[PropName];

type PropMergers = {
[propName in CustomizableMergingPropName]?: PropMerger<propName>;
};

type AnyProps = Record<string, any>;

const defaultPropMergers = {
style: (slotPropValue, childPropValue) => ({
...slotPropValue,
...childPropValue,
}),
className: (slotPropValue, childPropValue) =>
[slotPropValue, childPropValue].filter(Boolean).join(' '),
} as const satisfies PropMergers;

function isSlottable(child: React.ReactNode): child is React.ReactElement {
return React.isValidElement(child) && child.type === Slottable;
}

function mergeProps(slotProps: AnyProps, childProps: AnyProps) {
function mergeProps(slotProps: AnyProps, childProps: AnyProps, propMergers: PropMergers) {
// all child props should override
const overrideProps = { ...childProps };

for (const propName in childProps) {
const slotPropValue = slotProps[propName];
const childPropValue = childProps[propName];

const isHandler = /^on[A-Z]/.test(propName);
if (isHandler) {
// if the handler exists on both, we compose them
if (slotPropValue && childPropValue) {
overrideProps[propName] = (...args: unknown[]) => {
childPropValue(...args);
slotPropValue(...args);
};
if (propName in propMergers) {
overrideProps[propName] = propMergers[propName as CustomizableMergingPropName]!(
slotPropValue,
childPropValue
);
} else {
const isHandler = /^on[A-Z]/.test(propName);
if (isHandler) {
// if the handler exists on both, we compose them
if (slotPropValue && childPropValue) {
overrideProps[propName] = (...args: unknown[]) => {
childPropValue(...args);
slotPropValue(...args);
};
}
// but if it exists only on the slot, we use only this one
else if (slotPropValue) {
overrideProps[propName] = slotPropValue;
}
}
// but if it exists only on the slot, we use only this one
else if (slotPropValue) {
overrideProps[propName] = slotPropValue;
}
}
// if it's `style`, we merge them
else if (propName === 'style') {
overrideProps[propName] = { ...slotPropValue, ...childPropValue };
} else if (propName === 'className') {
overrideProps[propName] = [slotPropValue, childPropValue].filter(Boolean).join(' ');
}
}

Expand Down Expand Up @@ -155,4 +178,4 @@ export {
//
Root,
};
export type { SlotProps };
export type { SlotProps, PropMergers, PropMerger, CustomizableMergingPropName };
2 changes: 1 addition & 1 deletion packages/react/slot/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ export {
//
Root,
} from './Slot';
export type { SlotProps } from './Slot';
export type { SlotProps, PropMergers, PropMerger, CustomizableMergingPropName } from './Slot';