From 3da9a9012a880620e436f58d20f86f53b2b0429d Mon Sep 17 00:00:00 2001 From: Sylvain Lesage Date: Tue, 14 Jan 2025 00:43:25 +0100 Subject: [PATCH 01/15] add aria attributes, change left cells from td to th, add test on column sort --- src/HighTable.css | 18 +++++++-------- src/HighTable.tsx | 26 ++++++++++++--------- src/TableHeader.tsx | 3 ++- test/HighTable.test.tsx | 51 +++++++++++++++++++++++++++++++++++++++-- 4 files changed, 75 insertions(+), 23 deletions(-) diff --git a/src/HighTable.css b/src/HighTable.css index 55b8340..1f6578a 100644 --- a/src/HighTable.css +++ b/src/HighTable.css @@ -56,7 +56,7 @@ cursor: default; width: 0; } -.table tbody tr:first-child td { +.table tbody tr:first-child td, .table tbody tr:first-child th { border-top: 1px solid transparent; } @@ -89,7 +89,7 @@ } /* row numbers */ -.table td:first-child { +.table th:first-child { background-color: #eaeaeb; border-right: 1px solid #ddd; color: #888; @@ -104,23 +104,23 @@ width: 32px; cursor: pointer; } -.table td:first-child span { +.table th:first-child span { display: inline; } -.table td:first-child input { +.table th:first-child input { display: none; } -.selectable td:first-child:hover span, .selectable tr.selected td:first-child span { +.selectable th:first-child:hover span, .selectable tr.selected th:first-child span { display: none; } -.selectable td:first-child:hover input, .selectable tr.selected td:first-child input { +.selectable th:first-child:hover input, .selectable tr.selected th:first-child input { display: inline; cursor: pointer; } .selectable tr.selected { background-color: #fbf7bf; } -.selectable tr.selected td:first-child { +.selectable tr.selected th:first-child { background-color: #f1edbb; } @@ -175,7 +175,7 @@ } /* pending table state */ -.table th::before { +.table thead th::before { content: ''; position: absolute; top: 0; @@ -185,7 +185,7 @@ background-color: #706fb1; z-index: 100; } -.pending .table th::before { +.pending .table thead th::before { animation: shimmer 2s infinite linear; } @keyframes shimmer { diff --git a/src/HighTable.tsx b/src/HighTable.tsx index 5adcfce..83e1276 100644 --- a/src/HighTable.tsx +++ b/src/HighTable.tsx @@ -356,8 +356,9 @@ export default function HighTable({
{prePadding.map((_, prePaddingIndex) => { const tableIndex = startIndex - prePadding.length + prePaddingIndex - return - + })} {rows.map((row, sliceIndex) => { const { tableIndex, dataIndex } = getRowIndexes(sliceIndex) - return - + {data.header.map((col, colIndex) => Cell(row[col], colIndex, dataIndex) )} @@ -402,14 +406,14 @@ export default function HighTable({ })} {postPadding.map((_, postPaddingIndex) => { const tableIndex = startIndex + rows.length + postPaddingIndex - return - + })} diff --git a/src/TableHeader.tsx b/src/TableHeader.tsx index 2bc8e5a..486f777 100644 --- a/src/TableHeader.tsx +++ b/src/TableHeader.tsx @@ -146,10 +146,11 @@ export default function TableHeader({ const memoizedStyles = useMemo(() => columnWidths.map(cellStyle), [columnWidths]) return - + {header.map((columnHeader, columnIndex) => {data.header.map((col, colIndex) => Cell(row[col], colIndex, dataIndex) @@ -422,7 +422,7 @@ export default function HighTable({
selectable && dispatch({ type: 'SET_SELECTION', selection: toggleAll({ selection, length: rows.length }), anchor: undefined })}>   - +
 
From 727168c506f36b76f336bfc384c7648b5fbef587 Mon Sep 17 00:00:00 2001 From: Sylvain Lesage Date: Tue, 14 Jan 2025 19:31:34 +0100 Subject: [PATCH 03/15] mandatory __index__ in rows --- src/HighTable.tsx | 73 ++++++++++++++++------------------------- src/dataframe.ts | 39 ++++++++++++++-------- test/HighTable.test.tsx | 27 +++++++++++++-- test/dataframe.test.ts | 27 +++++++-------- test/rowCache.test.ts | 21 +++++++----- 5 files changed, 105 insertions(+), 82 deletions(-) diff --git a/src/HighTable.tsx b/src/HighTable.tsx index 742a482..76ad2c4 100644 --- a/src/HighTable.tsx +++ b/src/HighTable.tsx @@ -1,5 +1,5 @@ import { ReactNode, useCallback, useEffect, useMemo, useReducer, useRef } from 'react' -import { AsyncRow, DataFrame, Row, asyncRows } from './dataframe.js' +import { DataFrame, Row, asyncRows } from './dataframe.js' import { Selection, areAllSelected, extendFromAnchor, isSelected, toggleAll, toggleIndex } from './selection.js' import TableHeader, { cellStyle } from './TableHeader.js' export { @@ -49,7 +49,7 @@ type State = { columnWidths: Array // width of each column invalidate: boolean // true if the data must be fetched again hasCompleteRow: boolean // true if at least one row is fully resolved (all of its cells) - rows: AsyncRow[] // slice of the virtual table rows (sorted rows) to render as HTML + rows: (Row | undefined)[] // slice of the virtual table rows (sorted rows) to render as HTML - undefined means pending startIndex: number // offset of the slice of sorted rows to render (rows[0] is the startIndex'th sorted row) orderBy?: string // column name to sort by selection: Selection // rows selection. The values are indexes of the virtual table (sorted rows), and thus depend on the order. @@ -57,7 +57,7 @@ type State = { } type Action = - | { type: 'SET_ROWS', start: number, rows: AsyncRow[], hasCompleteRow: boolean } + | { type: 'SET_ROWS', start: number, rows: (Row | undefined)[], hasCompleteRow: boolean } | { type: 'SET_COLUMN_WIDTH', columnIndex: number, columnWidth: number | undefined } | { type: 'SET_COLUMN_WIDTHS', columnWidths: Array } | { type: 'SET_ORDER', orderBy: string | undefined } @@ -180,14 +180,20 @@ export default function HighTable({ try { const requestId = ++pendingRequest.current const unwrapped = data.rows(start, end, orderBy) - const rows = asyncRows(unwrapped, end - start, data.header) + const rowsChunk = asyncRows(unwrapped, end - start, data.header) const updateRows = throttle(() => { - const resolved: Row[] = [] + const resolved: (Row | undefined)[] = [] let hasCompleteRow = false // true if at least one row is fully resolved - for (const row of rows) { + for (const row of rowsChunk) { + const __index__ = row.__index__.resolved + if (__index__ === undefined) { + // This is a pending row + resolved.push(undefined) + continue + } // Return only resolved values - const resolvedRow: Row = {} + const resolvedRow: Row = { __index__ } let isRowComplete = true for (const [key, promise] of Object.entries(row)) { if ('resolved' in promise) { @@ -205,8 +211,8 @@ export default function HighTable({ updateRows() // initial update // Subscribe to data updates - for (const row of rows) { - for (const [key, promise] of Object.entries(row)) { + for (const row of rowsChunk) { + for (const [_, promise] of Object.entries(row)) { promise.then(() => { if (pendingRequest.current === requestId) { updateRows() @@ -216,7 +222,7 @@ export default function HighTable({ } // Await all pending promises - for (const row of rows) { + for (const row of rowsChunk) { for (const promise of Object.values(row)) { await promise } @@ -263,9 +269,11 @@ export default function HighTable({ /** * Validate row length */ - function rowError(row: Record, dataIndex: number): string | undefined { - if (row.length > 0 && row.length !== data.header.length) { - return `Row ${rowLabel(dataIndex)} length ${row.length} does not match header length ${data.header.length}` + function rowError(row?: Row): string | undefined { + if (!row) return 'Pending' + const numKeys = Object.keys(row).length - 1 // exclude __index__ + if (numKeys > 0 && numKeys !== data.header.length) { + return `Row ${rowLabel(row.__index__)} length ${numKeys} does not match header length ${data.header.length}` } } @@ -304,28 +312,6 @@ export default function HighTable({ } }, [focus]) - /** - * Get the row index in original (unsorted) data frame, and in the sorted virtual table. - * - * @param sliceIndex row index in the "rows" slice - * - * @returns an object with two properties: - * dataIndex: row index in the original (unsorted) data frame - * tableIndex: row index in the virtual table (sorted) - */ - const getRowIndexes = useCallback((sliceIndex: number): { dataIndex: number, tableIndex: number } => { - const tableIndex = startIndex + sliceIndex - /// TODO(SL): improve row typing to get __index__ type if sorted - /// Maybe even better to always have an __index__, sorted or not - const index = rows[sliceIndex].__index__ - const resolved = typeof index === 'object' ? index.resolved : index - return { - dataIndex: resolved ?? tableIndex, // .__index__ only exists if the rows are sorted. If not sorted, use the table index - tableIndex, - } - }, [rows, startIndex]) - - const onRowNumberClick = useCallback(({ useAnchor, tableIndex }: {useAnchor: boolean, tableIndex: number}) => { if (!selectable) return false if (useAnchor) { @@ -376,7 +362,7 @@ export default function HighTable({ {prePadding.map((_, prePaddingIndex) => { const tableIndex = startIndex - prePadding.length + prePaddingIndex - return + return })} {rows.map((row, sliceIndex) => { - const { tableIndex, dataIndex } = getRowIndexes(sliceIndex) - return - {data.header.map((col, colIndex) => - Cell(row[col], colIndex, dataIndex) + {row && data.header.map((col, colIndex) => + Cell(row[col], colIndex, row.__index__) )} })} {postPadding.map((_, postPaddingIndex) => { const tableIndex = startIndex + rows.length + postPaddingIndex - return + return {prePadding.map((_, prePaddingIndex) => { const tableIndex = startIndex - prePadding.length + prePaddingIndex - return + return })} {rows.map((row, sliceIndex) => { const tableIndex = startIndex + sliceIndex - return @@ -389,12 +388,11 @@ export default function HighTable({ })} {postPadding.map((_, postPaddingIndex) => { const tableIndex = startIndex + rows.length + postPaddingIndex - return + return From abb60f9e705e3f18e1c633891806828b7e627f8e Mon Sep 17 00:00:00 2001 From: Sylvain Lesage Date: Tue, 14 Jan 2025 23:08:33 +0100 Subject: [PATCH 05/15] expand text --- src/HighTable.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/HighTable.tsx b/src/HighTable.tsx index 9a07873..8fa60dc 100644 --- a/src/HighTable.tsx +++ b/src/HighTable.tsx @@ -270,7 +270,7 @@ export default function HighTable({ * Validate row length */ function rowError(row?: Row): string | undefined { - if (!row) return 'Pending' + if (!row) return 'Loading the row contents...' const numKeys = Object.keys(row).length - 1 // exclude __index__ if (numKeys > 0 && numKeys !== data.header.length) { return `Row ${rowLabel(row.__index__)} length ${numKeys} does not match header length ${data.header.length}` From 7d138f64a9437b9a60b1b56653ecb85f20cc3365 Mon Sep 17 00:00:00 2001 From: Sylvain Lesage Date: Tue, 14 Jan 2025 23:10:47 +0100 Subject: [PATCH 06/15] no need for a callback --- src/HighTable.tsx | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/HighTable.tsx b/src/HighTable.tsx index 8fa60dc..35da1c7 100644 --- a/src/HighTable.tsx +++ b/src/HighTable.tsx @@ -107,6 +107,11 @@ const initialState: State = { selection: [], } +function rowLabel(rowIndex: number): string { + // rowIndex + 1 because the displayed row numbers are 1-based + return (rowIndex + 1).toLocaleString() +} + /** * Render a table with streaming rows on demand from a DataFrame. */ @@ -259,13 +264,6 @@ export default function HighTable({ } }, [tableControl]) - const rowLabel = useCallback((rowIndex: number): string => { - // rowIndex + 1 because the displayed row numbers are 1-based - return (rowIndex + 1).toLocaleString() - }, [ - // no dependencies, but we could add a setting to allow 0-based row numbers - ]) - /** * Validate row length */ From 00c80d199c21dccf1253263bda50874d13b67028 Mon Sep 17 00:00:00 2001 From: Sylvain Lesage Date: Tue, 14 Jan 2025 23:12:34 +0100 Subject: [PATCH 07/15] improve comment --- src/dataframe.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/dataframe.ts b/src/dataframe.ts index 8333f65..89b7abd 100644 --- a/src/dataframe.ts +++ b/src/dataframe.ts @@ -118,7 +118,8 @@ export function sortableDataFrame(data: DataFrame): DataFrame { rows(start: number, end: number, orderBy?: string): AsyncRow[] | Promise { if (orderBy) { if (!data.header.includes(orderBy)) { - /// TODO(SL): should we allow '__index__'? + // '__index__' is not allowed, and it would not make sense anyway + // as it's the same as orderBy=undefined, since we only support ascending order throw new Error(`Invalid orderBy field: ${orderBy}`) } if (!all) { From 3bf259d7c5d64a0c846e9b86d9d5e8cd2de1824b Mon Sep 17 00:00:00 2001 From: Sylvain Lesage Date: Wed, 15 Jan 2025 17:07:26 +0100 Subject: [PATCH 08/15] remove breaking change on Row / AsyncRow definitions if __index__ is not present in sorted rows, we: - don't show the number in the left column - cancel the callbacks on mouse down click and double click on a cell --- src/HighTable.tsx | 43 +++++++++++++++++++---------------------- src/dataframe.ts | 39 +++++++++++++++---------------------- test/HighTable.test.tsx | 2 -- test/dataframe.test.ts | 21 ++++++++++---------- test/rowCache.test.ts | 21 +++++++++----------- 5 files changed, 55 insertions(+), 71 deletions(-) diff --git a/src/HighTable.tsx b/src/HighTable.tsx index 35da1c7..670eada 100644 --- a/src/HighTable.tsx +++ b/src/HighTable.tsx @@ -49,7 +49,7 @@ type State = { columnWidths: Array // width of each column invalidate: boolean // true if the data must be fetched again hasCompleteRow: boolean // true if at least one row is fully resolved (all of its cells) - rows: (Row | undefined)[] // slice of the virtual table rows (sorted rows) to render as HTML - undefined means pending + rows: Row[] // slice of the virtual table rows (sorted rows) to render as HTML. It might contain incomplete rows, and __index__ might be missing. startIndex: number // offset of the slice of sorted rows to render (rows[0] is the startIndex'th sorted row) orderBy?: string // column name to sort by selection: Selection // rows selection. The values are indexes of the virtual table (sorted rows), and thus depend on the order. @@ -57,7 +57,7 @@ type State = { } type Action = - | { type: 'SET_ROWS', start: number, rows: (Row | undefined)[], hasCompleteRow: boolean } + | { type: 'SET_ROWS', start: number, rows: Row[], hasCompleteRow: boolean } | { type: 'SET_COLUMN_WIDTH', columnIndex: number, columnWidth: number | undefined } | { type: 'SET_COLUMN_WIDTHS', columnWidths: Array } | { type: 'SET_ORDER', orderBy: string | undefined } @@ -107,7 +107,8 @@ const initialState: State = { selection: [], } -function rowLabel(rowIndex: number): string { +function rowLabel(rowIndex?: number): string { + if (rowIndex === undefined) return '' // rowIndex + 1 because the displayed row numbers are 1-based return (rowIndex + 1).toLocaleString() } @@ -188,19 +189,14 @@ export default function HighTable({ const rowsChunk = asyncRows(unwrapped, end - start, data.header) const updateRows = throttle(() => { - const resolved: (Row | undefined)[] = [] + const resolved: Row[] = [] let hasCompleteRow = false // true if at least one row is fully resolved for (const row of rowsChunk) { - const __index__ = row.__index__.resolved - if (__index__ === undefined) { - // This is a pending row - resolved.push(undefined) - continue - } // Return only resolved values - const resolvedRow: Row = { __index__ } + const resolvedRow: Row = {} let isRowComplete = true for (const [key, promise] of Object.entries(row)) { + // it might or not include __index__ if ('resolved' in promise) { resolvedRow[key] = promise.resolved } else { @@ -267,11 +263,11 @@ export default function HighTable({ /** * Validate row length */ - function rowError(row?: Row): string | undefined { - if (!row) return 'Loading the row contents...' - const numKeys = Object.keys(row).length - 1 // exclude __index__ + function rowError(row: Row, index?: number): string | undefined { + // __index__ is considered a reserved field - an error will be displayed if a column is named '__index__' in data.header + const numKeys = Object.keys(row).filter(d => d !== '__index__').length if (numKeys > 0 && numKeys !== data.header.length) { - return `Row ${rowLabel(row.__index__)} length ${numKeys} does not match header length ${data.header.length}` + return `Row ${rowLabel(index)} length ${numKeys} does not match header length ${data.header.length}` } } @@ -282,9 +278,9 @@ export default function HighTable({ * * @param value cell value * @param col column index - * @param row row index in the original (unsorted) data frame + * @param row row index. Can be undefined. */ - function Cell(value: any, col: number, row: number): ReactNode { + function Cell(value: any, col: number, row?: number): ReactNode { // render as truncated text let str = stringify(value) let title: string | undefined @@ -295,8 +291,8 @@ export default function HighTable({ return - {row && data.header.map((col, colIndex) => - Cell(row[col], colIndex, row.__index__) + {data.header.map((col, colIndex) => + Cell(row[col], colIndex, index) )} })} diff --git a/src/dataframe.ts b/src/dataframe.ts index 89b7abd..c829a2b 100644 --- a/src/dataframe.ts +++ b/src/dataframe.ts @@ -7,18 +7,13 @@ type WrappedPromise = Promise & { * A row where each cell is a promise. * The promise must be wrapped with `wrapPromise` so that HighTable can render * the state synchronously. - * - * `__index__` is a promise that resolves to the row index in the "original data". Beware of - * the collisions with the header keys. */ -export type AsyncRow = Record> & { __index__: WrappedPromise } +export type AsyncRow = Record> /** * A row where each cell is a resolved value. - * - * `__index__` is the row index in the "original data". Beware of the collisions with the header keys. */ -export type Row = Record & { __index__: number } +export type Row = Record /** * Streamable row data @@ -53,7 +48,10 @@ export function asyncRows(rows: AsyncRow[] | Promise, numRows: number, he for (const key of header) { wrapped[i][key].resolve(row[key]) } - wrapped[i].__index__.resolve(row.__index__) + // resolve the row index if present + if ('__index__' in row) { + wrapped[i].__index__.resolve(row.__index__) + } } }).catch(error => { // Reject all promises on error @@ -61,7 +59,6 @@ export function asyncRows(rows: AsyncRow[] | Promise, numRows: number, he for (const key of header) { wrapped[i][key].reject(error) } - wrapped[i].__index__.reject(error) } }) return wrapped @@ -111,7 +108,7 @@ export function resolvablePromise(): ResolvablePromise { */ export function sortableDataFrame(data: DataFrame): DataFrame { if (data.sortable) return data // already sortable - // Fetch all rows and set index + // Fetch all rows and add __index__ column let all: Promise return { ...data, @@ -123,8 +120,10 @@ export function sortableDataFrame(data: DataFrame): DataFrame { throw new Error(`Invalid orderBy field: ${orderBy}`) } if (!all) { - // Fetch all rows + // Fetch all rows and add __index__ column + // Note that it will erase any existing __index__ column all = awaitRows(data.rows(0, data.numRows)) + .then(rows => rows.map((row, i) => ({ __index__: i, ...row }))) } const sorted = all.then(all => { return all.sort((a, b) => { @@ -145,13 +144,9 @@ export function sortableDataFrame(data: DataFrame): DataFrame { /** * Await all promises in an AsyncRow and return resolved row. */ -export async function awaitRow(row: AsyncRow): Promise { - const otherEntries = Object.entries(row).filter( ([key]) => key !== '__index__' ) - const [ __index__, ...values ] = await Promise.all([row.__index__, ...otherEntries.map( ([_, value]) => value )]) - return Object.fromEntries([ - ['__index__', __index__], - ...otherEntries.map(([key, _], i) => [key, values[i]]), - ]) +export function awaitRow(row: AsyncRow): Promise { + return Promise.all(Object.values(row)) + .then(values => Object.fromEntries(Object.keys(row).map((key, i) => [key, values[i]]))) } /** @@ -162,15 +157,13 @@ export function awaitRows(rows: AsyncRow[] | Promise): Promise { return Promise.all(rows.map(awaitRow)) } -export function arrayDataFrame(data: Record[]): DataFrame { +export function arrayDataFrame(data: Row[]): DataFrame { if (!data.length) return { header: [], numRows: 0, rows: () => Promise.resolve([]) } return { - header: Object.keys(data[0]).filter(key => key !== '__index__'), + header: Object.keys(data[0]), numRows: data.length, rows(start: number, end: number): Promise { - return Promise.resolve(data.slice(start, end).map( - (row, i) => ({ ...row, __index__: i + start }) - )) + return Promise.resolve(data.slice(start, end)) }, } } diff --git a/test/HighTable.test.tsx b/test/HighTable.test.tsx index 2bd9399..9ca010d 100644 --- a/test/HighTable.test.tsx +++ b/test/HighTable.test.tsx @@ -10,7 +10,6 @@ describe('HighTable', () => { numRows: 100, rows: vi.fn((start, end) => Promise.resolve( Array.from({ length: end - start }, (_, index) => ({ - __index__: index + start, ID: index + start, Name: 'Name ' + (index + start), Age: 20 + index % 50, @@ -99,7 +98,6 @@ describe('When sorted, HighTable', () => { numRows: 1000, rows: (start: number, end: number) => Promise.resolve( Array.from({ length: end - start }, (_, index) => ({ - __index__: index + start, ID: 'row ' + (index + start), Count: 1000 - start - index, })) diff --git a/test/dataframe.test.ts b/test/dataframe.test.ts index 9e490b4..a425ffa 100644 --- a/test/dataframe.test.ts +++ b/test/dataframe.test.ts @@ -4,10 +4,9 @@ import { } from '../src/dataframe.js' export function wrapObject(obj: Row): AsyncRow { - return Object.fromEntries([ - ['__index__', wrapPromise(obj.__index__)], - ...Object.entries(obj).filter(([key]) => key !== '__index__').map(([key, value]) => [key, wrapPromise(value)]), - ]) + return Object.fromEntries( + Object.entries(obj).map(([key, value]) => [key, wrapPromise(value)]) + ) } describe('resolvablePromise', () => { @@ -37,7 +36,7 @@ describe('sortableDataFrame', () => { { id: 1, name: 'Alice', age: 30 }, { id: 2, name: 'Bob', age: 20 }, { id: 4, name: 'Dani', age: 20 }, - ].map((d, i) => ({ ...d, __index__: i })) + ] const dataFrame: DataFrame = { header: ['id', 'name', 'age'], @@ -63,9 +62,9 @@ describe('sortableDataFrame', () => { it('should return unsorted data when orderBy is not provided', async () => { const rows = await awaitRows(sortableDf.rows(0, 3)) expect(rows).toEqual([ - { id: 3, name: 'Charlie', age: 25, __index__: 0 }, - { id: 1, name: 'Alice', age: 30, __index__: 1 }, - { id: 2, name: 'Bob', age: 20, __index__: 2 }, + { id: 3, name: 'Charlie', age: 25 }, + { id: 1, name: 'Alice', age: 30 }, + { id: 2, name: 'Bob', age: 20 }, ]) }) @@ -103,7 +102,7 @@ describe('arrayDataFrame', () => { { id: 1, name: 'Alice', age: 30 }, { id: 2, name: 'Bob', age: 25 }, { id: 3, name: 'Charlie', age: 35 }, - ].map((d, i) => ({ ...d, __index__: i })) + ] it('should create a DataFrame with correct header and numRows', () => { const df = arrayDataFrame(testData) @@ -122,8 +121,8 @@ describe('arrayDataFrame', () => { const df = arrayDataFrame(testData) const rows = await df.rows(0, 2) expect(rows).toEqual([ - { id: 1, name: 'Alice', age: 30, __index__: 0 }, - { id: 2, name: 'Bob', age: 25, __index__: 1 }, + { id: 1, name: 'Alice', age: 30 }, + { id: 2, name: 'Bob', age: 25 }, ]) }) diff --git a/test/rowCache.test.ts b/test/rowCache.test.ts index 51af779..2c5bbee 100644 --- a/test/rowCache.test.ts +++ b/test/rowCache.test.ts @@ -1,18 +1,15 @@ import { describe, expect, it, vi } from 'vitest' -import { AsyncRow, awaitRows } from '../src/dataframe.js' +import { Row, awaitRows } from '../src/dataframe.js' import { rowCache } from '../src/rowCache.js' -import { wrapObject } from './dataframe.test.js' // Mock DataFrame function makeDf() { return { header: ['id'], numRows: 10, - rows: vi.fn((start: number, end: number): AsyncRow[] => { + rows: vi.fn((start: number, end: number): Row[] => { return new Array(end - start).fill(null) - .map((_, index) => wrapObject({ - __index__: start + index, - })) + .map((_, index) => ({ id: start + index })) }), } } @@ -22,7 +19,7 @@ describe('rowCache', () => { const df = makeDf() const dfCached = rowCache(df) const rows = await awaitRows(dfCached.rows(0, 3)) - expect(rows).toEqual([{ __index__: 0 }, { __index__: 1 }, { __index__: 2 }]) + expect(rows).toEqual([{ id: 0 }, { id: 1 }, { id: 2 }]) expect(df.rows).toHaveBeenCalledTimes(1) expect(df.rows).toHaveBeenCalledWith(0, 3, undefined) }) @@ -33,13 +30,13 @@ describe('rowCache', () => { // Initial fetch to cache rows const rowsPre = await awaitRows(dfCached.rows(3, 6)) - expect(rowsPre).toEqual([{ __index__: 3 }, { __index__: 4 }, { __index__: 5 }]) + expect(rowsPre).toEqual([{ id: 3 }, { id: 4 }, { id: 5 }]) expect(df.rows).toHaveBeenCalledTimes(1) expect(df.rows).toHaveBeenCalledWith(3, 6, undefined) // Subsequent fetch should use cache const rowsPost = await awaitRows(dfCached.rows(3, 6)) - expect(rowsPost).toEqual([{ __index__: 3 }, { __index__: 4 }, { __index__: 5 }]) + expect(rowsPost).toEqual([{ id: 3 }, { id: 4 }, { id: 5 }]) expect(df.rows).toHaveBeenCalledTimes(1) }) @@ -57,7 +54,7 @@ describe('rowCache', () => { const adjacentRows = await awaitRows(dfCached.rows(0, 6)) expect(adjacentRows).toEqual([ - { __index__: 0 }, { __index__: 1 }, { __index__: 2 }, { __index__: 3 }, { __index__: 4 }, { __index__: 5 }, + { id: 0 }, { id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }, { id: 5 }, ]) expect(df.rows).toHaveBeenCalledTimes(2) }) @@ -73,7 +70,7 @@ describe('rowCache', () => { // Fetch combined block const gapRows = await awaitRows(dfCached.rows(0, 6)) expect(gapRows).toEqual([ - { __index__: 0 }, { __index__: 1 }, { __index__: 2 }, { __index__: 3 }, { __index__: 4 }, { __index__: 5 }, + { id: 0 }, { id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }, { id: 5 }, ]) expect(df.rows).toHaveBeenCalledTimes(3) expect(df.rows).toHaveBeenCalledWith(2, 4, undefined) @@ -89,7 +86,7 @@ describe('rowCache', () => { // Fetch overlapping block const overlappingRows = await awaitRows(dfCached.rows(8, 11)) expect(overlappingRows).toEqual([ - { __index__: 8 }, { __index__: 9 }, { __index__: 10 }, + { id: 8 }, { id: 9 }, { id: 10 }, ]) expect(df.rows).toHaveBeenCalledTimes(2) expect(df.rows).toHaveBeenCalledWith(9, 11, undefined) From 8f45a174f1e2b5008c2d187fe2e5b41c3ebc52b2 Mon Sep 17 00:00:00 2001 From: Sylvain Lesage Date: Wed, 15 Jan 2025 17:27:08 +0100 Subject: [PATCH 09/15] centralize code that checks if __index__ is present and needed --- src/HighTable.tsx | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/src/HighTable.tsx b/src/HighTable.tsx index 670eada..051b3f6 100644 --- a/src/HighTable.tsx +++ b/src/HighTable.tsx @@ -306,6 +306,25 @@ export default function HighTable({ } }, [focus]) + /** + * Get the row index in original (unsorted) data frame, and in the sorted virtual table. + * + * @param rowIndex row index in the "rows" slice + * + * @returns an object with two properties: + * dataIndex: row index in the original (unsorted) data frame + * tableIndex: row index in the virtual table (sorted) + */ + const getRowIndexes = useCallback((rowIndex: number): { dataIndex?: number, tableIndex: number } => { + const tableIndex = startIndex + rowIndex + const dataIndex = orderBy === undefined + ? tableIndex + : rowIndex >= 0 && rowIndex < rows.length && '__index__' in rows[rowIndex] && typeof rows[rowIndex].__index__ === 'number' + ? rows[rowIndex].__index__ + : undefined + return { dataIndex, tableIndex } + }, [rows, startIndex, orderBy]) + const onRowNumberClick = useCallback(({ useAnchor, tableIndex }: {useAnchor: boolean, tableIndex: number}) => { if (!selectable) return false if (useAnchor) { @@ -355,39 +374,36 @@ export default function HighTable({ setOrderBy={orderBy => data.sortable && dispatch({ type: 'SET_ORDER', orderBy })} /> {prePadding.map((_, prePaddingIndex) => { - const tableIndex = startIndex - prePadding.length + prePaddingIndex + const { tableIndex, dataIndex } = getRowIndexes(-prePadding.length + prePaddingIndex) return })} - {rows.map((row, sliceIndex) => { - const tableIndex = startIndex + sliceIndex - const index = orderBy === undefined ? tableIndex : '__index__' in row && typeof row.__index__ === 'number' ? row.__index__ : undefined - return { + const { tableIndex, dataIndex } = getRowIndexes(rowIndex) + return {data.header.map((col, colIndex) => - Cell(row[col], colIndex, index) + Cell(row[col], colIndex, dataIndex) )} })} {postPadding.map((_, postPaddingIndex) => { - const tableIndex = startIndex + rows.length + postPaddingIndex + const { tableIndex, dataIndex } = getRowIndexes(rows.length + postPaddingIndex) return From ed23ba0535aa9545f4a9c1484a71d06590099baf Mon Sep 17 00:00:00 2001 From: Sylvain Lesage Date: Wed, 15 Jan 2025 17:29:23 +0100 Subject: [PATCH 10/15] update comment --- src/HighTable.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/HighTable.tsx b/src/HighTable.tsx index 051b3f6..ad457cb 100644 --- a/src/HighTable.tsx +++ b/src/HighTable.tsx @@ -49,7 +49,7 @@ type State = { columnWidths: Array // width of each column invalidate: boolean // true if the data must be fetched again hasCompleteRow: boolean // true if at least one row is fully resolved (all of its cells) - rows: Row[] // slice of the virtual table rows (sorted rows) to render as HTML. It might contain incomplete rows, and __index__ might be missing. + rows: Row[] // slice of the virtual table rows (sorted rows) to render as HTML. It might contain incomplete rows. Rows are expected to include __index__ if sorted. startIndex: number // offset of the slice of sorted rows to render (rows[0] is the startIndex'th sorted row) orderBy?: string // column name to sort by selection: Selection // rows selection. The values are indexes of the virtual table (sorted rows), and thus depend on the order. From fd8fd37c2e16e34c8afaf01a4b98a8945ec3401a Mon Sep 17 00:00:00 2001 From: Sylvain Lesage Date: Wed, 15 Jan 2025 17:30:40 +0100 Subject: [PATCH 11/15] update docstring --- src/HighTable.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/HighTable.tsx b/src/HighTable.tsx index ad457cb..72c57de 100644 --- a/src/HighTable.tsx +++ b/src/HighTable.tsx @@ -278,7 +278,7 @@ export default function HighTable({ * * @param value cell value * @param col column index - * @param row row index. Can be undefined. + * @param row row index. If undefined, onDoubleClickCell and onMouseDownCell will not be called. */ function Cell(value: any, col: number, row?: number): ReactNode { // render as truncated text From 9769e68bf79bf6a5971182c6edbc082817edeadc Mon Sep 17 00:00:00 2001 From: Sylvain Lesage Date: Wed, 15 Jan 2025 17:34:36 +0100 Subject: [PATCH 12/15] restore two changes --- src/dataframe.ts | 4 ++-- test/dataframe.test.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dataframe.ts b/src/dataframe.ts index c829a2b..56aa430 100644 --- a/src/dataframe.ts +++ b/src/dataframe.ts @@ -26,8 +26,8 @@ export interface DataFrame { sortable?: boolean } -export function resolvableRow(header: string[]): { [key: string]: ResolvablePromise } & { __index__: ResolvablePromise } { - return Object.fromEntries([...header.map(key => [key, resolvablePromise()]), ['__index__', resolvablePromise()]]) +export function resolvableRow(header: string[]): { [key: string]: ResolvablePromise } { + return Object.fromEntries(header.map(key => [key, resolvablePromise()])) } /** diff --git a/test/dataframe.test.ts b/test/dataframe.test.ts index a425ffa..16408a7 100644 --- a/test/dataframe.test.ts +++ b/test/dataframe.test.ts @@ -3,7 +3,7 @@ import { AsyncRow, DataFrame, Row, arrayDataFrame, awaitRows, resolvablePromise, sortableDataFrame, wrapPromise, } from '../src/dataframe.js' -export function wrapObject(obj: Row): AsyncRow { +function wrapObject(obj: Row): AsyncRow { return Object.fromEntries( Object.entries(obj).map(([key, value]) => [key, wrapPromise(value)]) ) From 4f6c8837a5048c23906b7ff2ef11d2f42e008bbd Mon Sep 17 00:00:00 2001 From: Sylvain Lesage Date: Wed, 15 Jan 2025 17:52:30 +0100 Subject: [PATCH 13/15] Revert "restore two changes" This reverts commit 9769e68bf79bf6a5971182c6edbc082817edeadc. --- src/dataframe.ts | 4 ++-- test/dataframe.test.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dataframe.ts b/src/dataframe.ts index 56aa430..c829a2b 100644 --- a/src/dataframe.ts +++ b/src/dataframe.ts @@ -26,8 +26,8 @@ export interface DataFrame { sortable?: boolean } -export function resolvableRow(header: string[]): { [key: string]: ResolvablePromise } { - return Object.fromEntries(header.map(key => [key, resolvablePromise()])) +export function resolvableRow(header: string[]): { [key: string]: ResolvablePromise } & { __index__: ResolvablePromise } { + return Object.fromEntries([...header.map(key => [key, resolvablePromise()]), ['__index__', resolvablePromise()]]) } /** diff --git a/test/dataframe.test.ts b/test/dataframe.test.ts index 16408a7..a425ffa 100644 --- a/test/dataframe.test.ts +++ b/test/dataframe.test.ts @@ -3,7 +3,7 @@ import { AsyncRow, DataFrame, Row, arrayDataFrame, awaitRows, resolvablePromise, sortableDataFrame, wrapPromise, } from '../src/dataframe.js' -function wrapObject(obj: Row): AsyncRow { +export function wrapObject(obj: Row): AsyncRow { return Object.fromEntries( Object.entries(obj).map(([key, value]) => [key, wrapPromise(value)]) ) From b6ffbdf114df1785a06d71e924f824708df35674 Mon Sep 17 00:00:00 2001 From: Sylvain Lesage Date: Wed, 15 Jan 2025 17:52:55 +0100 Subject: [PATCH 14/15] no need to export --- test/dataframe.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dataframe.test.ts b/test/dataframe.test.ts index a425ffa..16408a7 100644 --- a/test/dataframe.test.ts +++ b/test/dataframe.test.ts @@ -3,7 +3,7 @@ import { AsyncRow, DataFrame, Row, arrayDataFrame, awaitRows, resolvablePromise, sortableDataFrame, wrapPromise, } from '../src/dataframe.js' -export function wrapObject(obj: Row): AsyncRow { +function wrapObject(obj: Row): AsyncRow { return Object.fromEntries( Object.entries(obj).map(([key, value]) => [key, wrapPromise(value)]) ) From 20e30fd72f98a30d4adee4c3175597958684bb98 Mon Sep 17 00:00:00 2001 From: Sylvain Lesage Date: Wed, 15 Jan 2025 18:03:06 +0100 Subject: [PATCH 15/15] only add __index__ if required --- src/dataframe.ts | 10 +++++----- test/dataframe.test.ts | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/dataframe.ts b/src/dataframe.ts index c829a2b..918d963 100644 --- a/src/dataframe.ts +++ b/src/dataframe.ts @@ -26,8 +26,8 @@ export interface DataFrame { sortable?: boolean } -export function resolvableRow(header: string[]): { [key: string]: ResolvablePromise } & { __index__: ResolvablePromise } { - return Object.fromEntries([...header.map(key => [key, resolvablePromise()]), ['__index__', resolvablePromise()]]) +export function resolvableRow(header: string[]): { [key: string]: ResolvablePromise } { + return Object.fromEntries(header.map(key => [key, resolvablePromise()])) } /** @@ -49,7 +49,8 @@ export function asyncRows(rows: AsyncRow[] | Promise, numRows: number, he wrapped[i][key].resolve(row[key]) } // resolve the row index if present - if ('__index__' in row) { + if (!('__index__' in wrapped) && '__index__' in row && typeof row.__index__ === 'number') { + wrapped[i].__index__ = resolvablePromise() wrapped[i].__index__.resolve(row.__index__) } } @@ -120,8 +121,7 @@ export function sortableDataFrame(data: DataFrame): DataFrame { throw new Error(`Invalid orderBy field: ${orderBy}`) } if (!all) { - // Fetch all rows and add __index__ column - // Note that it will erase any existing __index__ column + // Fetch all rows and add __index__ column (if not already present) all = awaitRows(data.rows(0, data.numRows)) .then(rows => rows.map((row, i) => ({ __index__: i, ...row }))) } diff --git a/test/dataframe.test.ts b/test/dataframe.test.ts index 16408a7..a425ffa 100644 --- a/test/dataframe.test.ts +++ b/test/dataframe.test.ts @@ -3,7 +3,7 @@ import { AsyncRow, DataFrame, Row, arrayDataFrame, awaitRows, resolvablePromise, sortableDataFrame, wrapPromise, } from '../src/dataframe.js' -function wrapObject(obj: Row): AsyncRow { +export function wrapObject(obj: Row): AsyncRow { return Object.fromEntries( Object.entries(obj).map(([key, value]) => [key, wrapPromise(value)]) )
+ return
{ /// TODO(SL): if the data is sorted, this sequence of row labels is incorrect and might include duplicate /// labels with respect to the next slice of rows. Better to hide this number if the data is sorted? rowLabel(tableIndex) } - +
onRowNumberClick({ useAnchor: event.shiftKey, tableIndex })}> + return
onRowNumberClick({ useAnchor: event.shiftKey, tableIndex })}> { /// TODO(SL): we might want to show two columns: one for the tableIndex (for selection) and one for the dataIndex (to refer to the original data ids) rowLabel(dataIndex) } - +
+ return
{ /// TODO(SL): if the data is sorted, this sequence of row labels is incorrect and might include duplicate /// labels with respect to the previous slice of rows. Better to hide this number if the data is sorted? rowLabel(tableIndex) } - +
{ const mockData = { @@ -89,3 +90,49 @@ describe('HighTable', () => { expect(container.querySelector('div.pending')).toBeNull() }) }) + + +describe('When sorted, HighTable', () => { + const data = { + header: ['ID', 'Count'], + numRows: 1000, + rows: (start: number, end: number) => Promise.resolve( + Array.from({ length: end - start }, (_, index) => ({ + ID: 'row ' + (index + start), + Count: 1000 - start - index, + })) + ), + } + + function checkRowContents(row: HTMLElement, rowNumber: string, ID: string, Count: string) { + const selectionCell = within(row).getByRole('rowheader') + expect(selectionCell).toBeDefined() + expect(selectionCell.textContent).toBe(rowNumber) + + const columns = within(row).getAllByRole('cell') + expect(columns).toHaveLength(2) + expect(columns[0].textContent).toBe(ID) + expect(columns[1].textContent).toBe(Count) + } + + it('shows the rows in the right order', async () => { + const { findByRole, getByRole, findAllByRole } = render() + + expect(getByRole('columnheader', { name: 'ID' })).toBeDefined() + await findByRole('cell', { name: 'row 1' }) + + const table = getByRole('grid') // not table! because the table is interactive. See https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/grid_role + // first rowgroup is for the thead second is for tbody + const tbody = within(table).getAllByRole('rowgroup')[1] + let rows = within(tbody).getAllByRole('row') + checkRowContents(rows[0], '1', 'row 0', '1,000') + + // Click on the Count header to sort by Count + const countHeader = getByRole('columnheader', { name: 'Count' }) + fireEvent.click(countHeader) + await findAllByRole('cell', { name: 'row 999' }) + + rows = within(within(getByRole('grid')).getAllByRole('rowgroup')[1]).getAllByRole('row') + checkRowContents(rows[0], '1', 'row 999', '1') + }) +}) From 4163b7bceec7803d15839c702e0249df73a27462 Mon Sep 17 00:00:00 2001 From: Sylvain Lesage Date: Tue, 14 Jan 2025 11:35:23 +0100 Subject: [PATCH 02/15] set readonly to the checkbox for now to remove warning in tests --- src/HighTable.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/HighTable.tsx b/src/HighTable.tsx index 83e1276..742a482 100644 --- a/src/HighTable.tsx +++ b/src/HighTable.tsx @@ -397,7 +397,7 @@ export default function HighTable({ /// TODO(SL): we might want to show two columns: one for the tableIndex (for selection) and one for the dataIndex (to refer to the original data ids) rowLabel(dataIndex) } - +
{ /// TODO(SL): if the data is sorted, this sequence of row labels is incorrect and might include duplicate @@ -387,26 +373,23 @@ export default function HighTable({
onRowNumberClick({ useAnchor: event.shiftKey, tableIndex })}> - { - /// TODO(SL): we might want to show two columns: one for the tableIndex (for selection) and one for the dataIndex (to refer to the original data ids) - rowLabel(dataIndex) - } + { row?.__index__ === undefined ? '' : rowLabel(row.__index__) }
{ /// TODO(SL): if the data is sorted, this sequence of row labels is incorrect and might include duplicate diff --git a/src/dataframe.ts b/src/dataframe.ts index 48ae777..8333f65 100644 --- a/src/dataframe.ts +++ b/src/dataframe.ts @@ -7,13 +7,18 @@ type WrappedPromise = Promise & { * A row where each cell is a promise. * The promise must be wrapped with `wrapPromise` so that HighTable can render * the state synchronously. + * + * `__index__` is a promise that resolves to the row index in the "original data". Beware of + * the collisions with the header keys. */ -export type AsyncRow = Record> +export type AsyncRow = Record> & { __index__: WrappedPromise } /** * A row where each cell is a resolved value. + * + * `__index__` is the row index in the "original data". Beware of the collisions with the header keys. */ -export type Row = Record +export type Row = Record & { __index__: number } /** * Streamable row data @@ -26,8 +31,8 @@ export interface DataFrame { sortable?: boolean } -export function resolvableRow(header: string[]): { [key: string]: ResolvablePromise } { - return Object.fromEntries(header.map(key => [key, resolvablePromise()])) +export function resolvableRow(header: string[]): { [key: string]: ResolvablePromise } & { __index__: ResolvablePromise } { + return Object.fromEntries([...header.map(key => [key, resolvablePromise()]), ['__index__', resolvablePromise()]]) } /** @@ -48,6 +53,7 @@ export function asyncRows(rows: AsyncRow[] | Promise, numRows: number, he for (const key of header) { wrapped[i][key].resolve(row[key]) } + wrapped[i].__index__.resolve(row.__index__) } }).catch(error => { // Reject all promises on error @@ -55,6 +61,7 @@ export function asyncRows(rows: AsyncRow[] | Promise, numRows: number, he for (const key of header) { wrapped[i][key].reject(error) } + wrapped[i].__index__.reject(error) } }) return wrapped @@ -104,19 +111,19 @@ export function resolvablePromise(): ResolvablePromise { */ export function sortableDataFrame(data: DataFrame): DataFrame { if (data.sortable) return data // already sortable - // Fetch all rows and add __index__ column + // Fetch all rows and set index let all: Promise return { ...data, rows(start: number, end: number, orderBy?: string): AsyncRow[] | Promise { if (orderBy) { if (!data.header.includes(orderBy)) { + /// TODO(SL): should we allow '__index__'? throw new Error(`Invalid orderBy field: ${orderBy}`) } if (!all) { - // Fetch all rows and add __index__ column + // Fetch all rows all = awaitRows(data.rows(0, data.numRows)) - .then(rows => rows.map((row, i) => ({ __index__: i, ...row }))) } const sorted = all.then(all => { return all.sort((a, b) => { @@ -137,9 +144,13 @@ export function sortableDataFrame(data: DataFrame): DataFrame { /** * Await all promises in an AsyncRow and return resolved row. */ -export function awaitRow(row: AsyncRow): Promise { - return Promise.all(Object.values(row)) - .then(values => Object.fromEntries(Object.keys(row).map((key, i) => [key, values[i]]))) +export async function awaitRow(row: AsyncRow): Promise { + const otherEntries = Object.entries(row).filter( ([key]) => key !== '__index__' ) + const [ __index__, ...values ] = await Promise.all([row.__index__, ...otherEntries.map( ([_, value]) => value )]) + return Object.fromEntries([ + ['__index__', __index__], + ...otherEntries.map(([key, _], i) => [key, values[i]]), + ]) } /** @@ -150,13 +161,15 @@ export function awaitRows(rows: AsyncRow[] | Promise): Promise { return Promise.all(rows.map(awaitRow)) } -export function arrayDataFrame(data: Row[]): DataFrame { +export function arrayDataFrame(data: Record[]): DataFrame { if (!data.length) return { header: [], numRows: 0, rows: () => Promise.resolve([]) } return { - header: Object.keys(data[0]), + header: Object.keys(data[0]).filter(key => key !== '__index__'), numRows: data.length, rows(start: number, end: number): Promise { - return Promise.resolve(data.slice(start, end)) + return Promise.resolve(data.slice(start, end).map( + (row, i) => ({ ...row, __index__: i + start }) + )) }, } } diff --git a/test/HighTable.test.tsx b/test/HighTable.test.tsx index c014061..2bd9399 100644 --- a/test/HighTable.test.tsx +++ b/test/HighTable.test.tsx @@ -10,6 +10,7 @@ describe('HighTable', () => { numRows: 100, rows: vi.fn((start, end) => Promise.resolve( Array.from({ length: end - start }, (_, index) => ({ + __index__: index + start, ID: index + start, Name: 'Name ' + (index + start), Age: 20 + index % 50, @@ -98,6 +99,7 @@ describe('When sorted, HighTable', () => { numRows: 1000, rows: (start: number, end: number) => Promise.resolve( Array.from({ length: end - start }, (_, index) => ({ + __index__: index + start, ID: 'row ' + (index + start), Count: 1000 - start - index, })) @@ -119,7 +121,7 @@ describe('When sorted, HighTable', () => { const { findByRole, getByRole, findAllByRole } = render() expect(getByRole('columnheader', { name: 'ID' })).toBeDefined() - await findByRole('cell', { name: 'row 1' }) + await findByRole('cell', { name: 'row 0' }) const table = getByRole('grid') // not table! because the table is interactive. See https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/grid_role // first rowgroup is for the thead second is for tbody @@ -133,6 +135,27 @@ describe('When sorted, HighTable', () => { await findAllByRole('cell', { name: 'row 999' }) rows = within(within(getByRole('grid')).getAllByRole('rowgroup')[1]).getAllByRole('row') - checkRowContents(rows[0], '1', 'row 999', '1') + checkRowContents(rows[0], '1,000', 'row 999', '1') + }) + + it('provides the double click callback with the right row index', async () => { + const mockDoubleClick = vi.fn() + const { findByRole, getByRole } = render() + const cell0 = await findByRole('cell', { name: 'row 0' }) + + fireEvent.doubleClick(cell0) + + expect(mockDoubleClick).toHaveBeenCalledWith(expect.anything(), 0, 0) + vi.clearAllMocks() + + // Click on the Count header to sort by Count + const countHeader = getByRole('columnheader', { name: 'Count' }) + fireEvent.click(countHeader) + + const cell999 = await findByRole('cell', { name: 'row 999' }) + + fireEvent.doubleClick(cell999) + + expect(mockDoubleClick).toHaveBeenCalledWith(expect.anything(), 0, 999) }) }) diff --git a/test/dataframe.test.ts b/test/dataframe.test.ts index 0999dda..9e490b4 100644 --- a/test/dataframe.test.ts +++ b/test/dataframe.test.ts @@ -1,12 +1,13 @@ import { describe, expect, it } from 'vitest' import { - DataFrame, Row, arrayDataFrame, awaitRows, resolvablePromise, sortableDataFrame, wrapPromise, + AsyncRow, DataFrame, Row, arrayDataFrame, awaitRows, resolvablePromise, sortableDataFrame, wrapPromise, } from '../src/dataframe.js' -function wrapObject(obj: Record): Row { - return Object.fromEntries( - Object.entries(obj).map(([key, value]) => [key, wrapPromise(value)]) - ) +export function wrapObject(obj: Row): AsyncRow { + return Object.fromEntries([ + ['__index__', wrapPromise(obj.__index__)], + ...Object.entries(obj).filter(([key]) => key !== '__index__').map(([key, value]) => [key, wrapPromise(value)]), + ]) } describe('resolvablePromise', () => { @@ -36,12 +37,12 @@ describe('sortableDataFrame', () => { { id: 1, name: 'Alice', age: 30 }, { id: 2, name: 'Bob', age: 20 }, { id: 4, name: 'Dani', age: 20 }, - ] + ].map((d, i) => ({ ...d, __index__: i })) const dataFrame: DataFrame = { header: ['id', 'name', 'age'], numRows: data.length, - rows(start: number, end: number): Row[] { + rows(start: number, end: number): AsyncRow[] { // Return the slice of data between start and end indices return data.slice(start, end).map(wrapObject) }, @@ -62,9 +63,9 @@ describe('sortableDataFrame', () => { it('should return unsorted data when orderBy is not provided', async () => { const rows = await awaitRows(sortableDf.rows(0, 3)) expect(rows).toEqual([ - { id: 3, name: 'Charlie', age: 25 }, - { id: 1, name: 'Alice', age: 30 }, - { id: 2, name: 'Bob', age: 20 }, + { id: 3, name: 'Charlie', age: 25, __index__: 0 }, + { id: 1, name: 'Alice', age: 30, __index__: 1 }, + { id: 2, name: 'Bob', age: 20, __index__: 2 }, ]) }) @@ -102,7 +103,7 @@ describe('arrayDataFrame', () => { { id: 1, name: 'Alice', age: 30 }, { id: 2, name: 'Bob', age: 25 }, { id: 3, name: 'Charlie', age: 35 }, - ] + ].map((d, i) => ({ ...d, __index__: i })) it('should create a DataFrame with correct header and numRows', () => { const df = arrayDataFrame(testData) @@ -121,8 +122,8 @@ describe('arrayDataFrame', () => { const df = arrayDataFrame(testData) const rows = await df.rows(0, 2) expect(rows).toEqual([ - { id: 1, name: 'Alice', age: 30 }, - { id: 2, name: 'Bob', age: 25 }, + { id: 1, name: 'Alice', age: 30, __index__: 0 }, + { id: 2, name: 'Bob', age: 25, __index__: 1 }, ]) }) diff --git a/test/rowCache.test.ts b/test/rowCache.test.ts index 2c5bbee..51af779 100644 --- a/test/rowCache.test.ts +++ b/test/rowCache.test.ts @@ -1,15 +1,18 @@ import { describe, expect, it, vi } from 'vitest' -import { Row, awaitRows } from '../src/dataframe.js' +import { AsyncRow, awaitRows } from '../src/dataframe.js' import { rowCache } from '../src/rowCache.js' +import { wrapObject } from './dataframe.test.js' // Mock DataFrame function makeDf() { return { header: ['id'], numRows: 10, - rows: vi.fn((start: number, end: number): Row[] => { + rows: vi.fn((start: number, end: number): AsyncRow[] => { return new Array(end - start).fill(null) - .map((_, index) => ({ id: start + index })) + .map((_, index) => wrapObject({ + __index__: start + index, + })) }), } } @@ -19,7 +22,7 @@ describe('rowCache', () => { const df = makeDf() const dfCached = rowCache(df) const rows = await awaitRows(dfCached.rows(0, 3)) - expect(rows).toEqual([{ id: 0 }, { id: 1 }, { id: 2 }]) + expect(rows).toEqual([{ __index__: 0 }, { __index__: 1 }, { __index__: 2 }]) expect(df.rows).toHaveBeenCalledTimes(1) expect(df.rows).toHaveBeenCalledWith(0, 3, undefined) }) @@ -30,13 +33,13 @@ describe('rowCache', () => { // Initial fetch to cache rows const rowsPre = await awaitRows(dfCached.rows(3, 6)) - expect(rowsPre).toEqual([{ id: 3 }, { id: 4 }, { id: 5 }]) + expect(rowsPre).toEqual([{ __index__: 3 }, { __index__: 4 }, { __index__: 5 }]) expect(df.rows).toHaveBeenCalledTimes(1) expect(df.rows).toHaveBeenCalledWith(3, 6, undefined) // Subsequent fetch should use cache const rowsPost = await awaitRows(dfCached.rows(3, 6)) - expect(rowsPost).toEqual([{ id: 3 }, { id: 4 }, { id: 5 }]) + expect(rowsPost).toEqual([{ __index__: 3 }, { __index__: 4 }, { __index__: 5 }]) expect(df.rows).toHaveBeenCalledTimes(1) }) @@ -54,7 +57,7 @@ describe('rowCache', () => { const adjacentRows = await awaitRows(dfCached.rows(0, 6)) expect(adjacentRows).toEqual([ - { id: 0 }, { id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }, { id: 5 }, + { __index__: 0 }, { __index__: 1 }, { __index__: 2 }, { __index__: 3 }, { __index__: 4 }, { __index__: 5 }, ]) expect(df.rows).toHaveBeenCalledTimes(2) }) @@ -70,7 +73,7 @@ describe('rowCache', () => { // Fetch combined block const gapRows = await awaitRows(dfCached.rows(0, 6)) expect(gapRows).toEqual([ - { id: 0 }, { id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }, { id: 5 }, + { __index__: 0 }, { __index__: 1 }, { __index__: 2 }, { __index__: 3 }, { __index__: 4 }, { __index__: 5 }, ]) expect(df.rows).toHaveBeenCalledTimes(3) expect(df.rows).toHaveBeenCalledWith(2, 4, undefined) @@ -86,7 +89,7 @@ describe('rowCache', () => { // Fetch overlapping block const overlappingRows = await awaitRows(dfCached.rows(8, 11)) expect(overlappingRows).toEqual([ - { id: 8 }, { id: 9 }, { id: 10 }, + { __index__: 8 }, { __index__: 9 }, { __index__: 10 }, ]) expect(df.rows).toHaveBeenCalledTimes(2) expect(df.rows).toHaveBeenCalledWith(9, 11, undefined) From 9cc1581303188fb9930fa3ab01d689a2845cb42f Mon Sep 17 00:00:00 2001 From: Sylvain Lesage Date: Tue, 14 Jan 2025 23:05:39 +0100 Subject: [PATCH 04/15] remove wrong comments + hide unknown row numbers when sorted --- src/HighTable.tsx | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/HighTable.tsx b/src/HighTable.tsx index 76ad2c4..9a07873 100644 --- a/src/HighTable.tsx +++ b/src/HighTable.tsx @@ -362,19 +362,18 @@ export default function HighTable({
{ - /// TODO(SL): if the data is sorted, this sequence of row labels is incorrect and might include duplicate - /// labels with respect to the next slice of rows. Better to hide this number if the data is sorted? - rowLabel(tableIndex) + // If the data is sorted, we don't know the previous row indexes + orderBy === undefined ? rowLabel(tableIndex) : '' }
{ - /// TODO(SL): if the data is sorted, this sequence of row labels is incorrect and might include duplicate - /// labels with respect to the previous slice of rows. Better to hide this number if the data is sorted? - rowLabel(tableIndex) + // If the data is sorted, we don't know the next row indexes + orderBy === undefined ? rowLabel(tableIndex) : '' }
onDoubleClickCell?.(e, col, row)} - onMouseDown={e => onMouseDownCell?.(e, col, row)} + onDoubleClick={e => row === undefined ? console.warn('Cell onDoubleClick is cancelled because row index is undefined') : onDoubleClickCell?.(e, col, row)} + onMouseDown={e => row === undefined ? console.warn('Cell onMouseDown is cancelled because row index is undefined') : onMouseDownCell?.(e, col, row)} style={memoizedStyles[col]} title={title}> {str} @@ -371,16 +367,17 @@ export default function HighTable({ })} {rows.map((row, sliceIndex) => { const tableIndex = startIndex + sliceIndex - return
onRowNumberClick({ useAnchor: event.shiftKey, tableIndex })}> - { row?.__index__ === undefined ? '' : rowLabel(row.__index__) } + { rowLabel(index) }
{ - // If the data is sorted, we don't know the previous row indexes - orderBy === undefined ? rowLabel(tableIndex) : '' + rowLabel(dataIndex) }
onRowNumberClick({ useAnchor: event.shiftKey, tableIndex })}> - { rowLabel(index) } + { rowLabel(dataIndex) }
{ - // If the data is sorted, we don't know the next row indexes - orderBy === undefined ? rowLabel(tableIndex) : '' + rowLabel(dataIndex) }