diff --git a/packages/jsapi-components/src/useInitializeViewportData.test.ts b/packages/jsapi-components/src/useInitializeViewportData.test.ts index 99f378e486..a5c4055595 100644 --- a/packages/jsapi-components/src/useInitializeViewportData.test.ts +++ b/packages/jsapi-components/src/useInitializeViewportData.test.ts @@ -1,5 +1,5 @@ import { act, renderHook } from '@testing-library/react-hooks'; -import type { Table } from '@deephaven/jsapi-types'; +import type { dh } from '@deephaven/jsapi-types'; import { generateEmptyKeyedItems } from '@deephaven/jsapi-utils'; import { KeyedItem, TestUtils } from '@deephaven/utils'; import useInitializeViewportData from './useInitializeViewportData'; @@ -11,81 +11,128 @@ function expectedInitialItems(size: number) { return [...generateEmptyKeyedItems(0, size - 1)]; } -const tableA = TestUtils.createMockProxy({ size: 4 }); -const expectedInitialA: KeyedItem[] = expectedInitialItems( - tableA.size +const tableSize4 = TestUtils.createMockProxy({ size: 4 }); +const expectedInitial4: KeyedItem[] = expectedInitialItems( + tableSize4.size ); -const tableB = TestUtils.createMockProxy
({ size: 2 }); -const expectedInitialB = expectedInitialItems(tableB.size); +const tableSize2 = TestUtils.createMockProxy({ size: 2 }); +const expectedInitial2 = expectedInitialItems(tableSize2.size); -beforeEach(() => { - TestUtils.asMock(useTableSize).mockImplementation(table => table?.size ?? 0); -}); +function updatedItems(size: number): { key: string; item: string }[] { + const items: { key: string; item: string }[] = []; -it.each([null])('should safely handle no table: %s', noTable => { - const { result } = renderHook(() => useInitializeViewportData(noTable)); - expect(result.current.items).toEqual([]); -}); + for (let i = 0; i < size; i += 1) { + items.push({ key: `${i}`, item: `mock.item.${i}` }); + } -it('should initialize a ListData object based on Table size', () => { - const { result } = renderHook(() => useInitializeViewportData(tableA)); + return items; +} - expect(result.current.items).toEqual(expectedInitialA); +beforeEach(() => { + TestUtils.asMock(useTableSize).mockImplementation(table => table?.size ?? 0); }); -it('should re-initialize a ListData object if Table reference changes', () => { - const { result, rerender } = renderHook( - ({ table }) => useInitializeViewportData(table), - { - initialProps: { table: tableA }, - } - ); - - // Update an item - const updatedItem = { key: '0', item: 'mock.item' }; - act(() => { - result.current.update(updatedItem.key, updatedItem); - }); +describe.each([undefined, true, false])( + 'reuseItemsOnTableResize: %s', + reuseItemsOnTableResize => { + it.each([null])('should safely handle no table: %s', noTable => { + const { result } = renderHook(() => + useInitializeViewportData(noTable, reuseItemsOnTableResize) + ); + expect(result.current.items).toEqual([]); + }); - const expectedAfterUpdate = [updatedItem, ...expectedInitialA.slice(1)]; - expect(result.current.items).toEqual(expectedAfterUpdate); + it('should initialize a ListData object based on Table size', () => { + const { result } = renderHook(() => + useInitializeViewportData(tableSize4, reuseItemsOnTableResize) + ); - // Re-render with a new table instance - rerender({ table: tableB }); + expect(result.current.items).toEqual(expectedInitial4); + }); - const expectedAfterRerender = expectedInitialB; - expect(result.current.items).toEqual(expectedAfterRerender); -}); + it('should re-initialize a ListData object if Table reference changes', () => { + const { result, rerender } = renderHook( + ({ table }) => + useInitializeViewportData(table, reuseItemsOnTableResize), + { + initialProps: { table: tableSize4 }, + } + ); + + const updatedItems4 = updatedItems(tableSize4.size); + + // Update items + act(() => { + updatedItems4.forEach(updatedItem => { + result.current.update(updatedItem.key, updatedItem); + }); + }); + + expect(result.current.items).toEqual(updatedItems4); + + // Re-render with a smaller table instance + rerender({ table: tableSize2 }); + + expect(result.current.items).toEqual( + reuseItemsOnTableResize === true + ? updatedItems4.slice(0, tableSize2.size) + : expectedInitial2 + ); + + // Re-render with a larger table instance + rerender({ table: tableSize4 }); + + expect(result.current.items).toEqual( + reuseItemsOnTableResize === true + ? [ + ...updatedItems4.slice(0, tableSize2.size), + ...expectedInitial4.slice(tableSize2.size), + ] + : expectedInitial4 + ); + }); -it.each([ - [3, expectedInitialItems(3)], - [5, expectedInitialItems(5)], -])( - 'should re-initialize a ListData object if Table size changes: %s, %s', - (newSize, expectedAfterSizeChange) => { - const { result, rerender } = renderHook( - ({ table }) => useInitializeViewportData(table), - { - initialProps: { table: tableA }, + it.each([3, 5])( + 'should re-initialize a ListData object if Table size changes: %s', + newSize => { + const { result, rerender } = renderHook( + ({ table }) => + useInitializeViewportData(table, reuseItemsOnTableResize), + { + initialProps: { table: tableSize4 }, + } + ); + + expect(result.current.items).toEqual(expectedInitial4); + + const updatedItems4 = updatedItems(tableSize4.size); + + // Update items + act(() => { + updatedItems4.forEach(updatedItem => { + result.current.update(updatedItem.key, updatedItem); + }); + }); + + expect(result.current.items).toEqual(updatedItems4); + + // Re-render with new size + TestUtils.asMock(useTableSize).mockImplementation(() => newSize); + rerender({ + table: + tableSize4 /* this table is not actually used due to mocking `useTableSize` above */, + }); + + expect(result.current.items).toEqual( + reuseItemsOnTableResize === true + ? [ + ...updatedItems4.slice(0, newSize), + ...expectedInitialItems(newSize).slice(4), + ] + : expectedInitialItems(newSize) + ); } ); - - expect(result.current.items).toEqual(expectedInitialA); - - // Update an item - const updatedItem = { key: '0', item: 'mock.item' }; - act(() => { - result.current.update(updatedItem.key, updatedItem); - }); - - const expectedAfterUpdate = [updatedItem, ...expectedInitialA.slice(1)]; - expect(result.current.items).toEqual(expectedAfterUpdate); - - // Re-render with new size - TestUtils.asMock(useTableSize).mockImplementation(() => newSize); - rerender({ table: tableA }); - - expect(result.current.items).toEqual(expectedAfterSizeChange); } ); diff --git a/packages/jsapi-components/src/useInitializeViewportData.ts b/packages/jsapi-components/src/useInitializeViewportData.ts index b6a8c3114f..ec7652836e 100644 --- a/packages/jsapi-components/src/useInitializeViewportData.ts +++ b/packages/jsapi-components/src/useInitializeViewportData.ts @@ -45,7 +45,7 @@ function resizeItemsArray({ // Drop extra items if (currentSize > targetSize) { - return items.slice(0, targetSize - 1); + return items.slice(0, targetSize); } // Add missing items @@ -62,9 +62,17 @@ function resizeItemsArray({ * source table. This is intended for "human" sized tables such as those used in * admin panels. This is not suitable for "machine" scale with millions+ rows. * @param table The table that will be used to determine the list size. - * @param reuseItemsOnTableResize If true, the items will be reused when the - * table resizes. Defaults to false which will replace the items when the table - * resizes. + * @param reuseItemsOnTableResize Whether to reuse existing items when the table + * size changes (defaults to false). + * - If true, existing items will be reused when the table resizes. This is + * recommended for ticking tables where the data is frequently updated in order + * to avoid UI flicker. + * - If false, all of the items will be replaced when the table resizes. This is + * recommnded for tables that don't change size frequently but may change size + * due to a user interaction. e.g. Filter via a search input. This avoids a + * different kind of flicker, where the item values will shift around as the + * user types. It is less jarring for the items to all reset to empty and then + * show the new results all at once. * @returns a WindowedListData object. */ export function useInitializeViewportData(