feat(Slot): allow customizing prop merging behavior with the new `pro…

…pMergers` prop
radix-ui · Aug 23, 2024 · be58d9c · be58d9c
1 parent 660060a
commit be58d9c
Show file tree

Hide file tree

Showing 4 changed files with 123 additions and 24 deletions.
diff --git a/.yarn/versions/63dd769c.yml b/.yarn/versions/63dd769c.yml
@@ -0,0 +1,2 @@
+releases:
+  "@radix-ui/react-slot": minor
diff --git a/packages/react/slot/src/Slot.test.tsx b/packages/react/slot/src/Slot.test.tsx
@@ -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', () => {
@@ -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 () => {

diff --git a/packages/react/slot/src/Slot.tsx b/packages/react/slot/src/Slot.tsx
@@ -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) => {
@@ -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,
     });
@@ -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(' ');
     }
   }
 
@@ -155,4 +178,4 @@ export {
   //
   Root,
 };
-export type { SlotProps };
+export type { SlotProps, PropMergers, PropMerger, CustomizableMergingPropName };
diff --git a/packages/react/slot/src/index.ts b/packages/react/slot/src/index.ts
@@ -4,4 +4,4 @@ export {
   //
   Root,
 } from './Slot';
-export type { SlotProps } from './Slot';
+export type { SlotProps, PropMergers, PropMerger, CustomizableMergingPropName } from './Slot';