diff --git a/desktop/flipper-plugin/src/data-source/DataSource.tsx b/desktop/flipper-plugin/src/data-source/DataSource.tsx index a53792265..3c1ac7a10 100644 --- a/desktop/flipper-plugin/src/data-source/DataSource.tsx +++ b/desktop/flipper-plugin/src/data-source/DataSource.tsx @@ -472,7 +472,7 @@ class DataSourceView { */ public windowEnd = 0; - private outputChangeListener?: (change: OutputChange) => void; + private outputChangeListeners = new Set<(change: OutputChange) => void>(); /** * This is the base view data, that is filtered and sorted, but not reversed or windowed @@ -520,11 +520,11 @@ class DataSourceView { this.windowEnd = end; } - public setListener(listener?: (change: OutputChange) => void) { - if (this.outputChangeListener && listener) { - console.warn('outputChangeListener already set'); - } - this.outputChangeListener = listener; + public addListener(listener: (change: OutputChange) => void) { + this.outputChangeListeners.add(listener); + return () => { + this.outputChangeListeners.delete(listener); + }; } public setSortBy(sortBy: undefined | keyof T | ((a: T) => Primitive)) { @@ -617,23 +617,27 @@ class DataSourceView { }; } + private notifyAllListeners(change: OutputChange) { + this.outputChangeListeners.forEach((listener) => listener(change)); + } + private notifyItemUpdated(viewIndex: number) { viewIndex = this.normalizeIndex(viewIndex); if ( - !this.outputChangeListener || + !this.outputChangeListeners.size || viewIndex < this.windowStart || viewIndex >= this.windowEnd ) { return; } - this.outputChangeListener({ + this.notifyAllListeners({ type: 'update', index: viewIndex, }); } private notifyItemShift(index: number, delta: number) { - if (!this.outputChangeListener) { + if (!this.outputChangeListeners.size) { return; } let viewIndex = this.normalizeIndex(index); @@ -641,7 +645,7 @@ class DataSourceView { viewIndex -= delta; // we need to correct for normalize already using the new length after applying this change } // Idea: we could add an option to automatically shift the window for before events. - this.outputChangeListener({ + this.notifyAllListeners({ type: 'shift', delta, index: viewIndex, @@ -656,7 +660,7 @@ class DataSourceView { } private notifyReset(count: number) { - this.outputChangeListener?.({ + this.notifyAllListeners({ type: 'reset', newCount: count, }); diff --git a/desktop/flipper-plugin/src/data-source/DataSourceRendererStatic.tsx b/desktop/flipper-plugin/src/data-source/DataSourceRendererStatic.tsx index 0cb97f195..4de2bfb8a 100644 --- a/desktop/flipper-plugin/src/data-source/DataSourceRendererStatic.tsx +++ b/desktop/flipper-plugin/src/data-source/DataSourceRendererStatic.tsx @@ -66,7 +66,7 @@ export const DataSourceRendererStatic: ( let unmounted = false; dataSource.view.setWindow(0, dataSource.limit); - dataSource.view.setListener((_event) => { + const unsubscribe = dataSource.view.addListener((_event) => { if (unmounted) { return; } @@ -75,7 +75,7 @@ export const DataSourceRendererStatic: ( return () => { unmounted = true; - dataSource.view.setListener(undefined); + unsubscribe(); }; }, [dataSource, setForceUpdate, useFixedRowHeight], diff --git a/desktop/flipper-plugin/src/data-source/DataSourceRendererVirtual.tsx b/desktop/flipper-plugin/src/data-source/DataSourceRendererVirtual.tsx index c045bf5b6..9e53b222a 100644 --- a/desktop/flipper-plugin/src/data-source/DataSourceRendererVirtual.tsx +++ b/desktop/flipper-plugin/src/data-source/DataSourceRendererVirtual.tsx @@ -170,7 +170,7 @@ export const DataSourceRendererVirtual: ( } } - dataSource.view.setListener((event) => { + const unsubscribe = dataSource.view.addListener((event) => { switch (event.type) { case 'reset': rerender(UpdatePrio.HIGH, true); @@ -201,7 +201,7 @@ export const DataSourceRendererVirtual: ( return () => { unmounted = true; - dataSource.view.setListener(undefined); + unsubscribe(); }; }, [dataSource, setForceUpdate, useFixedRowHeight, isUnitTest], diff --git a/desktop/flipper-plugin/src/data-source/__tests__/datasource-basics.node.tsx b/desktop/flipper-plugin/src/data-source/__tests__/datasource-basics.node.tsx index 58ce1320d..bc4e0df51 100644 --- a/desktop/flipper-plugin/src/data-source/__tests__/datasource-basics.node.tsx +++ b/desktop/flipper-plugin/src/data-source/__tests__/datasource-basics.node.tsx @@ -489,9 +489,9 @@ function testEvents( ): any[] { const ds = createDataSource(initial, {key}); const events: any[] = []; - ds.view.setListener((e) => events.push(e)); + const unsubscribe = ds.view.addListener((e) => events.push(e)); op(ds); - ds.view.setListener(undefined); + unsubscribe(); return events; } diff --git a/desktop/flipper-plugin/src/ui/MasterDetail.tsx b/desktop/flipper-plugin/src/ui/MasterDetail.tsx index 75c7a79aa..5406ea504 100644 --- a/desktop/flipper-plugin/src/ui/MasterDetail.tsx +++ b/desktop/flipper-plugin/src/ui/MasterDetail.tsx @@ -104,7 +104,8 @@ export function MasterDetail({ // if a tableManagerRef is provided, we piggy back on that same ref // eslint-disable-next-line - const tableManagerRef = tableProps.tableManagerRef ?? createRef>(); + const tableManagerRef = + tableProps.tableManagerRef ?? createRef>(); const pausedState = useValue(isPaused, false); diff --git a/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx b/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx index 461a70a0b..8ceec96c9 100644 --- a/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx @@ -54,6 +54,7 @@ import {debounce} from 'lodash'; import {useInUnitTest} from '../../utils/useInUnitTest'; import {createDataSource} from '../../state/createDataSource'; import {HighlightProvider} from '../Highlight'; +import {useLatestRef} from '../../utils/useLatestRef'; type DataTableBaseProps = { columns: DataTableColumn[]; @@ -179,6 +180,26 @@ export function DataTable( const {columns, selection, searchValue, sorting} = tableState; + const latestSelectionRef = useLatestRef(selection); + const latestOnSelectRef = useLatestRef(onSelect); + useEffect(() => { + if (dataSource) { + const unsubscribe = dataSource.view.addListener((change) => { + if ( + change.type === 'update' && + latestSelectionRef.current.items.has(change.index) + ) { + latestOnSelectRef.current?.( + getSelectedItem(dataSource, latestSelectionRef.current), + getSelectedItems(dataSource, latestSelectionRef.current), + ); + } + }); + + return unsubscribe; + } + }, [dataSource, latestSelectionRef, latestOnSelectRef]); + const visibleColumns = useMemo( () => columns.filter((column) => column.visible), [columns], diff --git a/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTable.node.tsx b/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTable.node.tsx index 066888924..66d5a5692 100644 --- a/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTable.node.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTable.node.tsx @@ -737,6 +737,8 @@ test('selection always has the latest state', () => { act(() => { ds.update(2, item3updated); }); + expect(events.splice(0)).toEqual([[item3updated, [item3updated]]]); + act(() => { ref.current!.addRangeToSelection(0, 0); }); @@ -745,5 +747,16 @@ test('selection always has the latest state', () => { [item1, [item1, item3updated]], // update reflected in callback! ]); + const item1updated = { + title: 'item 1 updated', + done: false, + }; + act(() => { + ds.update(0, item1updated); + }); + expect(events.splice(0)).toEqual([ + [item1updated, [item1updated, item3updated]], // update reflected in callback! + ]); + rendering.unmount(); });