Skip to content

Commit

Permalink
fix: Use correct offset in snapshot
Browse files Browse the repository at this point in the history
- 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()
```
  • Loading branch information
mofojed committed Sep 11, 2024
1 parent 99b8d59 commit ced37e5
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 49 deletions.
36 changes: 9 additions & 27 deletions packages/jsapi-utils/src/ViewportDataUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import {
ITEM_KEY_PREFIX,
OnTableUpdatedEvent,
RowDeserializer,
ViewportRow,
createKeyFromOffsetRow,
createOnTableUpdatedHandler,
defaultRowDeserializer,
generateEmptyKeyedItems,
Expand All @@ -18,10 +16,6 @@ import {

const { asMock, createMockProxy } = TestUtils;

function mockViewportRow(offsetInSnapshot: number): ViewportRow {
return { offsetInSnapshot } as ViewportRow;
}

function mockColumn(name: string) {
return {
name,
Expand All @@ -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,
Expand Down Expand Up @@ -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'),
Expand Down
27 changes: 5 additions & 22 deletions packages/jsapi-utils/src/ViewportDataUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -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`
Expand All @@ -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 });
});

Expand All @@ -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) => {
Expand Down

0 comments on commit ced37e5

Please sign in to comment.