diff --git a/packages/@react-spectrum/table/src/TableView.tsx b/packages/@react-spectrum/table/src/TableView.tsx index 2979874f0c8..f63cca10c74 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,19 @@ function TableView(props: SpectrumTableProps, ref: DOMRef { + if (virtualizer.current && inRender) { + virtualizer.current.relayout({sizeChanged: true}); + } else if (virtualizer.current) { + virtualizer.current.relayoutNow({sizeChanged: true}); + } + }, + onColumnResizeEnd: () => { + setIsInResizeMode(false); + } + }), getDefaultWidth}, state.collection); layout.collection = state.collection; layout.getColumnWidth = columnState.getColumnWidth; @@ -165,7 +181,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 +382,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 +421,7 @@ function TableVirtualizer({layout, collection, lastResizeInteractionModality, fo }, transitionDuration: isLoading ? 160 : 220 }); + virtualizer.current = state.virtualizer; let {virtualizerProps} = useVirtualizer({ focusedKey, @@ -409,11 +441,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 +457,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 +471,15 @@ 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 && prevContentSizeHeight.current < state.virtualizer.visibleRect.height) { 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..98c8a484774 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(3); + expect(onLoadMore).toHaveBeenCalledTimes(1); }); it('should automatically fire onLoadMore if there aren\'t enough items to fill the Table', function () { @@ -3890,11 +3907,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..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 c8fd7c9b5ef..36420c6db7e 100644 --- a/packages/@react-stately/table/src/useTableColumnResizeState.ts +++ b/packages/@react-stately/table/src/useTableColumnResizeState.ts @@ -41,7 +41,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?: (inRender: boolean) => void } interface ColumnState { @@ -49,25 +50,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); - function setColumnWidthsForRef(newWidths: Map) { + let prevWidths = useRef>(columnWidthsRef.current); + function setColumnWidthsForRef(newWidths: Map, inRender = false) { columnWidthsRef.current = newWidths; - // new map so that change detection is triggered - setColumnWidths(newWidths); + if (prevWidths.current !== newWidths && onResize) { + onResize(inRender); + prevWidths.current = newWidths; + } else if (!onResize) { + setColumnWidths(newWidths); + } } /* returns the resolved column width in this order: @@ -110,11 +116,10 @@ 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])) { + 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) {