fix: Use correct offset in snapshot (#2217)

- Was using `row.offsetInSnapshot`, which was a "protected" API and
recently removed from the JS API
- API was removed in JS API refactoring:
deephaven/deephaven-core#5890
- Instead just use the viewport `offset` and add the index of the row in
the snapshot, which is there in both versions of the API
- New API does support `row.index`, but this way is compatible with both
- Updated unit tests
- Tested using a deephaven.ui.list_view:
```python
from deephaven import time_table, ui
import datetime

initial_row_count = 200
column_types = time_table(
    "PT1S",
    start_time=datetime.datetime.now() - datetime.timedelta(seconds=initial_row_count),
).update(
    [
        "Id=new Integer(i)",
        "Display=new String(`Display `+i)",
    ]
)

@ui.component
def ui_list_view_table():
    value, set_value = ui.use_state([])

    lv = ui.list_view(
        column_types,
        aria_label="List View",
        on_change=set_value,
        selected_keys=value,
    )

    text = ui.text("Selection: " + ", ".join(map(str, value)))

    return ui.flex(
        lv,
        text,
        direction="column",
        margin=10,
        gap=10,
        width=500,
        # necessary to avoid overflowing container height
        min_height=0,
    )

lv_table = ui_list_view_table()
```

Author: mofojed

packages/jsapi-components/src/useViewportData.test.tsx

@@ -3,7 +3,6 @@ import type { dh as DhType } from '@deephaven/jsapi-types';
 import dh from '@deephaven/jsapi-shim';
 import {
   OnTableUpdatedEvent,
-  ViewportRow,
   generateEmptyKeyedItems,
   isClosed,
   ITEM_KEY_PREFIX,
@@ -23,13 +22,9 @@ jest.mock('@deephaven/react-hooks', () => ({
 jest.mock('./useSetPaddedViewportCallback');
 jest.mock('./useTableSize');
 
-function mockViewportRow(offsetInSnapshot: number): ViewportRow {
-  return { offsetInSnapshot } as ViewportRow;
-}
-
 function mockUpdateEvent(
   offset: number,
-  rows: ViewportRow[]
+  rows: DhType.Row[]
 ): OnTableUpdatedEvent {
   return {
     detail: {
@@ -213,14 +208,14 @@ it('should update state on dh.Table.EVENT_UPDATED event', () => {
   );
 
   const offset = 3;
-  const row = mockViewportRow(5);
+  const row = TestUtils.createMockProxy<DhType.Row>();
   const event = mockUpdateEvent(offset, [row]);
 
   act(() => {
     updateEventHandler?.(event);
   });
 
-  const expectedKeyIndex = offset + row.offsetInSnapshot;
+  const expectedKeyIndex = offset;
   const expectedInitialItems = [...generateEmptyKeyedItems(0, table.size - 1)];
   const expectedItems = [
     ...expectedInitialItems.slice(0, expectedKeyIndex),

packages/jsapi-utils/src/ViewportDataUtils.test.ts

@@ -5,8 +5,6 @@ import {
   ITEM_KEY_PREFIX,
   OnTableUpdatedEvent,
   RowDeserializer,
-  ViewportRow,
-  createKeyFromOffsetRow,
   createOnTableUpdatedHandler,
   defaultRowDeserializer,
   generateEmptyKeyedItems,
@@ -18,10 +16,6 @@ import {
 
 const { asMock, createMockProxy } = TestUtils;
 
-function mockViewportRow(offsetInSnapshot: number): ViewportRow {
-  return { offsetInSnapshot } as ViewportRow;
-}
-
 function mockColumn(name: string) {
   return {
     name,
@@ -44,28 +38,15 @@ describe('createdKeyedItemKey', () => {
   });
 });
 
-describe('createKeyFromOffsetRow', () => {
-  it.each([
-    [{ offsetInSnapshot: 4 } as ViewportRow, 5, `${ITEM_KEY_PREFIX}_9`],
-    [{ offsetInSnapshot: 27 } as ViewportRow, 99, `${ITEM_KEY_PREFIX}_126`],
-  ] as const)(
-    'should create a string key based on the actual row offset: %o',
-    (row, offset, expected) => {
-      const actual = createKeyFromOffsetRow(row, offset);
-      expect(actual).toEqual(expected);
-    }
-  );
-});
-
 describe('createOnTableUpdatedHandler', () => {
   const mock = {
     deserializeRow: jest.fn() as RowDeserializer<unknown>,
     rows: [
-      createMockProxy<ViewportRow>({ offsetInSnapshot: 0 }),
-      createMockProxy<ViewportRow>({ offsetInSnapshot: 1 }),
-      createMockProxy<ViewportRow>({ offsetInSnapshot: 2 }),
+      createMockProxy<dh.Row>(),
+      createMockProxy<dh.Row>(),
+      createMockProxy<dh.Row>(),
     ],
-    updateEvent: (offset: number, rows: ViewportRow[], columns: dh.Column[]) =>
+    updateEvent: (offset: number, rows: dh.Row[], columns: dh.Column[]) =>
       createMockProxy<OnTableUpdatedEvent>({
         detail: {
           offset,
@@ -107,10 +88,11 @@ describe('createOnTableUpdatedHandler', () => {
 
 describe('defaultRowDeserializer', () => {
   it('should map all columns with original names', () => {
-    const row = mockViewportRow(10);
-    // mock our get function by mapping capital column name to lowercase value
-    // e.g. A: 'a'
-    row.get = jest.fn(({ name }: { name: string }) => name.toLowerCase());
+    const row = createMockProxy<dh.Row>({
+      // mock our get function by mapping capital column name to lowercase value
+      // e.g. A: 'a'
+      get: jest.fn(({ name }: { name: string }) => name.toLowerCase()),
+    });
 
     const actual = defaultRowDeserializer(row, [
       mockColumn('A'),

packages/jsapi-utils/src/ViewportDataUtils.ts

@@ -9,12 +9,10 @@ export const ITEM_KEY_PREFIX = 'DH_ITEM_KEY';
 export type OnTableUpdatedEvent = CustomEvent<{
   offset: number;
   columns: dh.Column[];
-  rows: ViewportRow[];
+  rows: dh.Row[];
 }>;
 
-export type RowDeserializer<T> = (row: ViewportRow, columns: dh.Column[]) => T;
-
-export type ViewportRow = dh.Row & { offsetInSnapshot: number };
+export type RowDeserializer<T> = (row: dh.Row, columns: dh.Column[]) => T;
 
 const log = Log.module('ViewportDataUtils');
 
@@ -29,21 +27,6 @@ export function createKeyedItemKey(index: number): string {
   return `${ITEM_KEY_PREFIX}_${index}`;
 }
 
-/**
- * Create a unique string key for a row based on its ordinal position in its
- * source table. This is calculated based on it's offset in the viewport
- * (row.offsetInSnapshot) + the offset of the snapshot.
- * @param row Row from a Table update event.
- * @param offset Offset of the current viewport.
- * @returns Unique string key for the ordinal position of the given row.
- */
-export function createKeyFromOffsetRow(
-  row: ViewportRow,
-  offset: number
-): string {
-  return createKeyedItemKey(row.offsetInSnapshot + offset);
-}
-
 /**
  * Creates a handler function for a `dh.Table.EVENT_UPDATED` event. Rows that
  * get passed to the handler will be bulk updated in the given `viewportData`
@@ -66,9 +49,9 @@ export function createOnTableUpdatedHandler<T>(
 
     const updateKeyMap = new Map<Key, KeyedItem<T>>();
 
-    rows.forEach(row => {
+    rows.forEach((row, offsetInSnapshot) => {
       const item = deserializeRow(row, columns);
-      const key = createKeyFromOffsetRow(row, offset);
+      const key = createKeyedItemKey(offset + offsetInSnapshot);
       updateKeyMap.set(key, { key, item });
     });
 
@@ -86,7 +69,7 @@ export function createOnTableUpdatedHandler<T>(
  * @returns A key / value object for the row.
  */
 export function defaultRowDeserializer<T>(
-  row: ViewportRow,
+  row: dh.Row,
   columns: dh.Column[]
 ): T {
   return columns.reduce((result, col) => {