fix: Added getKey to SelectionUtils.optimizeSelection

packages/jsapi-components/src/useDebouncedViewportSelectionFilter.test.ts

@@ -1,5 +1,6 @@
 import { renderHook } from '@testing-library/react-hooks';
 import {
+  createKeyedItemKey,
   createSelectedValuesFilter,
   FilterConditionFactory,
   TableUtils,
@@ -82,7 +83,8 @@ it('should map selection to values', () => {
 
   expect(useMappedSelection).toHaveBeenCalledWith(
     mockViewportData,
-    mapItemToValue
+    mapItemToValue,
+    createKeyedItemKey
   );
 });
 

packages/jsapi-components/src/useDebouncedViewportSelectionFilter.ts

@@ -1,4 +1,5 @@
 import {
+  createKeyedItemKey,
   createSelectedValuesFilter,
   FilterConditionFactory,
 } from '@deephaven/jsapi-utils';
@@ -40,7 +41,11 @@ export function useDebouncedViewportSelectionFilter<TItem, TValue>({
   const tableUtils = useTableUtils();
 
   // Map selection to values contained in the column to filter
-  const valuesSelection = useMappedSelection(viewportData, mapItemToValue);
+  const valuesSelection = useMappedSelection(
+    viewportData,
+    mapItemToValue,
+    createKeyedItemKey
+  );
 
   // Debounce so user can rapidly select multiple items in a row without the
   // cost of updating the table on each change

packages/react-hooks/src/SelectionUtils.test.ts

@@ -122,11 +122,13 @@ describe('mapSelection', () => {
 });
 
 describe('optimizeSelection', () => {
+  const getKey = (i: number) => 'abcdefg'.charAt(i);
+
   it('should invert selection if selection is "all"', () => {
     const selection = 'all';
     const totalRecords = 10;
 
-    const actual = optimizeSelection(selection, totalRecords);
+    const actual = optimizeSelection(selection, totalRecords, getKey);
 
     expect(actual).toEqual({
       isInverted: true,
@@ -137,16 +139,16 @@ describe('optimizeSelection', () => {
   it.each([
     // Odd record count
     [new Set(''), 5, { isInverted: false, selection: new Set('') }],
-    [new Set('12'), 5, { isInverted: false, selection: new Set('12') }],
-    [new Set('123'), 5, { isInverted: true, selection: new Set('04') }],
+    [new Set('bc'), 5, { isInverted: false, selection: new Set('bc') }],
+    [new Set('bcd'), 5, { isInverted: true, selection: new Set('ae') }],
     // Even record count
     [new Set(''), 6, { isInverted: false, selection: new Set('') }],
-    [new Set('123'), 6, { isInverted: false, selection: new Set('123') }],
-    [new Set('1234'), 6, { isInverted: true, selection: new Set('05') }],
+    [new Set('bcd'), 6, { isInverted: false, selection: new Set('bcd') }],
+    [new Set('bcde'), 6, { isInverted: true, selection: new Set('af') }],
   ] as const)(
     'should invert selection if selection size > half the total size',
     (selection, totalRecords, expected) => {
-      const actual = optimizeSelection(selection, totalRecords);
+      const actual = optimizeSelection(selection, totalRecords, getKey);
 
       expect(actual).toEqual(expected);
     }

packages/react-hooks/src/SelectionUtils.ts

@@ -94,10 +94,12 @@ export function mapSelection<TItem, TMap>(
  * "no filter".
  * @param selection The selection to optimize
  * @param totalRecords The total number of records that can potentially be selected
+ * @param getKey A function to get the key for a given index.
  */
 export function optimizeSelection(
   selection: Selection,
-  totalRecords: number
+  totalRecords: number,
+  getKey: (i: number) => string
 ): { selection: Selection; isInverted: boolean } {
   const isInverted = selection === 'all' || selection.size > totalRecords / 2;
 
@@ -110,8 +112,8 @@ export function optimizeSelection(
         : new Set<Key>(
             // Create a new set from any key that is not selected
             [...generateRange(0, totalRecords - 1)]
-              .filter(i => !selection.has(String(i)))
-              .map(i => String(i))
+              .map(getKey)
+              .filter(key => !selection.has(key))
           );
   }
 

packages/react-hooks/src/useMappedSelection.test.ts

@@ -40,13 +40,16 @@ beforeEach(() => {
 });
 
 it('should optimize selection and map it to another selection', () => {
+  const getKey = jest.fn().mockName('getKey');
+
   const { result } = renderHook(() =>
-    useMappedSelection(viewportData, mapName)
+    useMappedSelection(viewportData, mapName, getKey)
   );
 
   expect(optimizeSelection).toHaveBeenCalledWith(
     viewportData.selectedKeys,
-    viewportData.items.length
+    viewportData.items.length,
+    getKey
   );
 
   expect(mapSelection).toHaveBeenCalledWith(

packages/react-hooks/src/useMappedSelection.ts

@@ -8,24 +8,27 @@ import type { WindowedListData } from './useWindowedListData';
  * The keys are mapped to new values via a `mapItem` function.
  * @param viewportData The viewport to map selections for
  * @param mapItem A function to map an item in the viewport to a new value
+ * @param getKey A function to get the key for an item in the viewport.
  */
 export function useMappedSelection<TItem, TValue>(
   viewportData: WindowedListData<KeyedItem<TItem>>,
-  mapItem: (item: KeyedItem<TItem>) => TValue
+  mapItem: (item: KeyedItem<TItem>) => TValue,
+  getKey: (i: number) => string
 ): SelectionMaybeInverted<TValue> {
   const { getItem, selectedKeys, items } = viewportData;
 
   return useMemo(() => {
     const { selection, isInverted } = optimizeSelection(
       selectedKeys,
-      items.length
+      items.length,
+      getKey
     );
 
     return {
       selection: mapSelection(selection, getItem, mapItem),
       isInverted,
     };
-  }, [selectedKeys, items.length, getItem, mapItem]);
+  }, [selectedKeys, items.length, getKey, getItem, mapItem]);
 }
 
 export default useMappedSelection;