From 1828d46f625c59d67fddc783c0075c1f3965caa3 Mon Sep 17 00:00:00 2001 From: Uyen Doan Date: Thu, 12 Sep 2024 12:41:56 -0500 Subject: [PATCH 1/9] separate expandable content to another component --- .../itwinui-react/src/core/Table/Table.tsx | 63 ++++++++++----- .../Table/TableExpandableContentMemoized.tsx | 47 +++++++++++ .../src/core/Table/TableRowMemoized.tsx | 17 +--- playgrounds/vite/src/App.tsx | 77 +++++++++++++++++-- 4 files changed, 161 insertions(+), 43 deletions(-) create mode 100644 packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx diff --git a/packages/itwinui-react/src/core/Table/Table.tsx b/packages/itwinui-react/src/core/Table/Table.tsx index 2e52d5941d6..4fa51b4cbca 100644 --- a/packages/itwinui-react/src/core/Table/Table.tsx +++ b/packages/itwinui-react/src/core/Table/Table.tsx @@ -38,6 +38,7 @@ import { useMergedRefs, useLatestRef, useVirtualScroll, + WithCSSTransition, } from '../../utils/index.js'; import type { CommonProps } from '../../utils/index.js'; import { TableColumnsContext } from './utils.js'; @@ -66,6 +67,7 @@ import { import { SELECTION_CELL_ID } from './columns/index.js'; import { Virtualizer, type VirtualItem } from '@tanstack/react-virtual'; import { ColumnHeader } from './ColumnHeader.js'; +import { TableExpandableContentMemoized } from './TableExpandableContentMemoized.js'; const singleRowSelectedAction = 'singleRowSelected'; const shiftRowSelectedAction = 'shiftRowSelected'; @@ -835,27 +837,46 @@ export const Table = < const row = page[index]; prepareRow(row); return ( - + <> + + {subComponent && ( + + + {subComponent(row)} + + + )} + ); }, [ diff --git a/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx b/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx new file mode 100644 index 00000000000..53763c97529 --- /dev/null +++ b/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx @@ -0,0 +1,47 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Bentley Systems, Incorporated. All rights reserved. + * See LICENSE.md in the project root for license terms and full copyright notice. + *--------------------------------------------------------------------------------------------*/ +import cx from 'classnames'; +import * as React from 'react'; +import { Box } from '../../utils/index.js'; +import type { VirtualItem } from '@tanstack/react-virtual'; +import type { PolymorphicForwardRefComponent } from '../../utils/index.js'; + +type TableExpandableContentProps = { + virtualItem?: VirtualItem; + isDisabled: boolean; + children: React.ReactNode; + isSelected?: boolean; +}; + +const TableExpandableContent = React.forwardRef((props, ref) => { + return ( + + {props.children} + + ); +}) as PolymorphicForwardRefComponent<'div', TableExpandableContentProps>; + +export const TableExpandableContentMemoized = React.memo( + TableExpandableContent, + (prevProp, nextProp) => + prevProp.children === nextProp.children && + prevProp.virtualItem === nextProp.virtualItem && + prevProp.isDisabled === nextProp.isDisabled && + prevProp.isSelected === nextProp.isSelected, +); diff --git a/packages/itwinui-react/src/core/Table/TableRowMemoized.tsx b/packages/itwinui-react/src/core/Table/TableRowMemoized.tsx index 77065627c8a..666378c6932 100644 --- a/packages/itwinui-react/src/core/Table/TableRowMemoized.tsx +++ b/packages/itwinui-react/src/core/Table/TableRowMemoized.tsx @@ -10,12 +10,7 @@ import type { TableInstance, TableState, } from '../../react-table/react-table.js'; -import { - Box, - useIntersection, - useMergedRefs, - WithCSSTransition, -} from '../../utils/index.js'; +import { Box, useIntersection, useMergedRefs } from '../../utils/index.js'; import { TableCell } from './TableCell.js'; import type { Virtualizer, VirtualItem } from '@tanstack/react-virtual'; @@ -153,16 +148,6 @@ export const TableRow = >(props: { ); })} - {subComponent && ( - - - {subComponent(row)} - - - )} ); }; diff --git a/playgrounds/vite/src/App.tsx b/playgrounds/vite/src/App.tsx index 308ffd82cc0..8176493840a 100644 --- a/playgrounds/vite/src/App.tsx +++ b/playgrounds/vite/src/App.tsx @@ -1,11 +1,76 @@ -import { Button } from '@itwin/itwinui-react'; +import { useCallback, useMemo } from 'react'; +import * as React from 'react'; +import { Divider, Table, Text } from '@itwin/itwinui-react'; +import type { Row } from '@itwin/itwinui-react/react-table'; + +export default function App() { + const columns = useMemo( + () => [ + { + id: 'name', + Header: 'Name', + accessor: 'name', + }, + { + id: 'description', + Header: 'Description', + accessor: 'description', + }, + ], + [], + ); + const data = useMemo(() => { + const size = 5; + const arr = new Array(size); + for (let i = 0; i < size; ++i) { + arr[i] = { + name: `Name${i}`, + description: `Description${i}`, + //subRows: [{ name: 'Name', description: 'Description' }], + }; + } + return arr; + }, []); + + const expandedSubComponent = useCallback( + (row: Row) => ( +
+ Extra information + 
+          
+            {JSON.stringify(
+              {
+                values: row.values,
+              },
+              null,
+              2,
+            )}
+          
+
+
+ ), + [], + ); -const App = () => { return ( <> - +

This is a test table.

+ + ); -}; - -export default App; +} From 50ba30cd1dc25df2a08a401f58005400625ae0ee Mon Sep 17 00:00:00 2001 From: Uyen Doan Date: Thu, 12 Sep 2024 13:04:38 -0500 Subject: [PATCH 2/9] fixed test --- playgrounds/vite/src/App.tsx | 77 +++--------------------------------- 1 file changed, 6 insertions(+), 71 deletions(-) diff --git a/playgrounds/vite/src/App.tsx b/playgrounds/vite/src/App.tsx index 8176493840a..308ffd82cc0 100644 --- a/playgrounds/vite/src/App.tsx +++ b/playgrounds/vite/src/App.tsx @@ -1,76 +1,11 @@ -import { useCallback, useMemo } from 'react'; -import * as React from 'react'; -import { Divider, Table, Text } from '@itwin/itwinui-react'; -import type { Row } from '@itwin/itwinui-react/react-table'; - -export default function App() { - const columns = useMemo( - () => [ - { - id: 'name', - Header: 'Name', - accessor: 'name', - }, - { - id: 'description', - Header: 'Description', - accessor: 'description', - }, - ], - [], - ); - const data = useMemo(() => { - const size = 5; - const arr = new Array(size); - for (let i = 0; i < size; ++i) { - arr[i] = { - name: `Name${i}`, - description: `Description${i}`, - //subRows: [{ name: 'Name', description: 'Description' }], - }; - } - return arr; - }, []); - - const expandedSubComponent = useCallback( - (row: Row) => ( -
- Extra information - 
-          
-            {JSON.stringify(
-              {
-                values: row.values,
-              },
-              null,
-              2,
-            )}
-          
-
-
- ), - [], - ); +import { Button } from '@itwin/itwinui-react'; +const App = () => { return ( <> -

This is a test table.

- -
+ ); -} +}; + +export default App; From f9689fdf1e70b8810f19f5933a948c74def39310 Mon Sep 17 00:00:00 2001 From: Uyen Doan Date: Thu, 12 Sep 2024 13:05:37 -0500 Subject: [PATCH 3/9] fixed test --- packages/itwinui-react/src/core/Table/Table.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/itwinui-react/src/core/Table/Table.test.tsx b/packages/itwinui-react/src/core/Table/Table.test.tsx index a0c98f73d7a..2f868a5c979 100644 --- a/packages/itwinui-react/src/core/Table/Table.test.tsx +++ b/packages/itwinui-react/src/core/Table/Table.test.tsx @@ -2040,7 +2040,7 @@ it('should render sub-rows and handle expansions', async () => { expect(onExpand).toHaveBeenNthCalledWith(1, [data[0]], expect.any(Object)); expect(onExpand).toHaveBeenNthCalledWith( 2, - [data[0], data[1]], + [data[0], data[0].subRows[1]], expect.any(Object), ); expect(onExpand).toHaveBeenNthCalledWith( From 974372f5c0d99c97ea51c6a7df2573e3f5ed9965 Mon Sep 17 00:00:00 2001 From: Uyen Doan Date: Thu, 12 Sep 2024 15:35:39 -0500 Subject: [PATCH 4/9] obmitted second param in React.memo --- .../itwinui-react/src/core/Table/Table.tsx | 7 +- .../Table/TableExpandableContentMemoized.tsx | 50 +++++------- playgrounds/vite/src/App.tsx | 77 +++++++++++++++++-- 3 files changed, 92 insertions(+), 42 deletions(-) diff --git a/packages/itwinui-react/src/core/Table/Table.tsx b/packages/itwinui-react/src/core/Table/Table.tsx index 4fa51b4cbca..431add54d49 100644 --- a/packages/itwinui-react/src/core/Table/Table.tsx +++ b/packages/itwinui-react/src/core/Table/Table.tsx @@ -863,13 +863,8 @@ export const Table = < {subComponent(row)} diff --git a/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx b/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx index 53763c97529..8e590951c65 100644 --- a/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx +++ b/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx @@ -5,43 +5,33 @@ import cx from 'classnames'; import * as React from 'react'; import { Box } from '../../utils/index.js'; -import type { VirtualItem } from '@tanstack/react-virtual'; import type { PolymorphicForwardRefComponent } from '../../utils/index.js'; type TableExpandableContentProps = { - virtualItem?: VirtualItem; isDisabled: boolean; children: React.ReactNode; isSelected?: boolean; }; -const TableExpandableContent = React.forwardRef((props, ref) => { - return ( - - {props.children} - - ); -}) as PolymorphicForwardRefComponent<'div', TableExpandableContentProps>; - export const TableExpandableContentMemoized = React.memo( - TableExpandableContent, - (prevProp, nextProp) => - prevProp.children === nextProp.children && - prevProp.virtualItem === nextProp.virtualItem && - prevProp.isDisabled === nextProp.isDisabled && - prevProp.isSelected === nextProp.isSelected, + React.forwardRef((props, ref) => { + const { isDisabled, children, isSelected, className, style, ...rest } = + props; + return ( + + {children} + + ); + }) as PolymorphicForwardRefComponent<'div', TableExpandableContentProps>, ); diff --git a/playgrounds/vite/src/App.tsx b/playgrounds/vite/src/App.tsx index 308ffd82cc0..8176493840a 100644 --- a/playgrounds/vite/src/App.tsx +++ b/playgrounds/vite/src/App.tsx @@ -1,11 +1,76 @@ -import { Button } from '@itwin/itwinui-react'; +import { useCallback, useMemo } from 'react'; +import * as React from 'react'; +import { Divider, Table, Text } from '@itwin/itwinui-react'; +import type { Row } from '@itwin/itwinui-react/react-table'; + +export default function App() { + const columns = useMemo( + () => [ + { + id: 'name', + Header: 'Name', + accessor: 'name', + }, + { + id: 'description', + Header: 'Description', + accessor: 'description', + }, + ], + [], + ); + const data = useMemo(() => { + const size = 5; + const arr = new Array(size); + for (let i = 0; i < size; ++i) { + arr[i] = { + name: `Name${i}`, + description: `Description${i}`, + //subRows: [{ name: 'Name', description: 'Description' }], + }; + } + return arr; + }, []); + + const expandedSubComponent = useCallback( + (row: Row) => ( +
+ Extra information + 
+          
+            {JSON.stringify(
+              {
+                values: row.values,
+              },
+              null,
+              2,
+            )}
+          
+
+
+ ), + [], + ); -const App = () => { return ( <> - +

This is a test table.

+ +
); -}; - -export default App; +} From 1748cafc4a2dfac6d512b40b40433de05b251554 Mon Sep 17 00:00:00 2001 From: Uyen Doan Date: Thu, 12 Sep 2024 15:37:38 -0500 Subject: [PATCH 5/9] revert App.tsx --- playgrounds/vite/src/App.tsx | 77 +++--------------------------------- 1 file changed, 6 insertions(+), 71 deletions(-) diff --git a/playgrounds/vite/src/App.tsx b/playgrounds/vite/src/App.tsx index 8176493840a..308ffd82cc0 100644 --- a/playgrounds/vite/src/App.tsx +++ b/playgrounds/vite/src/App.tsx @@ -1,76 +1,11 @@ -import { useCallback, useMemo } from 'react'; -import * as React from 'react'; -import { Divider, Table, Text } from '@itwin/itwinui-react'; -import type { Row } from '@itwin/itwinui-react/react-table'; - -export default function App() { - const columns = useMemo( - () => [ - { - id: 'name', - Header: 'Name', - accessor: 'name', - }, - { - id: 'description', - Header: 'Description', - accessor: 'description', - }, - ], - [], - ); - const data = useMemo(() => { - const size = 5; - const arr = new Array(size); - for (let i = 0; i < size; ++i) { - arr[i] = { - name: `Name${i}`, - description: `Description${i}`, - //subRows: [{ name: 'Name', description: 'Description' }], - }; - } - return arr; - }, []); - - const expandedSubComponent = useCallback( - (row: Row) => ( -
- Extra information - 
-          
-            {JSON.stringify(
-              {
-                values: row.values,
-              },
-              null,
-              2,
-            )}
-          
-
-
- ), - [], - ); +import { Button } from '@itwin/itwinui-react'; +const App = () => { return ( <> -

This is a test table.

- -
+ ); -} +}; + +export default App; From 5786f1a96767f90465edf9d7203055ac8b311346 Mon Sep 17 00:00:00 2001 From: Uyen Doan Date: Thu, 12 Sep 2024 15:44:34 -0500 Subject: [PATCH 6/9] wrapped React.memo in a separate var --- .../Table/TableExpandableContentMemoized.tsx | 41 ++++++++++--------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx b/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx index 8e590951c65..3398ac35baf 100644 --- a/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx +++ b/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx @@ -13,25 +13,26 @@ type TableExpandableContentProps = { isSelected?: boolean; }; +const TableExpandableContent = React.forwardRef((props, ref) => { + const { isDisabled, children, isSelected, className, style, ...rest } = props; + return ( + + {children} + + ); +}) as PolymorphicForwardRefComponent<'div', TableExpandableContentProps>; + export const TableExpandableContentMemoized = React.memo( - React.forwardRef((props, ref) => { - const { isDisabled, children, isSelected, className, style, ...rest } = - props; - return ( - - {children} - - ); - }) as PolymorphicForwardRefComponent<'div', TableExpandableContentProps>, + TableExpandableContent, ); From b9d0fd4cbb85dc76e6e30b06bf15632059b9a3c2 Mon Sep 17 00:00:00 2001 From: Uyen Doan Date: Fri, 13 Sep 2024 08:58:29 -0500 Subject: [PATCH 7/9] removed aria attributes --- packages/itwinui-react/src/core/Table/Table.tsx | 2 -- .../src/core/Table/TableExpandableContentMemoized.tsx | 6 +----- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/itwinui-react/src/core/Table/Table.tsx b/packages/itwinui-react/src/core/Table/Table.tsx index 431add54d49..fd9e2839d2e 100644 --- a/packages/itwinui-react/src/core/Table/Table.tsx +++ b/packages/itwinui-react/src/core/Table/Table.tsx @@ -863,9 +863,7 @@ export const Table = < {subComponent(row)} diff --git a/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx b/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx index 3398ac35baf..7d9dc0ccaca 100644 --- a/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx +++ b/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx @@ -8,13 +8,11 @@ import { Box } from '../../utils/index.js'; import type { PolymorphicForwardRefComponent } from '../../utils/index.js'; type TableExpandableContentProps = { - isDisabled: boolean; children: React.ReactNode; - isSelected?: boolean; }; const TableExpandableContent = React.forwardRef((props, ref) => { - const { isDisabled, children, isSelected, className, style, ...rest } = props; + const { children, className, style, ...rest } = props; return ( { minWidth: '100%', ...style, }} - aria-disabled={isDisabled} - aria-selected={isSelected} ref={ref} {...rest} > From 6c30127cdafcec44b43f10bf3ac20d8fd282b6cf Mon Sep 17 00:00:00 2001 From: Uyen Doan Date: Fri, 13 Sep 2024 16:00:39 -0500 Subject: [PATCH 8/9] added isDisabled --- .../itwinui-react/src/core/Table/Table.tsx | 1 + .../Table/TableExpandableContentMemoized.tsx | 4 +- playgrounds/vite/src/App.tsx | 78 +++++++++++++++++-- 3 files changed, 76 insertions(+), 7 deletions(-) diff --git a/packages/itwinui-react/src/core/Table/Table.tsx b/packages/itwinui-react/src/core/Table/Table.tsx index fd9e2839d2e..f3f2ae3c75c 100644 --- a/packages/itwinui-react/src/core/Table/Table.tsx +++ b/packages/itwinui-react/src/core/Table/Table.tsx @@ -864,6 +864,7 @@ export const Table = < {subComponent(row)} diff --git a/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx b/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx index 7d9dc0ccaca..dfb42e75015 100644 --- a/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx +++ b/packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx @@ -9,10 +9,11 @@ import type { PolymorphicForwardRefComponent } from '../../utils/index.js'; type TableExpandableContentProps = { children: React.ReactNode; + isDisabled?: boolean; }; const TableExpandableContent = React.forwardRef((props, ref) => { - const { children, className, style, ...rest } = props; + const { children, className, style, isDisabled, ...rest } = props; return ( { minWidth: '100%', ...style, }} + aria-disabled={isDisabled} ref={ref} {...rest} > diff --git a/playgrounds/vite/src/App.tsx b/playgrounds/vite/src/App.tsx index 308ffd82cc0..6a1dabff9fd 100644 --- a/playgrounds/vite/src/App.tsx +++ b/playgrounds/vite/src/App.tsx @@ -1,11 +1,77 @@ -import { Button } from '@itwin/itwinui-react'; +import { useCallback, useMemo } from 'react'; +import * as React from 'react'; +import { Divider, Table, Text } from '@itwin/itwinui-react'; +import type { Row } from '@itwin/itwinui-react/react-table'; + +export default function App() { + const columns = useMemo( + () => [ + { + id: 'name', + Header: 'Name', + accessor: 'name', + }, + { + id: 'description', + Header: 'Description', + accessor: 'description', + }, + ], + [], + ); + const data = useMemo(() => { + const size = 5; + const arr = new Array(size); + for (let i = 0; i < size; ++i) { + arr[i] = { + name: `Name${i}`, + description: `Description${i}`, + //subRows: [{ name: 'Name', description: 'Description' }], + }; + } + return arr; + }, []); + + const expandedSubComponent = useCallback( + (row: Row) => ( +
+ Extra information + 
+          
+            {JSON.stringify(
+              {
+                values: row.values,
+              },
+              null,
+              2,
+            )}
+          
+
+
+ ), + [], + ); -const App = () => { return ( <> - +

This is a test table.

+ +
); -}; - -export default App; +} From 023128c7dad0f29a93600968f193cb61c1588180 Mon Sep 17 00:00:00 2001 From: Uyen Doan Date: Fri, 13 Sep 2024 16:13:45 -0500 Subject: [PATCH 9/9] revert App.tsx --- playgrounds/vite/src/App.tsx | 78 +++--------------------------------- 1 file changed, 6 insertions(+), 72 deletions(-) diff --git a/playgrounds/vite/src/App.tsx b/playgrounds/vite/src/App.tsx index 6a1dabff9fd..308ffd82cc0 100644 --- a/playgrounds/vite/src/App.tsx +++ b/playgrounds/vite/src/App.tsx @@ -1,77 +1,11 @@ -import { useCallback, useMemo } from 'react'; -import * as React from 'react'; -import { Divider, Table, Text } from '@itwin/itwinui-react'; -import type { Row } from '@itwin/itwinui-react/react-table'; - -export default function App() { - const columns = useMemo( - () => [ - { - id: 'name', - Header: 'Name', - accessor: 'name', - }, - { - id: 'description', - Header: 'Description', - accessor: 'description', - }, - ], - [], - ); - const data = useMemo(() => { - const size = 5; - const arr = new Array(size); - for (let i = 0; i < size; ++i) { - arr[i] = { - name: `Name${i}`, - description: `Description${i}`, - //subRows: [{ name: 'Name', description: 'Description' }], - }; - } - return arr; - }, []); - - const expandedSubComponent = useCallback( - (row: Row) => ( -
- Extra information - 
-          
-            {JSON.stringify(
-              {
-                values: row.values,
-              },
-              null,
-              2,
-            )}
-          
-
-
- ), - [], - ); +import { Button } from '@itwin/itwinui-react'; +const App = () => { return ( <> -

This is a test table.

- -
+ ); -} +}; + +export default App;