From 8b329072dce8beaa8fc29154aea501afcdbcfad5 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Fri, 16 Sep 2022 13:54:50 -0700 Subject: [PATCH 1/5] TableView reduce renders and onLoadMore --- .../@react-spectrum/table/src/TableView.tsx | 70 ++++++++++++++----- .../@react-spectrum/table/test/Table.test.js | 8 +-- .../table/test/TableSizing.test.js | 2 +- packages/@react-stately/table/package.json | 1 + .../table/src/useTableColumnResizeState.ts | 34 +++++---- 5 files changed, 78 insertions(+), 37 deletions(-) diff --git a/packages/@react-spectrum/table/src/TableView.tsx b/packages/@react-spectrum/table/src/TableView.tsx index fb1a215d53f..1213e41d78f 100644 --- a/packages/@react-spectrum/table/src/TableView.tsx +++ b/packages/@react-spectrum/table/src/TableView.tsx @@ -21,7 +21,13 @@ import {GridNode} from '@react-types/grid'; // @ts-ignore import intlMessages from '../intl/*.json'; import {Item, Menu, MenuTrigger} from '@react-spectrum/menu'; -import {layoutInfoToStyle, ScrollView, setScrollLeft, useVirtualizer, VirtualizerItem} from '@react-aria/virtualizer'; +import { + layoutInfoToStyle, + ScrollView, + setScrollLeft, + useVirtualizer, + VirtualizerItem +} from '@react-aria/virtualizer'; import {Nubbin} from './Nubbin'; import {ProgressCircle} from '@react-spectrum/progress'; import React, {ReactElement, useCallback, useContext, useEffect, useMemo, useRef, useState} from 'react'; @@ -121,10 +127,6 @@ function TableView(props: SpectrumTableProps, ref: DOMRef { - setIsInResizeMode(false); - }}), getDefaultWidth}, state.collection); - // If the selection behavior changes in state, we need to update showSelectionCheckboxes here due to the circular dependency... let shouldShowCheckboxes = state.selectionManager.selectionBehavior !== 'replace'; if (shouldShowCheckboxes !== showSelectionCheckboxes) { @@ -134,6 +136,7 @@ function TableView(props: SpectrumTableProps, ref: DOMRef(); let bodyRef = useRef(); + let virtualizer = useRef(null); let stringFormatter = useLocalizedStringFormatter(intlMessages); let density = props.density || 'regular'; @@ -154,6 +157,17 @@ function TableView(props: SpectrumTableProps, ref: DOMRef { + if (virtualizer.current) { + virtualizer.current.relayoutNow({sizeChanged: true}); + } + }, + onColumnResizeEnd: () => { + setIsInResizeMode(false); + } + }), getDefaultWidth}, state.collection); layout.collection = state.collection; layout.getColumnWidth = columnState.getColumnWidth; @@ -165,7 +179,7 @@ function TableView(props: SpectrumTableProps, ref: DOMRef, unknown>; let renderWrapper = (parent: View, reusableView: View, children: View[], renderChildren: (views: View[]) => ReactElement[]) => { let style = layoutInfoToStyle(reusableView.layoutInfo, direction, parent && parent.layoutInfo); @@ -366,14 +380,29 @@ function TableView(props: SpectrumTableProps, ref: DOMRef + virtualizer={virtualizer} + isFocusVisible={isFocusVisible} /> ); } // This is a custom Virtualizer that also has a header that syncs its scroll position with the body. -function TableVirtualizer({layout, collection, lastResizeInteractionModality, focusedKey, renderView, renderWrapper, domRef, bodyRef, headerRef, setTableWidth, getColumnWidth, onVisibleRectChange: onVisibleRectChangeProp, isFocusVisible, ...otherProps}) { +function TableVirtualizer({ + layout, + collection, + lastResizeInteractionModality, + focusedKey, + renderView, + renderWrapper, + domRef, + bodyRef, + headerRef, + setTableWidth, + onVisibleRectChange: onVisibleRectChangeProp, + isFocusVisible, + virtualizer, + ...otherProps +}) { let {direction} = useLocale(); let {state: tableState, columnState} = useTableContext(); let loadingState = collection.body.props.loadingState; @@ -390,6 +419,7 @@ function TableVirtualizer({layout, collection, lastResizeInteractionModality, fo }, transitionDuration: isLoading ? 160 : 220 }); + virtualizer.current = state.virtualizer; let {virtualizerProps} = useVirtualizer({ focusedKey, @@ -409,11 +439,6 @@ function TableVirtualizer({layout, collection, lastResizeInteractionModality, fo } }, state, domRef); - // If columnwidths change, need to relayout. - useLayoutEffect(() => { - state.virtualizer.relayoutNow({sizeChanged: true}); - }, [getColumnWidth, state.virtualizer]); - useEffect(() => { if (lastResizeInteractionModality.current === 'keyboard' && headerRef.current.contains(document.activeElement)) { document.activeElement?.scrollIntoView?.(false); @@ -430,9 +455,11 @@ function TableVirtualizer({layout, collection, lastResizeInteractionModality, fo }, [bodyRef, headerRef]); let onVisibleRectChange = useCallback((rect: Rect) => { - setTableWidth(rect.width); - + // order matters here, setTableWidth calls a relayoutNow + // setVisibleRect calls a relayout after a raf, to combine them, they must be in this order + // so that the relayoutNow cancels the raf state.setVisibleRect(rect); + setTableWidth(rect.width); if (!isLoading && onLoadMore) { let scrollOffset = state.virtualizer.contentSize.height - rect.height * 2; @@ -442,13 +469,20 @@ function TableVirtualizer({layout, collection, lastResizeInteractionModality, fo } }, [onLoadMore, isLoading, state.setVisibleRect, state.virtualizer]); + let prevContentSizeHeight = useRef(state.virtualizer.contentSize.height); useLayoutEffect(() => { if (!isLoading && onLoadMore && !state.isAnimating) { - if (state.contentSize.height <= state.virtualizer.visibleRect.height) { + if (prevContentSizeHeight.current > 0 && state.virtualizer.contentSize.height < state.virtualizer.visibleRect.height) { + // choosing + // - state.contentSize.height > 0 && state.contentSize.height // if height reduces (delete the page) then we want to load more + // - state.virtualizer.contentSize.height // not an entry in dependency array, so if we remove the state.contentSize, this won't work if we removed the dependency we aren't using + // - ref for initial render + // - ref to store content size from virtualizer and remove dependency array <- onLoadMore(); } } - }, [state.contentSize, state.virtualizer, state.isAnimating, onLoadMore, isLoading]); + prevContentSizeHeight.current = state.virtualizer.contentSize.height; + }); let keysBefore = []; let key = columnState.currentlyResizingColumn; diff --git a/packages/@react-spectrum/table/test/Table.test.js b/packages/@react-spectrum/table/test/Table.test.js index f962b4c8be1..97adbecefed 100644 --- a/packages/@react-spectrum/table/test/Table.test.js +++ b/packages/@react-spectrum/table/test/Table.test.js @@ -3866,7 +3866,7 @@ describe('TableView', function () { fireEvent.scroll(scrollView); act(() => {jest.runAllTimers();}); - expect(onLoadMore).toHaveBeenCalledTimes(3); + expect(onLoadMore).toHaveBeenCalledTimes(1); }); it('should automatically fire onLoadMore if there aren\'t enough items to fill the Table', function () { @@ -3891,10 +3891,8 @@ describe('TableView', function () { render(); act(() => jest.runAllTimers()); - // first loadMore triggered by onVisibleRectChange, other 3 by useLayoutEffect - // we can't get better results than that without mocking every single clientHeight/Width - // this is a good candidate for storybook interactions test - expect(onLoadMoreSpy).toHaveBeenCalledTimes(4); + + expect(onLoadMoreSpy).toHaveBeenCalledTimes(2); }); }); diff --git a/packages/@react-spectrum/table/test/TableSizing.test.js b/packages/@react-spectrum/table/test/TableSizing.test.js index e58296d9a17..21ce97c984e 100644 --- a/packages/@react-spectrum/table/test/TableSizing.test.js +++ b/packages/@react-spectrum/table/test/TableSizing.test.js @@ -102,7 +102,7 @@ describe('TableViewSizing', function () { , scale); - it('should layout rows with default height', function () { + it('should layout rows with default height', async function () { let tree = renderTable(); let rows = tree.getAllByRole('row'); expect(rows).toHaveLength(3); diff --git a/packages/@react-stately/table/package.json b/packages/@react-stately/table/package.json index a7026333cc6..082e620934f 100644 --- a/packages/@react-stately/table/package.json +++ b/packages/@react-stately/table/package.json @@ -18,6 +18,7 @@ }, "dependencies": { "@babel/runtime": "^7.6.2", + "@react-aria/utils": "^3.13.3", "@react-stately/collections": "^3.4.3", "@react-stately/grid": "^3.3.1", "@react-stately/selection": "^3.10.3", diff --git a/packages/@react-stately/table/src/useTableColumnResizeState.ts b/packages/@react-stately/table/src/useTableColumnResizeState.ts index c8fd7c9b5ef..fdd640f8492 100644 --- a/packages/@react-stately/table/src/useTableColumnResizeState.ts +++ b/packages/@react-stately/table/src/useTableColumnResizeState.ts @@ -3,6 +3,7 @@ import {ColumnProps} from '@react-types/table'; import {getContentWidth, getDynamicColumnWidths, getMaxWidth, getMinWidth, isStatic, parseStaticWidth} from './utils'; import {GridNode} from '@react-types/grid'; import {Key, MutableRefObject, useCallback, useRef, useState} from 'react'; +import {useLayoutEffect} from '@react-aria/utils'; interface AffectedColumnWidth { /** The column key. */ @@ -41,7 +42,8 @@ export interface TableColumnResizeStateProps { /** Callback that is invoked when the resize event is ended. */ onColumnResizeEnd?: (affectedColumnWidths: AffectedColumnWidths) => void, /** The default table width. */ - tableWidth?: number + tableWidth?: number, + onResize?: () => void } interface ColumnState { @@ -49,25 +51,30 @@ interface ColumnState { } export function useTableColumnResizeState(props: TableColumnResizeStateProps, state: ColumnState): TableColumnResizeState { - const {getDefaultWidth, tableWidth: defaultTableWidth = null} = props; + const {getDefaultWidth, tableWidth: defaultTableWidth = null, onResize} = props; const {columns} = state; const columnsRef = useRef[]>([]); const tableWidth = useRef(defaultTableWidth); const isResizing = useRef(null); const startResizeContentWidth = useRef(); - const [columnWidths, setColumnWidths] = useState>(new Map(columns.map(col => [col.key, 0]))); - const columnWidthsRef = useRef>(columnWidths); + const columnWidthsRef = useRef>(new Map(columns.map(col => [col.key, 0]))); + const {1: setColumnWidths} = useState>(columnWidthsRef.current); const affectedColumnWidthsRef = useRef([]); const [resizedColumns, setResizedColumns] = useState>(new Set()); const resizedColumnsRef = useRef>(resizedColumns); const [currentlyResizingColumn, setCurrentlyResizingColumn] = useState(null); + let prevWidths = useRef>(columnWidthsRef.current); function setColumnWidthsForRef(newWidths: Map) { columnWidthsRef.current = newWidths; - // new map so that change detection is triggered - setColumnWidths(newWidths); + if (prevWidths.current !== newWidths && onResize) { + onResize(); + prevWidths.current = newWidths; + } else if (!onResize) { + setColumnWidths(newWidths); + } } /* returns the resolved column width in this order: @@ -109,13 +116,14 @@ export function useTableColumnResizeState(props: TableColumnResizeStateProps, const prevColKeys = columnsRef.current.map(col => col.key); - const colKeys = columns.map(col => col.key); - // if the columns change, need to rebuild widths. - if (prevColKeys.length !== colKeys.length || !colKeys.every((col, i) => col === prevColKeys[i])) { - columnsRef.current = columns; - const widths = buildColumnWidths(columns, tableWidth.current); - setColumnWidthsForRef(widths); - } + useLayoutEffect(() => { + const colKeys = columns.map(col => col.key); + if (prevColKeys.length !== colKeys.length || colKeys.some((col, i) => col !== prevColKeys[i])) { + columnsRef.current = columns; + const widths = buildColumnWidths(columns, tableWidth.current); + setColumnWidthsForRef(widths); + } + }); function setTableWidth(width: number) { if (width && width !== tableWidth.current) { From d105405d10e32d3e2d0711783834645a8dee6cca Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Fri, 16 Sep 2022 14:09:19 -0700 Subject: [PATCH 2/5] fix lint --- packages/@react-spectrum/table/test/TableSizing.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/table/test/TableSizing.test.js b/packages/@react-spectrum/table/test/TableSizing.test.js index 21ce97c984e..e58296d9a17 100644 --- a/packages/@react-spectrum/table/test/TableSizing.test.js +++ b/packages/@react-spectrum/table/test/TableSizing.test.js @@ -102,7 +102,7 @@ describe('TableViewSizing', function () { , scale); - it('should layout rows with default height', async function () { + it('should layout rows with default height', function () { let tree = renderTable(); let rows = tree.getAllByRole('row'); expect(rows).toHaveLength(3); From 4bd44be206b9128e85a958f385a1f8be5603616c Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Tue, 27 Sep 2022 18:06:47 -0700 Subject: [PATCH 3/5] reset to known working point --- .../@react-spectrum/table/src/TableView.tsx | 9 +- .../@react-spectrum/table/test/Table.test.js | 110 ++++++++++-------- .../table/test/TableSizing.test.js | 45 +++++-- .../table/src/useTableColumnResizeState.ts | 14 +-- 4 files changed, 105 insertions(+), 73 deletions(-) diff --git a/packages/@react-spectrum/table/src/TableView.tsx b/packages/@react-spectrum/table/src/TableView.tsx index 1213e41d78f..6d21cc9548b 100644 --- a/packages/@react-spectrum/table/src/TableView.tsx +++ b/packages/@react-spectrum/table/src/TableView.tsx @@ -161,7 +161,7 @@ function TableView(props: SpectrumTableProps, ref: DOMRef { if (virtualizer.current) { - virtualizer.current.relayoutNow({sizeChanged: true}); + virtualizer.current.relayout({sizeChanged: true}); } }, onColumnResizeEnd: () => { @@ -472,12 +472,7 @@ function TableVirtualizer({ let prevContentSizeHeight = useRef(state.virtualizer.contentSize.height); useLayoutEffect(() => { if (!isLoading && onLoadMore && !state.isAnimating) { - if (prevContentSizeHeight.current > 0 && state.virtualizer.contentSize.height < state.virtualizer.visibleRect.height) { - // choosing - // - state.contentSize.height > 0 && state.contentSize.height // if height reduces (delete the page) then we want to load more - // - state.virtualizer.contentSize.height // not an entry in dependency array, so if we remove the state.contentSize, this won't work if we removed the dependency we aren't using - // - ref for initial render - // - ref to store content size from virtualizer and remove dependency array <- + if (prevContentSizeHeight.current > 0 && prevContentSizeHeight.current < state.virtualizer.visibleRect.height) { onLoadMore(); } } diff --git a/packages/@react-spectrum/table/test/Table.test.js b/packages/@react-spectrum/table/test/Table.test.js index 97adbecefed..4b0b6850b06 100644 --- a/packages/@react-spectrum/table/test/Table.test.js +++ b/packages/@react-spectrum/table/test/Table.test.js @@ -144,17 +144,26 @@ describe('TableView', function () { act(() => {jest.runAllTimers();}); }); - let render = (children, scale = 'medium') => renderComponent( - - {children} - - ); + let render = (children, scale = 'medium') => { + let tree = renderComponent( + + {children} + + ); + // account for table column resizing to do initial pass due to relayout from useTableColumnResizeState render + act(() => {jest.runAllTimers();}); + return tree; + }; - let rerender = (tree, children, scale = 'medium') => tree.rerender( - - {children} - - ); + let rerender = (tree, children, scale = 'medium') => { + let newTree = tree.rerender( + + {children} + + ); + act(() => {jest.runAllTimers();}); + return newTree; + }; // I'd use tree.getByRole(role, {name: text}) here, but it's unbearably slow. let getCell = (tree, text) => { @@ -786,42 +795,50 @@ describe('TableView', function () { describe('keyboard focus', function () { // locale is being set here, since we can't nest them, use original render function - let renderTable = (locale = 'en-US', props = {}) => renderComponent( - - - - {column => {column.name}} - - - {item => - ( - {key => {item[key]}} - ) - } - - - - ); + let renderTable = (locale = 'en-US', props = {}) => { + let tree = renderComponent( + + + + {column => {column.name}} + + + {item => + ( + {key => {item[key]}} + ) + } + + + + ); + act(() => {jest.runAllTimers();}); + return tree; + }; // locale is being set here, since we can't nest them, use original render function - let renderNested = (locale = 'en-US') => renderComponent( - - - - {column => - {column.name} - } - - - {item => - ( - {key => {item[key]}} - ) - } - - - - ); + let renderNested = (locale = 'en-US') => { + let tree = renderComponent( + + + + {column => + {column.name} + } + + + {item => + ( + {key => {item[key]}} + ) + } + + + + ); + act(() => {jest.runAllTimers();}); + return tree; + }; let renderMany = () => render( @@ -3866,7 +3883,7 @@ describe('TableView', function () { fireEvent.scroll(scrollView); act(() => {jest.runAllTimers();}); - expect(onLoadMore).toHaveBeenCalledTimes(1); + expect(onLoadMore).toHaveBeenCalledTimes(2); }); it('should automatically fire onLoadMore if there aren\'t enough items to fill the Table', function () { @@ -3890,9 +3907,8 @@ describe('TableView', function () { ); render(); - act(() => jest.runAllTimers()); - expect(onLoadMoreSpy).toHaveBeenCalledTimes(2); + expect(onLoadMoreSpy).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/@react-spectrum/table/test/TableSizing.test.js b/packages/@react-spectrum/table/test/TableSizing.test.js index e58296d9a17..f6da1af01f1 100644 --- a/packages/@react-spectrum/table/test/TableSizing.test.js +++ b/packages/@react-spectrum/table/test/TableSizing.test.js @@ -73,17 +73,26 @@ describe('TableViewSizing', function () { act(() => {jest.runAllTimers();}); }); - let render = (children, scale = 'medium') => renderComponent( - - {children} - - ); - - let rerender = (tree, children, scale = 'medium') => tree.rerender( - - {children} - - ); + let render = (children, scale = 'medium') => { + let tree = renderComponent( + + {children} + + ); + // account for table column resizing to do initial pass due to relayout from useTableColumnResizeState render + act(() => {jest.runAllTimers();}); + return tree; + }; + + let rerender = (tree, children, scale = 'medium') => { + let newTree = tree.rerender( + + {children} + + ); + act(() => {jest.runAllTimers();}); + return newTree; + }; describe('layout', function () { describe('row heights', function () { @@ -679,6 +688,7 @@ describe('TableViewSizing', function () { fireEvent.pointerMove(resizer, {pointerType: 'mouse', pointerId: 1, pageX: 595, pageY: 25}); fireEvent.pointerUp(resizer, {pointerType: 'mouse', pointerId: 1}); + act(() => {jest.runAllTimers();}); for (let row of rows) { expect(row.childNodes[0].style.width).toBe('595px'); @@ -704,6 +714,7 @@ describe('TableViewSizing', function () { fireEvent.pointerMove(resizer, {pointerType: 'mouse', pointerId: 1, pageX: 620, pageY: 25}); fireEvent.pointerUp(resizer, {pointerType: 'mouse', pointerId: 1}); + act(() => {jest.runAllTimers();}); for (let row of rows) { expect(row.childNodes[0].style.width).toBe('620px'); @@ -775,6 +786,7 @@ describe('TableViewSizing', function () { fireEvent.pointerMove(resizer, {pointerType: 'mouse', pointerId: 1, pageX: 595, pageY: 25}); fireEvent.pointerUp(resizer, {pointerType: 'mouse', pointerId: 1}); + act(() => {jest.runAllTimers();}); for (let row of rows) { expect(row.childNodes[0].style.width).toBe('595px'); @@ -800,6 +812,7 @@ describe('TableViewSizing', function () { fireEvent.pointerMove(resizer, {pointerType: 'mouse', pointerId: 1, pageX: 620, pageY: 25}); fireEvent.pointerUp(resizer, {pointerType: 'mouse', pointerId: 1}); + act(() => {jest.runAllTimers();}); for (let row of rows) { expect(row.childNodes[0].style.width).toBe('620px'); @@ -884,6 +897,7 @@ describe('TableViewSizing', function () { fireEvent.pointerMove(resizer, {pointerType: 'touch', pointerId: 1, pageX: 595, pageY: 25}); fireEvent.pointerUp(resizer, {pointerType: 'touch', pointerId: 1}); + act(() => {jest.runAllTimers();}); for (let row of rows) { expect(row.childNodes[0].style.width).toBe('595px'); @@ -896,6 +910,7 @@ describe('TableViewSizing', function () { fireEvent.pointerMove(resizer, {pointerType: 'touch', pointerId: 1, pageX: 620, pageY: 25}); fireEvent.pointerUp(resizer, {pointerType: 'touch', pointerId: 1}); + act(() => {jest.runAllTimers();}); for (let row of rows) { expect(row.childNodes[0].style.width).toBe('620px'); @@ -980,6 +995,7 @@ describe('TableViewSizing', function () { fireEvent.pointerMove(resizer, {pointerType: 'touch', pointerId: 1, pageX: 595, pageY: 25}); fireEvent.pointerUp(resizer, {pointerType: 'touch', pointerId: 1}); + act(() => {jest.runAllTimers();}); for (let row of rows) { expect(row.childNodes[0].style.width).toBe('595px'); @@ -992,6 +1008,7 @@ describe('TableViewSizing', function () { fireEvent.pointerMove(resizer, {pointerType: 'touch', pointerId: 1, pageX: 620, pageY: 25}); fireEvent.pointerUp(resizer, {pointerType: 'touch', pointerId: 1}); + act(() => {jest.runAllTimers();}); for (let row of rows) { expect(row.childNodes[0].style.width).toBe('620px'); @@ -1076,6 +1093,7 @@ describe('TableViewSizing', function () { fireEvent.keyDown(document.activeElement, {key: 'ArrowRight'}); fireEvent.keyUp(document.activeElement, {key: 'ArrowRight'}); + act(() => {jest.runAllTimers();}); for (let row of rows) { expect(row.childNodes[0].style.width).toBe('620px'); @@ -1088,6 +1106,7 @@ describe('TableViewSizing', function () { fireEvent.keyDown(document.activeElement, {key: 'ArrowLeft'}); fireEvent.keyUp(document.activeElement, {key: 'ArrowLeft'}); + act(() => {jest.runAllTimers();}); for (let row of rows) { expect(row.childNodes[0].style.width).toBe('600px'); @@ -1100,6 +1119,7 @@ describe('TableViewSizing', function () { fireEvent.keyDown(document.activeElement, {key: 'ArrowUp'}); fireEvent.keyUp(document.activeElement, {key: 'ArrowUp'}); + act(() => {jest.runAllTimers();}); for (let row of rows) { expect(row.childNodes[0].style.width).toBe('620px'); @@ -1112,6 +1132,7 @@ describe('TableViewSizing', function () { fireEvent.keyDown(document.activeElement, {key: 'ArrowDown'}); fireEvent.keyUp(document.activeElement, {key: 'ArrowDown'}); + act(() => {jest.runAllTimers();}); for (let row of rows) { expect(row.childNodes[0].style.width).toBe('600px'); @@ -1192,6 +1213,7 @@ describe('TableViewSizing', function () { fireEvent.keyDown(document.activeElement, {key: 'ArrowRight'}); fireEvent.keyUp(document.activeElement, {key: 'ArrowRight'}); + act(() => {jest.runAllTimers();}); for (let row of rows) { expect(row.childNodes[0].style.width).toBe('620px'); @@ -1204,6 +1226,7 @@ describe('TableViewSizing', function () { fireEvent.keyDown(document.activeElement, {key: 'ArrowLeft'}); fireEvent.keyUp(document.activeElement, {key: 'ArrowLeft'}); + act(() => {jest.runAllTimers();}); for (let row of rows) { expect(row.childNodes[0].style.width).toBe('600px'); diff --git a/packages/@react-stately/table/src/useTableColumnResizeState.ts b/packages/@react-stately/table/src/useTableColumnResizeState.ts index fdd640f8492..39f03998d22 100644 --- a/packages/@react-stately/table/src/useTableColumnResizeState.ts +++ b/packages/@react-stately/table/src/useTableColumnResizeState.ts @@ -116,14 +116,12 @@ export function useTableColumnResizeState(props: TableColumnResizeStateProps, const prevColKeys = columnsRef.current.map(col => col.key); - useLayoutEffect(() => { - const colKeys = columns.map(col => col.key); - if (prevColKeys.length !== colKeys.length || colKeys.some((col, i) => col !== prevColKeys[i])) { - columnsRef.current = columns; - const widths = buildColumnWidths(columns, tableWidth.current); - setColumnWidthsForRef(widths); - } - }); + const colKeys = columns.map(col => col.key); + if (prevColKeys.length !== colKeys.length || colKeys.some((col, i) => col !== prevColKeys[i])) { + columnsRef.current = columns; + const widths = buildColumnWidths(columns, tableWidth.current); + setColumnWidthsForRef(widths); + } function setTableWidth(width: number) { if (width && width !== tableWidth.current) { From 41de2e7c9f89741242114a8cc736d4d8e8d8b8c0 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Tue, 27 Sep 2022 18:08:11 -0700 Subject: [PATCH 4/5] remove aria from stately --- packages/@react-stately/table/package.json | 1 - packages/@react-stately/table/src/useTableColumnResizeState.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/@react-stately/table/package.json b/packages/@react-stately/table/package.json index 082e620934f..a7026333cc6 100644 --- a/packages/@react-stately/table/package.json +++ b/packages/@react-stately/table/package.json @@ -18,7 +18,6 @@ }, "dependencies": { "@babel/runtime": "^7.6.2", - "@react-aria/utils": "^3.13.3", "@react-stately/collections": "^3.4.3", "@react-stately/grid": "^3.3.1", "@react-stately/selection": "^3.10.3", diff --git a/packages/@react-stately/table/src/useTableColumnResizeState.ts b/packages/@react-stately/table/src/useTableColumnResizeState.ts index 39f03998d22..89dde07daab 100644 --- a/packages/@react-stately/table/src/useTableColumnResizeState.ts +++ b/packages/@react-stately/table/src/useTableColumnResizeState.ts @@ -3,7 +3,6 @@ import {ColumnProps} from '@react-types/table'; import {getContentWidth, getDynamicColumnWidths, getMaxWidth, getMinWidth, isStatic, parseStaticWidth} from './utils'; import {GridNode} from '@react-types/grid'; import {Key, MutableRefObject, useCallback, useRef, useState} from 'react'; -import {useLayoutEffect} from '@react-aria/utils'; interface AffectedColumnWidth { /** The column key. */ From 3672fb2f2217d6bb6c46c39ee953c1fee561ed92 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Tue, 27 Sep 2022 18:41:56 -0700 Subject: [PATCH 5/5] Use relayoutNow when possible --- packages/@react-spectrum/table/src/TableView.tsx | 6 ++++-- packages/@react-spectrum/table/test/Table.test.js | 4 ++-- .../@react-stately/table/src/useTableColumnResizeState.ts | 8 ++++---- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/@react-spectrum/table/src/TableView.tsx b/packages/@react-spectrum/table/src/TableView.tsx index 6d21cc9548b..5ae8199a70e 100644 --- a/packages/@react-spectrum/table/src/TableView.tsx +++ b/packages/@react-spectrum/table/src/TableView.tsx @@ -159,9 +159,11 @@ function TableView(props: SpectrumTableProps, ref: DOMRef { - if (virtualizer.current) { + onResize: (inRender) => { + if (virtualizer.current && inRender) { virtualizer.current.relayout({sizeChanged: true}); + } else if (virtualizer.current) { + virtualizer.current.relayoutNow({sizeChanged: true}); } }, onColumnResizeEnd: () => { diff --git a/packages/@react-spectrum/table/test/Table.test.js b/packages/@react-spectrum/table/test/Table.test.js index 4b0b6850b06..98c8a484774 100644 --- a/packages/@react-spectrum/table/test/Table.test.js +++ b/packages/@react-spectrum/table/test/Table.test.js @@ -3883,7 +3883,7 @@ describe('TableView', function () { fireEvent.scroll(scrollView); act(() => {jest.runAllTimers();}); - expect(onLoadMore).toHaveBeenCalledTimes(2); + expect(onLoadMore).toHaveBeenCalledTimes(1); }); it('should automatically fire onLoadMore if there aren\'t enough items to fill the Table', function () { @@ -3908,7 +3908,7 @@ describe('TableView', function () { render(); - expect(onLoadMoreSpy).toHaveBeenCalledTimes(1); + expect(onLoadMoreSpy).toHaveBeenCalledTimes(2); }); }); diff --git a/packages/@react-stately/table/src/useTableColumnResizeState.ts b/packages/@react-stately/table/src/useTableColumnResizeState.ts index 89dde07daab..36420c6db7e 100644 --- a/packages/@react-stately/table/src/useTableColumnResizeState.ts +++ b/packages/@react-stately/table/src/useTableColumnResizeState.ts @@ -42,7 +42,7 @@ export interface TableColumnResizeStateProps { onColumnResizeEnd?: (affectedColumnWidths: AffectedColumnWidths) => void, /** The default table width. */ tableWidth?: number, - onResize?: () => void + onResize?: (inRender: boolean) => void } interface ColumnState { @@ -66,10 +66,10 @@ export function useTableColumnResizeState(props: TableColumnResizeStateProps, const [currentlyResizingColumn, setCurrentlyResizingColumn] = useState(null); let prevWidths = useRef>(columnWidthsRef.current); - function setColumnWidthsForRef(newWidths: Map) { + function setColumnWidthsForRef(newWidths: Map, inRender = false) { columnWidthsRef.current = newWidths; if (prevWidths.current !== newWidths && onResize) { - onResize(); + onResize(inRender); prevWidths.current = newWidths; } else if (!onResize) { setColumnWidths(newWidths); @@ -119,7 +119,7 @@ export function useTableColumnResizeState(props: TableColumnResizeStateProps, if (prevColKeys.length !== colKeys.length || colKeys.some((col, i) => col !== prevColKeys[i])) { columnsRef.current = columns; const widths = buildColumnWidths(columns, tableWidth.current); - setColumnWidthsForRef(widths); + setColumnWidthsForRef(widths, true); } function setTableWidth(width: number) {