Skip to content

Commit

Permalink
fix: Make the preview table column width reactive (#4864)
Browse files Browse the repository at this point in the history
* Make the preview table column width reactive

* Add test
  • Loading branch information
AdityaHegde authored May 9, 2024
1 parent 38796b4 commit 02515c3
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 17 deletions.
27 changes: 10 additions & 17 deletions web-common/src/components/virtualized-table/VirtualTable.svelte
Original file line number Diff line number Diff line change
@@ -1,19 +1,7 @@
<script lang="ts" context="module">
const columnSizes = (() => {
const sizes = new Map<string, number[]>();
return {
get: (key: string, calculator: () => number[]): number[] => {
let array = sizes.get(key);
if (!array) {
array = calculator();
sizes.set(key, array);
}
return array;
},
set: (key: string, value: number[]) => sizes.set(key, value),
};
})();
import { VirtualizedTableColumnSizes } from "@rilldata/web-common/components/virtualized-table/columnSizes";
const columnSizes = new VirtualizedTableColumnSizes();
export const ROW_HEIGHT = 24;
export const MIN_COL_WIDTH = 108;
Expand Down Expand Up @@ -107,7 +95,7 @@
let scrollLeft = 0;
let nextPinnedColumnPosition = ROW_HEADER_WIDTH;
$: columnWidths = columnSizes.get(name, () =>
$: columnWidths = columnSizes.get(name, columns, columnAccessor, () =>
initColumnWidths({
columns,
rows,
Expand Down Expand Up @@ -173,6 +161,11 @@
resizing.initialPixelWidth + delta,
maxColWidth,
);
columnSizes.set(
name,
columns[resizing.columnIndex].name as string,
columnWidths[resizing.columnIndex],
);
});
}
Expand All @@ -198,7 +191,7 @@
const value =
description ?? isHeader
? column
: (rows[index][column] as string | number | null);
: (rows[index]?.[column] as string | number | null);
const type = columns.find((c) => c.name === column)?.type ?? "string";
hovering = {
Expand Down
50 changes: 50 additions & 0 deletions web-common/src/components/virtualized-table/columnSizes.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { VirtualizedTableColumnSizes } from "@rilldata/web-common/components/virtualized-table/columnSizes";
import { describe, it, expect } from "vitest";

describe("VirtualizedTableColumnSizes", () => {
it("Sanity check", () => {
const sizes = new VirtualizedTableColumnSizes();
expect(
sizes.get(
"model",
[{ name: "col1" }, { name: "col2" }, { name: "col3" }],
"name",
() => [50, 50, 50],
),
).toEqual([50, 50, 50]);

// set the width for col2 to something specific
sizes.set("model", "col2", 100);
// the width is saved
expect(
sizes.get(
"model",
[{ name: "col1" }, { name: "col2" }, { name: "col3" }],
"name",
() => [50, 50, 50],
),
).toEqual([50, 100, 50]);

// the width is saved when one of the columns is removed
expect(
sizes.get("model", [{ name: "col1" }, { name: "col2" }], "name", () => [
50, 50,
]),
).toEqual([50, 100]);

expect(
sizes.get("model", [{ name: "col1" }, { name: "col4" }], "name", () => [
50, 50,
]),
).toEqual([50, 50]);
// removing a column will remove any saved sizes. this is for cleaning up leaks
expect(
sizes.get(
"model",
[{ name: "col1" }, { name: "col2" }, { name: "col4" }],
"name",
() => [50, 50, 50],
),
).toEqual([50, 50, 50]);
});
});
56 changes: 56 additions & 0 deletions web-common/src/components/virtualized-table/columnSizes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import type { VirtualizedTableColumns } from "@rilldata/web-common/components/virtualized-table/types";
import { V1MetricsViewColumn } from "@rilldata/web-common/runtime-client";

export class VirtualizedTableColumnSizes {
private readonly sizesCache = new Map<string, Map<string, number>>();

public get(
key: string,
columns: (VirtualizedTableColumns | V1MetricsViewColumn)[],
columnAccessor: keyof VirtualizedTableColumns,
calculator: () => number[],
): number[] {
const cache = this.sizesCache.get(key);
if (!cache) {
const sizes = calculator();
this.sizesCache.set(
key,
new Map<string, number>(
sizes.map((s, i) => [columns[i][columnAccessor], s]),
),
);
return sizes;
}

let missingSize = false;
const sizes = columns.map((column) => {
if (!cache.has(column[columnAccessor])) {
missingSize = true;
return 0;
}
return cache.get(column[columnAccessor]) as number;
});
if (!missingSize) return sizes;

const newSizes = calculator();
// retain sizes from cache
newSizes.forEach((_, i) => {
if (cache.has(columns[i][columnAccessor])) {
newSizes[i] = cache.get(columns[i][columnAccessor]) as number;
}
});
// reset the cache
this.sizesCache.set(
key,
new Map<string, number>(
newSizes.map((s, i) => [columns[i][columnAccessor], s]),
),
);

return newSizes;
}

public set(name: string, colName: string, value: number) {
this.sizesCache.get(name)?.set(colName, value);
}
}

1 comment on commit 02515c3

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.