restore old Breadcrumbs api (#1557)

.changeset/soft-crabs-flow.md

@@ -1,5 +1,5 @@
 ---
-'@itwin/itwinui-react': major
+'@itwin/itwinui-react': minor
 ---
 
-Added new `Breadcrumbs.Item` subcomponent to use for custom `a` and `button` elements within the `Breadcrumbs`
+Added new `Breadcrumbs.Item` subcomponent to use within `Breadcrumbs`. Directly passing `<a>`/`<Button>`/`<span>` as children is still supported but is deprecated.

packages/itwinui-react/src/core/Breadcrumbs/Breadcrumbs.test.tsx

@@ -3,12 +3,14 @@
  * See LICENSE.md in the project root for license terms and full copyright notice.
  *--------------------------------------------------------------------------------------------*/
 import * as React from 'react';
-import { fireEvent, render } from '@testing-library/react';
+import { fireEvent, render, screen } from '@testing-library/react';
 
 import { Breadcrumbs } from './Breadcrumbs.js';
 import { SvgChevronRight, SvgMore } from '../utils/index.js';
 import * as UseOverflow from '../utils/hooks/useOverflow.js';
 import { IconButton } from '../Buttons/IconButton/index.js';
+import { Button } from '../Buttons/index.js';
+import userEvent from '@testing-library/user-event';
 
 const renderComponent = (
   props?: Partial<React.ComponentProps<typeof Breadcrumbs>>,
@@ -160,3 +162,48 @@ it('should show the last item when only one can be visible', () => {
   expect(breadcrumbs[0].firstElementChild?.textContent).toContain('…');
   expect(breadcrumbs[1].textContent).toEqual('Item 2');
 });
+
+it('should support legacy api', async () => {
+  const onClick = jest.fn();
+
+  render(
+    <div data-testid='1'>
+      <Breadcrumbs>
+        <Button onClick={onClick}>Item 1</Button>
+        <a href='#'>Item 2</a>
+        <span>Item 3</span>
+      </Breadcrumbs>
+    </div>,
+  );
+  render(
+    <div data-testid='2'>
+      <Breadcrumbs>
+        <Breadcrumbs.Item onClick={onClick}>Item 1</Breadcrumbs.Item>
+        <Breadcrumbs.Item href='#'>Item 2</Breadcrumbs.Item>
+        <Breadcrumbs.Item>Item 3</Breadcrumbs.Item>
+      </Breadcrumbs>
+    </div>,
+  );
+
+  expect(screen.getByTestId('1').innerHTML).toEqual(
+    screen.getByTestId('2').innerHTML,
+  );
+
+  await userEvent.click(
+    screen.getByTestId('1').querySelector('button') as HTMLElement,
+  );
+  expect(onClick).toHaveBeenCalledTimes(1);
+  await userEvent.click(
+    screen.getByTestId('2').querySelector('button') as HTMLElement,
+  );
+  expect(onClick).toHaveBeenCalledTimes(2);
+
+  expect(screen.getByTestId('1').querySelector('a')).toHaveAttribute(
+    'href',
+    '#',
+  );
+  expect(screen.getByTestId('2').querySelector('a')).toHaveAttribute(
+    'href',
+    '#',
+  );
+});

packages/itwinui-react/src/core/Breadcrumbs/Breadcrumbs.tsx

@@ -9,8 +9,12 @@ import {
   useOverflow,
   SvgChevronRight,
   Box,
+  createWarningLogger,
 } from '../utils/index.js';
 import type { PolymorphicForwardRefComponent } from '../utils/index.js';
+import { Button } from '../Buttons/Button/index.js';
+
+const logWarningInDev = createWarningLogger();
 
 type BreadcrumbsProps = {
   /**
@@ -179,31 +183,50 @@ const BreadcrumbsComponent = React.forwardRef((props, ref) => {
   );
 }) as PolymorphicForwardRefComponent<'nav', BreadcrumbsProps>;
 
+// ----------------------------------------------------------------------------
+
 const ListItem = ({
   item,
   isActive,
 }: {
   item: React.ReactNode;
   isActive: boolean;
 }) => {
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  let children = item as any;
+
+  if (
+    children?.type === 'span' ||
+    children?.type === 'a' ||
+    children?.type === Button
+  ) {
+    logWarningInDev(
+      'Directly using Button/a/span as Breadcrumbs children is deprecated, please use `Breadcrumbs.Item` instead.',
+    );
+    children = <BreadcrumbsItem {...children.props} />;
+  }
+
   return (
     <Box as='li' className={'iui-breadcrumbs-item'}>
-      {React.isValidElement(item)
-        ? React.cloneElement(item as JSX.Element, {
-            'aria-current':
-              item.props['aria-current'] ?? isActive ? 'location' : undefined,
-          })
-        : item}
+      {children &&
+        React.cloneElement(children, {
+          'aria-current':
+            children.props['aria-current'] ?? isActive ? 'location' : undefined,
+        })}
     </Box>
   );
 };
 
+// ----------------------------------------------------------------------------
+
 const Separator = ({ separator }: Pick<BreadcrumbsProps, 'separator'>) => (
   <Box as='li' className='iui-breadcrumbs-separator' aria-hidden>
     {separator ?? <SvgChevronRight />}
   </Box>
 );
 
+// ----------------------------------------------------------------------------
+
 const BreadcrumbsItem = React.forwardRef((props, forwardedRef) => {
   const { children: childrenProp, className, ...rest } = props;
 
@@ -224,6 +247,8 @@ const BreadcrumbsItem = React.forwardRef((props, forwardedRef) => {
 }) as PolymorphicForwardRefComponent<'a'>;
 BreadcrumbsItem.displayName = 'Breadcrumbs.Item';
 
+// ----------------------------------------------------------------------------
+
 export const Breadcrumbs = Object.assign(BreadcrumbsComponent, {
   /**
    * Breadcrumbs item subcomponent

packages/itwinui-react/src/core/Table/Table.tsx

@@ -34,6 +34,7 @@ import {
   SvgSortUp,
   useIsomorphicLayoutEffect,
   Box,
+  createWarningLogger,
 } from '../utils/index.js';
 import type { CommonProps } from '../utils/index.js';
 import { getCellStyle, getStickyStyle } from './utils.js';
@@ -68,13 +69,7 @@ const shiftRowSelectedAction = 'shiftRowSelected';
 export const tableResizeStartAction = 'tableResizeStart';
 const tableResizeEndAction = 'tableResizeEnd';
 
-let didLogWarning = false;
-let isDev = false;
-
-// wrapping in try-catch because process might be undefined
-try {
-  isDev = process.env.NODE_ENV !== 'production';
-} catch {}
+const logWarningInDev = createWarningLogger();
 
 export type TablePaginatorRendererProps = {
   /**
@@ -631,12 +626,9 @@ export const Table = <
 
   if (columns.length === 1 && 'columns' in columns[0]) {
     headerGroups = _headerGroups.slice(1);
-    if (isDev && !didLogWarning) {
-      console.warn(
-        `Table's \`columns\` prop should not have a top-level \`Header\` or sub-columns. They are only allowed to be passed for backwards compatibility.\n See https://github.com/iTwin/iTwinUI/wiki/iTwinUI-react-v2-migration-guide#breaking-changes`,
-      );
-      didLogWarning = true;
-    }
+    logWarningInDev(
+      `Table's \`columns\` prop should not have a top-level \`Header\` or sub-columns. They are only allowed to be passed for backwards compatibility.\n See https://github.com/iTwin/iTwinUI/wiki/iTwinUI-react-v2-migration-guide#breaking-changes`,
+    );
   }
 
   const ariaDataAttributes = Object.entries(rest).reduce(

packages/itwinui-react/src/core/utils/functions/dev.ts

@@ -0,0 +1,33 @@
+/*---------------------------------------------------------------------------------------------
+ * Copyright (c) Bentley Systems, Incorporated. All rights reserved.
+ * See LICENSE.md in the project root for license terms and full copyright notice.
+ *--------------------------------------------------------------------------------------------*/
+const isJest = typeof jest !== 'undefined';
+
+let isDev = false;
+
+// wrapping in try-catch because process might be undefined
+try {
+  isDev = process.env.NODE_ENV !== 'production' && !isJest;
+} catch {}
+
+/**
+ * Logs message one time only in dev environments.
+ *
+ * @example
+ * const logWarningInDev = createWarningLogger();
+ * logWarningInDev("please don't use this")
+ */
+const createWarningLogger = !isDev
+  ? () => () => {}
+  : () => {
+      let logged = false;
+      return (message: string) => {
+        if (!logged) {
+          console.warn(message);
+          logged = true;
+        }
+      };
+    };
+
+export { isJest, isDev, createWarningLogger };

packages/itwinui-react/src/core/utils/functions/index.ts

@@ -11,3 +11,4 @@ export * from './supports.js';
 export * from './polymorphic.js';
 export * from './import.js';
 export * from './react.js';
+export * from './dev.js';

packages/itwinui-react/src/core/utils/hooks/useGlobals.ts

@@ -4,13 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 import * as React from 'react';
 import { ThemeContext } from '../../ThemeProvider/ThemeContext.js';
-
-let isDev = false;
-
-// wrapping in try-catch because process might be undefined
-try {
-  isDev = process.env.NODE_ENV !== 'production';
-} catch {}
+import { isDev } from '../functions/dev.js';
 
 const didLogWarning = {
   fontSize: false,