Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
dimKeyFromDimValues,
dimKeyFromRow,
} from "@rilldata/web-common/features/dashboards/pivot/pivot-click-selection";
import { parentExpandKey } from "@rilldata/web-common/features/dashboards/pivot/pivot-expand-keys";
import {
type ExtractedFilter,
type PivotRowSelectionState,
Expand Down Expand Up @@ -614,7 +615,7 @@ export function createPivotClickToFilter(
const $clickSelection = get(clickSelectionStore);

// In nested mode, row data stores all values under rowDimensions[0],
// so we must use positional rowId navigation to get correct dim→value pairs.
// so we must resolve the value-based rowId to get correct dim→value pairs.
const isNested = !$config.isFlat;
const dimValues = isNested
? Object.fromEntries(
Expand All @@ -627,7 +628,7 @@ export function createPivotClickToFilter(
// For nested child rows (depth > 0), build dimKey from the fully-resolved
// dimValues (which include parent dimension values); dimKeyFromRow only
// sees rowDimensions[0] and would produce identical keys across parents.
const isNestedChild = isNested && rowId.includes(".");
const isNestedChild = isNested && parentExpandKey(rowId) !== "";
const dk = isNestedChild
? dimKeyFromDimValues(dimValues, $config.rowDimensionNames)
: dimKeyFromRow(rowData, $config.rowDimensionNames);
Expand Down
10 changes: 6 additions & 4 deletions web-common/src/features/dashboards/pivot/FlatTable.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
type PivotClickSelectionState,
dimKeyFromRow,
} from "./pivot-click-selection";
import { PIVOT_TOTALS_ROW_ID } from "./pivot-expand-keys";
import type { PivotRowSelectionState } from "./pivot-row-selection";
import type { CellFormatter } from "./pivot-conditional-formatting";
import PivotHeaderLabel from "./PivotHeaderLabel.svelte";
Expand Down Expand Up @@ -76,9 +77,10 @@

$: headers = headerGroups[0].headers;

// The totals row is always tanstack row "0" (see PivotTable.svelte). When
// pinned to the bottom it is skipped in the virtualized body and rendered
// once more in a sticky <tfoot>, so no row ids or index math change.
// The totals row is always the first tanstack row, with id
// PIVOT_TOTALS_ROW_ID (see PivotTable.svelte). When pinned to the bottom it
// is skipped in the virtualized body and rendered once more in a sticky
// <tfoot>, so no row ids or index math change.
$: totalsRowAtBottom = !!totalsRow && totalsRowPosition === "bottom";

// Initialize column lengths if not already set
Expand Down Expand Up @@ -291,7 +293,7 @@
{@const rowId = rows[rowIndex].id}
{@const rowData = rows[rowIndex].original}
{@const dk = dimKeyFromRow(rowData, config?.rowDimensionNames ?? [])}
{@const isTotalsRow = !!totalsRow && rowId === "0"}
{@const isTotalsRow = !!totalsRow && rowId === PIVOT_TOTALS_ROW_ID}
{@const isSelected = rowSelectionState?.isRowSelected(rowData) ?? false}
{@const hasClickedCell = clickSelection?.hasSelectedCellInRow(dk) ?? false}
{@const effectiveDimIdx = computeEffectiveDimIdx(
Expand Down
10 changes: 6 additions & 4 deletions web-common/src/features/dashboards/pivot/NestedTable.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
getNestedRowDimensionWidthKey,
COLUMN_WIDTH_CONSTANTS as WIDTHS,
} from "./pivot-column-width-utils";
import { PIVOT_TOTALS_ROW_ID } from "./pivot-expand-keys";
import type { PivotRowSelectionState } from "./pivot-row-selection";
import {
computeAncestorRowIds,
Expand Down Expand Up @@ -120,9 +121,10 @@
$: hasExpandableRows = rowDimensions.length > 1;
$: hasMeasures = measures.length > 0;

// The totals row is always tanstack row "0" (see PivotTable.svelte). When
// pinned to the bottom it is skipped in the virtualized body and rendered
// once more in a sticky <tfoot>, so no row ids or index math change.
// The totals row is always the first tanstack row, with id
// PIVOT_TOTALS_ROW_ID (see PivotTable.svelte). When pinned to the bottom it
// is skipped in the virtualized body and rendered once more in a sticky
// <tfoot>, so no row ids or index math change.
$: totalsRowAtBottom = !!totalsRow && totalsRowPosition === "bottom";
$: rowDimensionNames = rowDimensions.map((d) => d.name);
$: rowDimensionLabel = getRowNestedLabel(rowDimensions);
Expand Down Expand Up @@ -631,7 +633,7 @@
rows[rowIndex].depth > 0
? nestedDimKeyFromRow(rows[rowIndex], rowDimensionNames)
: dimKeyFromRow(rowData, rowDimensionNames)}
{@const isTotalsRow = !!totalsRow && rowId === "0"}
{@const isTotalsRow = !!totalsRow && rowId === PIVOT_TOTALS_ROW_ID}
{@const filterSelected =
rowSelectionState?.isRowSelected(
rowData,
Expand Down
45 changes: 33 additions & 12 deletions web-common/src/features/dashboards/pivot/PivotTable.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@
isShowMoreRow,
splitPivotChips,
} from "@rilldata/web-common/features/dashboards/pivot/pivot-utils";
import {
buildExpandKey,
childExpandKey,
encodeExpandKeyValue,
parentExpandKey,
PIVOT_TOTALS_ROW_ID,
} from "@rilldata/web-common/features/dashboards/pivot/pivot-expand-keys";
import { copyToClipboard } from "@rilldata/web-common/lib/actions/copy-to-clipboard";
import {
createVirtualizer,
Expand Down Expand Up @@ -89,15 +96,30 @@
export let clickSelection: PivotClickSelectionState | undefined = undefined;

const options: Readable<TableOptions<PivotDataRow>> = derived(
[pivotDataStore, pivotState],
([pivotData, state]) => {
[pivotDataStore, pivotState, config],
([pivotData, state, cfg]) => {
let tableData = [...pivotData.data];
if (pivotData.totalsRowData) {
tableData = [pivotData.totalsRowData, ...pivotData.data];
}
const totalsRowData = pivotData.totalsRowData;
const rowDims = cfg.rowDimensionNames;
return {
data: tableData,
columns: pivotData.columnDef,
// Value-based, hierarchical row ids (parent id + this row's value) so
// expansion survives sorting, adding a field, and data refreshes.
getRowId: (row, _index, parent) => {
if (totalsRowData && row === totalsRowData)
return PIVOT_TOTALS_ROW_ID;
if (cfg.isFlat) {
return buildExpandKey(rowDims.map((dim) => row[dim]));
}
const anchor = rowDims[0];
return parent
? childExpandKey(parent.id, row[anchor])
: encodeExpandKeyValue(row[anchor]);
},
state: {
expanded: state.expanded,
sorting: state.sorting,
Expand Down Expand Up @@ -240,7 +262,7 @@
if (needsDomains) {
for (const row of flatRows) {
// Always skip the prepended grand-totals row.
if (hasTotalsRow && row.id === "0") continue;
if (hasTotalsRow && row.id === PIVOT_TOTALS_ROW_ID) continue;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Three sibling totals-row checks still compare against the positional id "0", which getRowId no longer returns: PivotTable.svelte:391, FlatTable.svelte:245 and NestedTable.svelte:570. In PivotTable.svelte a canvas click on a totals-row measure cell is therefore no longer short-circuited and falls through to onCellClickToFilter; getValuesForExpandedKey resolves no row values for "\0totals", so getFiltersForCell applies a filter built from the column entries alone. In the two table components isTotalsRow is false for the totals row, so getCellFormatting stops returning null and totals cells pick up conditional-formatting colours, and flatCellState/nestedCellState mark them interactiveCell when click-to-filter is enabled.

const target = row.subRows.length > 0 ? parentValues : leafValues;
for (const cell of row.getAllCells()) {
const meta = cell.column.columnDef.meta;
Expand Down Expand Up @@ -364,21 +386,20 @@

if (!nextLimit) return;

// Check if this is the outermost dimension or a nested dimension
// Outermost dimension has rowId like "0", "1", etc. (no dots)
// Nested dimensions have rowId like "0.1", "0.1.2", etc.
const isOutermostDimension = !rowId.includes(".");
// The outermost "Show more" row sits at the top level (its parent key
// is the root ""); a nested one has a real parent node whose child
// limit we raise.
const parentKey = parentExpandKey(rowId);

if (isOutermostDimension) {
if (parentKey === "") {
// Handle outermost dimension "Show more" click
if (setPivotOutermostRowLimit) {
setPivotOutermostRowLimit(nextLimit);
}
} else {
// Handle nested dimension "Show more" click
const expandIndex = rowId.split(".").slice(0, -1).join(".");
if (expandIndex && setPivotRowLimitForExpanded) {
setPivotRowLimitForExpanded(expandIndex, nextLimit);
if (setPivotRowLimitForExpanded) {
setPivotRowLimitForExpanded(parentKey, nextLimit);
}
}
return;
Expand All @@ -391,7 +412,7 @@
if (row.getCanExpand()) row.getToggleExpandedHandler()();
} else {
// Skip totals row for filtering
const isTotalsRow = totalsRow && rowId === "0";
const isTotalsRow = totalsRow && rowId === PIVOT_TOTALS_ROW_ID;
if (isTotalsRow && onCellClickToFilter) {
return;
}
Expand Down
53 changes: 53 additions & 0 deletions web-common/src/features/dashboards/pivot/pivot-expand-keys.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { describe, expect, it } from "vitest";
import {
buildExpandKey,
childExpandKey,
expandKeyDepth,
expandKeySegments,
parentExpandKey,
} from "./pivot-expand-keys";

describe("pivot-expand-keys", () => {
it("round-trips a value path through build/segments", () => {
const values = ["Galatea-Stories", "facebook", "Jul 2026"];
const key = buildExpandKey(values);
expect(expandKeySegments(key)).toEqual(values);
expect(expandKeyDepth(key)).toBe(3);
});

it("keeps values that contain dots or spaces intact", () => {
const values = ["a.b.c", "x y z"];
const key = buildExpandKey(values);
expect(expandKeySegments(key)).toEqual(values);
expect(expandKeyDepth(key)).toBe(2);
});

it("encodes null distinctly from a genuinely absent level", () => {
const withNull = buildExpandKey(["app", null]);
const shallow = buildExpandKey(["app"]);
expect(withNull).not.toBe(shallow);
expect(expandKeyDepth(withNull)).toBe(2);
expect(expandKeyDepth(shallow)).toBe(1);
});

it("computes the parent by stripping the deepest segment", () => {
const key = buildExpandKey(["app", "src", "month"]);
const parent = parentExpandKey(key);
expect(expandKeySegments(parent)).toEqual(["app", "src"]);
expect(parentExpandKey(buildExpandKey(["app"]))).toBe("");
expect(parentExpandKey("")).toBe("");
});

it("childExpandKey extends a parent and is inverse of parentExpandKey", () => {
const parent = buildExpandKey(["app", "src"]);
const child = childExpandKey(parent, "month");
expect(expandKeySegments(child)).toEqual(["app", "src", "month"]);
expect(parentExpandKey(child)).toBe(parent);
expect(childExpandKey("", "app")).toBe(buildExpandKey(["app"]));
});

it("treats the empty root key as depth 0", () => {
expect(expandKeyDepth("")).toBe(0);
expect(expandKeySegments("")).toEqual([]);
});
});
44 changes: 44 additions & 0 deletions web-common/src/features/dashboards/pivot/pivot-expand-keys.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/**
* Value-based keys for pivot row expansion. A row is keyed by the dimension
* values from the root to it, NUL-joined and hierarchical, so the key stays
* stable across sorting, adding a field, and data refreshes. Uses the same
* separator and null sentinel as the dimKeys in pivot-click-selection.ts,
* except that undefined also maps to the sentinel here.
*/

// NUL separator, since dimension values won't contain it.
export const EXPAND_KEY_SEP = "\0";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Consumers of the old dot-path row ids were not migrated to this separator. web-common/src/features/canvas/components/pivot/pivot-click-to-filter.ts:658 still computes isNestedChild = isNested && rowId.includes("."), so every child row whose value has no dot now takes the dimKeyFromRow branch, and that branch only reads rowDimensionNames[0]. With rows [outer, inner], clicking the revenue cell of X under A stores dimKey "X" while NestedTable.svelte computes the same row's key via nestedDimKeyFromRow as "A\0X": the clicked cell never renders as selected, a second click is not recognised as a deselect, and X collides across parents, which is exactly the collision the comment above that line says the branch exists to prevent. parentExpandKey(rowId) !== "" is the equivalent test. pivot-click-to-filter.spec.ts also fails 23 of 47 tests on this head (all pass on main) because its fixtures still pass positional ids such as "1.0".


// Sentinel for null values, so a null at depth N differs from an absent level.
// A literal "<NULL>" string value encodes the same way, which only matters if
// both appear under the same parent; the dimKeys make the same trade-off.
const NULL_SENTINEL = "<NULL>";

// The leading separator can't occur in a real id, so this can't collide.
export const PIVOT_TOTALS_ROW_ID = EXPAND_KEY_SEP + "totals";

export function encodeExpandKeyValue(value: unknown): string {
return value === null || value === undefined ? NULL_SENTINEL : String(value);
}

export function buildExpandKey(values: unknown[]): string {
return values.map(encodeExpandKeyValue).join(EXPAND_KEY_SEP);
}

export function expandKeySegments(key: string): string[] {
return key === "" ? [] : key.split(EXPAND_KEY_SEP);
}

export function expandKeyDepth(key: string): number {
return expandKeySegments(key).length;
}

export function parentExpandKey(key: string): string {
const i = key.lastIndexOf(EXPAND_KEY_SEP);
return i === -1 ? "" : key.slice(0, i);
}

export function childExpandKey(parentKey: string, value: unknown): string {
const segment = encodeExpandKeyValue(value);
return parentKey === "" ? segment : parentKey + EXPAND_KEY_SEP + segment;
}
Loading