diff --git a/.changeset/nice-crabs-care.md b/.changeset/nice-crabs-care.md new file mode 100644 index 00000000000..c5130a3ab1e --- /dev/null +++ b/.changeset/nice-crabs-care.md @@ -0,0 +1,6 @@ +--- +"@jpmorganchase/uitk-lab": minor +--- + +Improve types for forwardCallbackProps +Ensure Dropdown callback props are merged diff --git a/packages/lab/src/__tests__/__e2e__/dropdown/Dropdown.keyboardNavigation.cy.tsx b/packages/lab/src/__tests__/__e2e__/dropdown/Dropdown.keyboardNavigation.cy.tsx index dab01ccb6bb..7a087393750 100644 --- a/packages/lab/src/__tests__/__e2e__/dropdown/Dropdown.keyboardNavigation.cy.tsx +++ b/packages/lab/src/__tests__/__e2e__/dropdown/Dropdown.keyboardNavigation.cy.tsx @@ -1,4 +1,8 @@ -import { Dropdown, SelectionStrategy } from "@jpmorganchase/uitk-lab"; +import { + Dropdown, + DropdownButton, + SelectionStrategy, +} from "@jpmorganchase/uitk-lab"; /** * Changes applied @@ -497,5 +501,30 @@ const testSource = ["Bar", "Foo", "Foo Bar", "Baz"]; cy.get("#test-popup").should("not.exist"); }); }); + + describe("Given a Dropdown with custom trigger component", () => { + it("Should respect callback props on cloned custom trigger", () => { + const keyDownSpy = cy.stub().as("keyDownSpy"); + cy.mount( + + } + > + ); + cy.get("#custom-button").focus(); + cy.realPress("ArrowDown"); + cy.get("@keyDownSpy").should("have.callCount", 1); + cy.get("#test-popup").should("exist"); + }); + }); }); }); diff --git a/packages/lab/src/dropdown/Dropdown.tsx b/packages/lab/src/dropdown/Dropdown.tsx index d5104fd3b50..6397c739ec7 100644 --- a/packages/lab/src/dropdown/Dropdown.tsx +++ b/packages/lab/src/dropdown/Dropdown.tsx @@ -19,10 +19,11 @@ import { } from "../common-hooks"; import { List } from "../list/List"; import { ListProps } from "../list/listTypes"; -import { DropdownBase } from "./DropdownBase"; +import { DropdownBase, MaybeChildProps } from "./DropdownBase"; import { DropdownButton } from "./DropdownButton"; import { DropdownBaseProps } from "./dropdownTypes"; import { useDropdown } from "./useDropdown"; +import { forwardCallbackProps } from "../utils"; export interface DropdownProps< Item = "string", @@ -119,8 +120,6 @@ export const Dropdown = forwardRef(function Dropdown< return itemOrItems.map((i) => i.value) as returnType; } else if (itemOrItems) { return itemOrItems.value as returnType; - } else { - return null as returnType; } }, [] @@ -134,10 +133,14 @@ export const Dropdown = forwardRef(function Dropdown< "aria-label": ariaLabel, }; if (triggerComponent) { - return cloneElement(triggerComponent, { - ...listControlProps, - ...ariaProps, - }); + const ownProps = triggerComponent.props as MaybeChildProps; + return cloneElement( + triggerComponent, + forwardCallbackProps(ownProps, { + ...listControlProps, + ...ariaProps, + }) + ); } else { return ( ( id = defaultId, role = defaultRole, ...ownProps - } = trigger.props as MaybeProps; + } = trigger.props as MaybeChildProps; return cloneElement( trigger, @@ -145,7 +145,7 @@ export const DropdownBase = forwardRef( id = defaultId, width: ownWidth, ...ownProps - } = popupComponent.props as MaybeProps; + } = popupComponent.props as MaybeChildProps; return cloneElement(popupComponent, { ...ownProps, ...restComponentProps, diff --git a/packages/lab/src/list/List.tsx b/packages/lab/src/list/List.tsx index 63b4ae97d6a..b3600f649ce 100644 --- a/packages/lab/src/list/List.tsx +++ b/packages/lab/src/list/List.tsx @@ -19,7 +19,6 @@ import { useCollectionItems, useImperativeScrollingAPI, } from "../common-hooks"; -import { forwardCallbackProps } from "../utils"; import { ListItem as DefaultListItem, ListItemProxy } from "./ListItem"; import { ListItemProps, ListProps } from "./listTypes"; @@ -223,7 +222,7 @@ export const List = forwardRef(function List< }; list.push( isChildItem ? ( - cloneElement(value, forwardCallbackProps(value.props, listItemProps)) + cloneElement(value, listItemProps) ) : ( ) diff --git a/packages/lab/src/utils/forwardCallbackProps.ts b/packages/lab/src/utils/forwardCallbackProps.ts index 72d6dbd741a..8648fd41bdc 100644 --- a/packages/lab/src/utils/forwardCallbackProps.ts +++ b/packages/lab/src/utils/forwardCallbackProps.ts @@ -1,31 +1,33 @@ -/* +/* When we clone a React element and inject props, if any of these are - callback props, make sure original callback props are also invoked. - + callback props, make sure original callback props are also invoked. + React.cloneElement( element, forwardCallbackProps(element.props, overrideProps) ) */ -export const forwardCallbackProps = (elementProps: any, overrideProps: any) => { - const callbackProps = Object.entries(elementProps).filter( - ([, value]) => typeof value === "function" + +type Props = Record; + +export const forwardCallbackProps = ( + ownProps: P1, + overrideProps: P2 +): P1 & P2 => { + const props = Object.keys(ownProps).reduce( + (map, name) => { + const ownProp = ownProps[name]; + const overrideProp = overrideProps[name]; + if (typeof ownProp === "function" && typeof overrideProp === "function") { + map[name] = (...args: unknown[]) => { + ownProp(...args); + overrideProp(...args); + }; + } + return map; + }, + { ...overrideProps } ); - if (callbackProps.some(([name]) => overrideProps[name] !== undefined)) { - return { - ...overrideProps, - ...callbackProps.reduce((map, [name, fn]) => { - if (overrideProps[name] && typeof overrideProps[name] === "function") { - map[name] = (...args: any) => { - overrideProps[name]?.(...args); - fn(...args); - }; - } - return map; - }, {} as any), - }; - } else { - return overrideProps; - } + return props as P1 & P2; };