From e0e0f5bb416a106485ca9148bad98bebf145223d Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Tue, 20 Aug 2024 13:12:16 -0400 Subject: [PATCH 1/3] remove direct `polymorphic()` calls --- .../itwinui-react/src/core/List/ListItem.tsx | 6 +++--- .../src/core/Menu/MenuDivider.tsx | 2 +- .../itwinui-react/src/core/Overlay/Overlay.tsx | 4 ++-- packages/itwinui-react/src/core/Tile/Tile.tsx | 18 ++++++++++-------- .../src/core/TransferList/TransferList.tsx | 4 ++-- .../src/utils/functions/polymorphic.test.tsx | 18 ------------------ .../src/utils/functions/polymorphic.tsx | 12 ++++-------- 7 files changed, 22 insertions(+), 42 deletions(-) diff --git a/packages/itwinui-react/src/core/List/ListItem.tsx b/packages/itwinui-react/src/core/List/ListItem.tsx index 2613baea238..36ba06ef69e 100644 --- a/packages/itwinui-react/src/core/List/ListItem.tsx +++ b/packages/itwinui-react/src/core/List/ListItem.tsx @@ -68,21 +68,21 @@ export type ListItemOwnProps = { // ---------------------------------------------------------------------------- -const ListItemIcon = polymorphic('iui-list-item-icon'); +const ListItemIcon = polymorphic.div('iui-list-item-icon'); if (process.env.NODE_ENV === 'development') { ListItemIcon.displayName = 'ListItem.Icon'; } // ---------------------------------------------------------------------------- -const ListItemContent = polymorphic('iui-list-item-content'); +const ListItemContent = polymorphic.div('iui-list-item-content'); if (process.env.NODE_ENV === 'development') { ListItemContent.displayName = 'ListItem.Content'; } // ---------------------------------------------------------------------------- -const ListItemDescription = polymorphic('iui-list-item-description'); +const ListItemDescription = polymorphic.div('iui-list-item-description'); if (process.env.NODE_ENV === 'development') { ListItemDescription.displayName = 'ListItem.Description'; } diff --git a/packages/itwinui-react/src/core/Menu/MenuDivider.tsx b/packages/itwinui-react/src/core/Menu/MenuDivider.tsx index adf34845a84..bc5037a1b30 100644 --- a/packages/itwinui-react/src/core/Menu/MenuDivider.tsx +++ b/packages/itwinui-react/src/core/Menu/MenuDivider.tsx @@ -19,7 +19,7 @@ import { polymorphic } from '../../utils/index.js'; * ]} * */ -export const MenuDivider = polymorphic('iui-menu-divider', { +export const MenuDivider = polymorphic.div('iui-menu-divider', { role: 'separator', }); if (process.env.NODE_ENV === 'development') { diff --git a/packages/itwinui-react/src/core/Overlay/Overlay.tsx b/packages/itwinui-react/src/core/Overlay/Overlay.tsx index a30b5336675..0a4754fd4ca 100644 --- a/packages/itwinui-react/src/core/Overlay/Overlay.tsx +++ b/packages/itwinui-react/src/core/Overlay/Overlay.tsx @@ -52,14 +52,14 @@ if (process.env.NODE_ENV === 'development') { // -------------------------------------------------------------------------------- -const OverlayOverlay = polymorphic('iui-overlay'); +const OverlayOverlay = polymorphic.div('iui-overlay'); if (process.env.NODE_ENV === 'development') { OverlayOverlay.displayName = 'Overlay.Overlay'; } // -------------------------------------------------------------------------------- -const OverlayWrapper = polymorphic('iui-overlay-wrapper'); +const OverlayWrapper = polymorphic.div('iui-overlay-wrapper'); if (process.env.NODE_ENV === 'development') { OverlayWrapper.displayName = 'Overlay.Wrapper'; } diff --git a/packages/itwinui-react/src/core/Tile/Tile.tsx b/packages/itwinui-react/src/core/Tile/Tile.tsx index fefac749d51..fb40292102c 100644 --- a/packages/itwinui-react/src/core/Tile/Tile.tsx +++ b/packages/itwinui-react/src/core/Tile/Tile.tsx @@ -186,7 +186,7 @@ if (process.env.NODE_ENV === 'development') { // ---------------------------------------------------------------------------- // Tile.ThumbnailArea component -const TileThumbnailArea = polymorphic('iui-tile-thumbnail'); +const TileThumbnailArea = polymorphic.div('iui-tile-thumbnail'); if (process.env.NODE_ENV === 'development') { TileThumbnailArea.displayName = 'Tile.ThumbnailArea'; } @@ -234,7 +234,7 @@ if (process.env.NODE_ENV === 'development') { // ---------------------------------------------------------------------------- // Tile.QuickAction component -const TileQuickAction = polymorphic('iui-tile-thumbnail-quick-action'); +const TileQuickAction = polymorphic.div('iui-tile-thumbnail-quick-action'); if (process.env.NODE_ENV === 'development') { TileQuickAction.displayName = 'Tile.QuickAction'; } @@ -242,7 +242,7 @@ if (process.env.NODE_ENV === 'development') { // ---------------------------------------------------------------------------- // Tile.TypeIndicator component -const TileTypeIndicator = polymorphic('iui-tile-thumbnail-type-indicator'); +const TileTypeIndicator = polymorphic.div('iui-tile-thumbnail-type-indicator'); if (process.env.NODE_ENV === 'development') { TileTypeIndicator.displayName = 'Tile.TypeIndicator'; } @@ -274,7 +274,9 @@ if (process.env.NODE_ENV === 'development') { // ---------------------------------------------------------------------------- // Tile.BadgeContainer component -const TileBadgeContainer = polymorphic('iui-tile-thumbnail-badge-container'); +const TileBadgeContainer = polymorphic.div( + 'iui-tile-thumbnail-badge-container', +); if (process.env.NODE_ENV === 'development') { TileBadgeContainer.displayName = 'Tile.BadgeContainer'; } @@ -349,7 +351,7 @@ if (process.env.NODE_ENV === 'development') { // ---------------------------------------------------------------------------- // Tile.ContentArea component -const TileContentArea = polymorphic('iui-tile-content'); +const TileContentArea = polymorphic.div('iui-tile-content'); if (process.env.NODE_ENV === 'development') { TileContentArea.displayName = 'Tile.ContentArea'; } @@ -357,7 +359,7 @@ if (process.env.NODE_ENV === 'development') { // ---------------------------------------------------------------------------- // Tile.Description component -const TileDescription = polymorphic('iui-tile-description'); +const TileDescription = polymorphic.div('iui-tile-description'); if (process.env.NODE_ENV === 'development') { TileDescription.displayName = 'Tile.Description'; } @@ -365,7 +367,7 @@ if (process.env.NODE_ENV === 'development') { // ---------------------------------------------------------------------------- // Tile.Metadata component -const TileMetadata = polymorphic('iui-tile-metadata'); +const TileMetadata = polymorphic.div('iui-tile-metadata'); if (process.env.NODE_ENV === 'development') { TileMetadata.displayName = 'Tile.Metadata'; } @@ -425,7 +427,7 @@ if (process.env.NODE_ENV === 'development') { // ---------------------------------------------------------------------------- // Tile.Buttons component -const TileButtons = polymorphic('iui-tile-buttons'); +const TileButtons = polymorphic.div('iui-tile-buttons'); if (process.env.NODE_ENV === 'development') { TileButtons.displayName = 'Tile.Buttons'; } diff --git a/packages/itwinui-react/src/core/TransferList/TransferList.tsx b/packages/itwinui-react/src/core/TransferList/TransferList.tsx index 7e084ac774d..2c85f7689c6 100644 --- a/packages/itwinui-react/src/core/TransferList/TransferList.tsx +++ b/packages/itwinui-react/src/core/TransferList/TransferList.tsx @@ -20,7 +20,7 @@ import { Label } from '../Label/Label.js'; // ---------------------------------------------------------------------------- // TransferListComponent -const TransferListComponent = polymorphic('iui-transfer-list-wrapper'); +const TransferListComponent = polymorphic.div('iui-transfer-list-wrapper'); if (process.env.NODE_ENV === 'development') { TransferListComponent.displayName = 'TransferList'; } @@ -226,7 +226,7 @@ if (process.env.NODE_ENV === 'development') { // ---------------------------------------------------------------------------- // TransferList.Toolbar component -const TransferListToolbar = polymorphic('iui-transfer-list-toolbar', { +const TransferListToolbar = polymorphic.div('iui-transfer-list-toolbar', { role: 'toolbar', }); if (process.env.NODE_ENV === 'development') { diff --git a/packages/itwinui-react/src/utils/functions/polymorphic.test.tsx b/packages/itwinui-react/src/utils/functions/polymorphic.test.tsx index 629b2e9a35e..3504800cfc0 100644 --- a/packages/itwinui-react/src/utils/functions/polymorphic.test.tsx +++ b/packages/itwinui-react/src/utils/functions/polymorphic.test.tsx @@ -5,24 +5,6 @@ import { render } from '@testing-library/react'; import { polymorphic } from './polymorphic.js'; -it('should work when called directly', () => { - const MyDiv = polymorphic('my-div'); - - const { container: container1 } = render(🍏); - const el1 = container1.querySelector('div.my-div') as HTMLElement; - expect(el1).toHaveTextContent('🍏'); - expect(el1).toHaveAttribute('data-testid', 'foo'); - - const { container: container2 } = render( - - 🥭 - , - ); - const el2 = container2.querySelector('span.my-div') as HTMLElement; - expect(el2).toHaveTextContent('🥭'); - expect(el2).toHaveAttribute('data-testid', 'bar'); -}); - it('should work when called as property', () => { const MyButton = polymorphic.button('my-button'); diff --git a/packages/itwinui-react/src/utils/functions/polymorphic.tsx b/packages/itwinui-react/src/utils/functions/polymorphic.tsx index 1c78436abfa..4f9a2a49400 100644 --- a/packages/itwinui-react/src/utils/functions/polymorphic.tsx +++ b/packages/itwinui-react/src/utils/functions/polymorphic.tsx @@ -49,8 +49,8 @@ const _base = ( /** * Utility to create a type-safe polymorphic component with a simple class. * - * Can be called directly or as a property of the `polymorphic` object. - * In both cases, returns a component that: + * Can be called as a property of the `polymorphic` object. + * Returns a component that: * - uses CSS-modules scoped classes * - supports `as` prop with default element * - forwards ref and spreads rest props @@ -58,16 +58,12 @@ const _base = ( * - adds tabIndex to interactive elements (Safari workaround) * * @example - * const MyPolyDiv = polymorphic('my-poly-div'); - * ...; - * - * @example * const MyPolyButton = polymorphic.button('my-poly-button', { type: 'button' }); * ...; * * @private */ -export const polymorphic = new Proxy(_base('div'), { +export const polymorphic = new Proxy({} as never, { get: (target, prop) => { if (typeof prop === 'string') { // eslint-disable-next-line -- string is as far as we can narrow it down @@ -76,7 +72,7 @@ export const polymorphic = new Proxy(_base('div'), { } return Reflect.get(target, prop); }, -}) as ReturnType & { +}) as { [key in keyof JSX.IntrinsicElements]: ReturnType>; }; From 7af39892870d941ac891615f17ec313db32a12fa Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Tue, 20 Aug 2024 15:12:14 -0400 Subject: [PATCH 2/3] allow chaining polymorphic components --- .../itwinui-react/src/core/Menu/MenuItem.tsx | 32 ++++++-------- .../src/utils/functions/polymorphic.test.tsx | 26 +++++++++++ .../src/utils/functions/polymorphic.tsx | 44 ++++++++++++++++--- 3 files changed, 76 insertions(+), 26 deletions(-) diff --git a/packages/itwinui-react/src/core/Menu/MenuItem.tsx b/packages/itwinui-react/src/core/Menu/MenuItem.tsx index a6c51494f04..c62d59b499e 100644 --- a/packages/itwinui-react/src/core/Menu/MenuItem.tsx +++ b/packages/itwinui-react/src/core/Menu/MenuItem.tsx @@ -8,12 +8,13 @@ import { useMergedRefs, useId, createWarningLogger, + ButtonBase, + polymorphic, } from '../../utils/index.js'; import type { PolymorphicForwardRefComponent } from '../../utils/index.js'; import { Menu, MenuContext } from './Menu.js'; import { ListItem } from '../List/ListItem.js'; import type { ListItemOwnProps } from '../List/ListItem.js'; -import cx from 'classnames'; const logWarning = createWarningLogger(); @@ -83,7 +84,6 @@ export type MenuItemProps = { */ export const MenuItem = React.forwardRef((props, forwardedRef) => { const { - className, children, isSelected, disabled, @@ -129,15 +129,7 @@ export const MenuItem = React.forwardRef((props, forwardedRef) => { } satisfies Parameters[0]['popoverProps']; }, [subMenuItems.length]); - const onClick = () => { - if (disabled) { - return; - } - - onClickProp?.(value); - }; - - const handlers: React.DOMAttributes = { onClick }; + const onClick = () => onClickProp?.(value); /** Index of this item out of all the focusable items in the parent `Menu` */ const focusableItemIndex = parentMenu?.focusableElements.findIndex( @@ -145,10 +137,7 @@ export const MenuItem = React.forwardRef((props, forwardedRef) => { ); const trigger = ( - { {...(parentMenu?.popoverGetItemProps != null ? parentMenu.popoverGetItemProps({ focusableItemIndex, - userProps: handlers, + userProps: { onClick }, }) - : handlers)} - {...(rest as React.DOMAttributes)} + : { onClick })} + {...(rest as React.DOMAttributes)} > {startIcon && ( @@ -187,7 +176,7 @@ export const MenuItem = React.forwardRef((props, forwardedRef) => { {endIcon} )} - + ); return ( @@ -206,6 +195,11 @@ if (process.env.NODE_ENV === 'development') { MenuItem.displayName = 'MenuItem'; } +const MenuItemBase = polymorphic( + ButtonBase, + ListItem, +) as PolymorphicForwardRefComponent<'button', ListItemOwnProps>; + // ---------------------------------------------------------------------------- export type TreeEvent = { diff --git a/packages/itwinui-react/src/utils/functions/polymorphic.test.tsx b/packages/itwinui-react/src/utils/functions/polymorphic.test.tsx index 3504800cfc0..d549bb2e4fa 100644 --- a/packages/itwinui-react/src/utils/functions/polymorphic.test.tsx +++ b/packages/itwinui-react/src/utils/functions/polymorphic.test.tsx @@ -4,6 +4,32 @@ *--------------------------------------------------------------------------------------------*/ import { render } from '@testing-library/react'; import { polymorphic } from './polymorphic.js'; +import type { PolymorphicForwardRefComponent } from '../props.js'; + +it('should work when called directly', () => { + const MyDiv1 = polymorphic.div('my-div1'); + const MyDiv2 = polymorphic.div('my-div2'); + const MyDiv = polymorphic( + MyDiv1, + MyDiv2, + ) as PolymorphicForwardRefComponent<'div'>; + + const { container: container1 } = render(🍏); + const el1 = container1.querySelector('div') as HTMLElement; + expect(el1).toHaveClass('my-div1', 'my-div2'); + expect(el1).toHaveTextContent('🍏'); + expect(el1).toHaveAttribute('data-testid', 'foo'); + + const { container: container2 } = render( + + 🥭 + , + ); + const el2 = container2.querySelector('span') as HTMLElement; + expect(el2).toHaveClass('my-div1', 'my-div2'); + expect(el2).toHaveTextContent('🥭'); + expect(el2).toHaveAttribute('data-testid', 'bar'); +}); it('should work when called as property', () => { const MyButton = polymorphic.button('my-button'); diff --git a/packages/itwinui-react/src/utils/functions/polymorphic.tsx b/packages/itwinui-react/src/utils/functions/polymorphic.tsx index 4f9a2a49400..542e4956725 100644 --- a/packages/itwinui-react/src/utils/functions/polymorphic.tsx +++ b/packages/itwinui-react/src/utils/functions/polymorphic.tsx @@ -8,7 +8,7 @@ import type { PolymorphicForwardRefComponent } from '../props.js'; import { useGlobals } from '../hooks/useGlobals.js'; import { styles } from '../../styles.js'; -const _base = ( +const _elementBase = ( defaultElement: As, ) => { return (className: string, attrs?: JSX.IntrinsicElements[As]) => { @@ -21,6 +21,16 @@ const _base = ( ), }; + useGlobals(); + + if (Array.isArray(as as any)) { + if (as.length > 1) { + const [Component, ...restAs] = as; + return ; + } + (as as any) = as[0]; + } + const Element = (as as any) || 'div'; // Add tabIndex to interactive elements if not already set. @@ -33,8 +43,6 @@ const _base = ( props.tabIndex ??= 0; } - useGlobals(); - return ; }) as PolymorphicForwardRefComponent>; @@ -46,10 +54,24 @@ const _base = ( }; }; +const _componentBase = ( + ...components: PolymorphicForwardRefComponent[] +) => { + const Comp = _elementBase('div')(''); + return React.forwardRef((props, ref) => ( + + )) as PolymorphicForwardRefComponent<'div'>; +}; + /** * Utility to create a type-safe polymorphic component with a simple class. * * Can be called as a property of the `polymorphic` object. + * * Returns a component that: * - uses CSS-modules scoped classes * - supports `as` prop with default element @@ -57,23 +79,31 @@ const _base = ( * - adds and merges CSS classes * - adds tabIndex to interactive elements (Safari workaround) * + * Alternatively, it can be called directly with multiple components to create a chain of + * components. This is useful for creating wrapper components that still need to support + * user-specified `as` prop. **Note:** All components in the chain must be polymorphic. + * * @example * const MyPolyButton = polymorphic.button('my-poly-button', { type: 'button' }); * ...; * + * @example + * const MyMenuButton = polymorphic(MyPolyButton, MyMenu) as PolymorphicForwardRefComponent<'button'>; + * ...; + * * @private */ -export const polymorphic = new Proxy({} as never, { +export const polymorphic = new Proxy(_componentBase, { get: (target, prop) => { if (typeof prop === 'string') { // eslint-disable-next-line -- string is as far as we can narrow it down // @ts-ignore - return _base(prop); + return _elementBase(prop); } return Reflect.get(target, prop); }, -}) as { - [key in keyof JSX.IntrinsicElements]: ReturnType>; +}) as typeof _componentBase & { + [key in keyof JSX.IntrinsicElements]: ReturnType>; }; // e.g. iui-list-item-icon -> ListItemIcon From 16e8422f7133eac1cb74892b429546af08536ba0 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Tue, 20 Aug 2024 15:30:06 -0400 Subject: [PATCH 3/3] words --- packages/itwinui-react/src/utils/functions/polymorphic.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/itwinui-react/src/utils/functions/polymorphic.tsx b/packages/itwinui-react/src/utils/functions/polymorphic.tsx index 4f9a2a49400..5b4aa0f894f 100644 --- a/packages/itwinui-react/src/utils/functions/polymorphic.tsx +++ b/packages/itwinui-react/src/utils/functions/polymorphic.tsx @@ -49,7 +49,7 @@ const _base = ( /** * Utility to create a type-safe polymorphic component with a simple class. * - * Can be called as a property of the `polymorphic` object. + * Should be called as a property of the `polymorphic` object. * Returns a component that: * - uses CSS-modules scoped classes * - supports `as` prop with default element