From db9c6ca8c83330b02fd18a6b5f04d9a0d909eae3 Mon Sep 17 00:00:00 2001 From: Paul Newling Date: Fri, 29 May 2026 15:04:48 -0700 Subject: [PATCH 01/13] MILAB-6002: fix PlDataTable column visibility reset on re-run Persist column visibility as the user's explicit show/hide deviations from the block-defined default (pl7.app/table/visibility) instead of the absolute hidden-column set. Once a saved absolute set existed it overrode the block default, so a column whose default flipped between runs (e.g. a filter/ranking column reverting to optional) was not re-hidden and leaked visible. makeColDef and createPlDataTableV3.computeHiddenColumns now reconcile the current default with user deviations; persisted state migrates v7 -> v8 (one-time reset of custom column show/hide). --- .../milab-6002-table-visibility-deviations.md | 18 ++++ .../createPlDataTable/createPlDataTableV3.ts | 32 ++++-- .../components/PlDataTable/state-migration.ts | 37 ++++++- .../src/components/PlDataTable/typesV7.ts | 30 +++++- .../PlAgDataTable/PlAgDataTableV2.vue | 97 ++++++++----------- .../PlAgDataTable/sources/table-source-v2.ts | 31 +++--- .../PlAgDataTable/sources/table-state-v2.ts | 7 ++ 7 files changed, 169 insertions(+), 83 deletions(-) create mode 100644 .changeset/milab-6002-table-visibility-deviations.md diff --git a/.changeset/milab-6002-table-visibility-deviations.md b/.changeset/milab-6002-table-visibility-deviations.md new file mode 100644 index 0000000000..04ad1c0a1a --- /dev/null +++ b/.changeset/milab-6002-table-visibility-deviations.md @@ -0,0 +1,18 @@ +--- +'@platforma-sdk/model': patch +'@platforma-sdk/ui-vue': patch +--- + +PlDataTable: persist column visibility as user deviations from block defaults + +Fixes column visibility resetting when a block re-runs with changed filter/ranking +configuration (MILAB-6002). The saved grid state previously stored the absolute +hidden-column set, which overrode the block-defined default visibility once it +existed — so a column whose default flipped between runs (e.g. a filter/ranking +column reverting to optional after the filter was removed) was not re-hidden. + +Column visibility is now stored as the user's explicit show/hide deviations from +the block's `pl7.app/table/visibility` default. The current default always applies +to untouched columns, and the block's full/visible table handles reconcile the same +way. Persisted state is migrated v7 -> v8 (one-time reset of custom column +show/hide; defaults apply correctly afterwards). diff --git a/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.ts b/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.ts index c74bc71adb..ab26acc53f 100644 --- a/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.ts +++ b/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.ts @@ -186,11 +186,13 @@ export function createPlDataTableV3( ]); const hiddenSpecs = state.pTableParams.hiddenColIds; + const shownSpecs = state.pTableParams.shownColIds; const hiddenColumnIds = computeHiddenColumns( [...annotated.direct, ...annotated.linked].map((v) => v.column), sorting, filters, hiddenSpecs, + shownSpecs, ); const visible = buildVisibleColumns(annotated, hiddenColumnIds); @@ -452,15 +454,33 @@ function computeHiddenColumns( sorting: Nil | PTableSorting[], filters: Nil | PlDataTableFilters, hiddenSpecs: Nil | PTableColumnId[], + shownSpecs: Nil | PTableColumnId[], ): Set { - const alwaysHidden = columns.filter((c) => isColumnHidden(c.spec)).map((c) => c.id); - const optionalHidden = !isNil(hiddenSpecs) - ? hiddenSpecs.filter((s): s is PTableColumnIdColumn => s.type === "column").map((s) => s.id) - : columns.filter((c) => isColumnOptional(c.spec)).map((c) => c.id); - const initial = [...alwaysHidden, ...optionalHidden]; + const userHidden = new Set( + (hiddenSpecs ?? []) + .filter((s): s is PTableColumnIdColumn => s.type === "column") + .map((s) => s.id), + ); + const userShown = new Set( + (shownSpecs ?? []) + .filter((s): s is PTableColumnIdColumn => s.type === "column") + .map((s) => s.id), + ); + // Block default visibility (isColumnHidden / isColumnOptional) reconciled with + // the user's explicit show/hide overrides. Defaults always apply to untouched + // columns, so a column whose default flips between runs (e.g. filter/ranking + // config changed) follows its current default rather than a stale saved state. + const hidden = columns + .filter((c) => { + if (isColumnHidden(c.spec)) return true; + if (userShown.has(c.id)) return false; + if (userHidden.has(c.id)) return true; + return isColumnOptional(c.spec); + }) + .map((c) => c.id); const preserved = collectPreservedColumnIds(sorting, filters); - return new Set(initial.filter((id) => !preserved.has(id))); + return new Set(hidden.filter((id) => !preserved.has(id))); } /** Collect IDs of columns that must remain visible (sorted, filtered). */ diff --git a/sdk/model/src/components/PlDataTable/state-migration.ts b/sdk/model/src/components/PlDataTable/state-migration.ts index 578bef7276..d11bea3db7 100644 --- a/sdk/model/src/components/PlDataTable/state-migration.ts +++ b/sdk/model/src/components/PlDataTable/state-migration.ts @@ -20,6 +20,7 @@ import type { PlDataTableSheetState, PlDataTableStateV2CacheEntry, PlDataTableStateV2Normalized, + PlDataTableStateV2V7, PlTableColumnIdJson, PTableParamsV2, } from "./typesV7"; @@ -144,6 +145,9 @@ export type PlDataTableStateV2 = // v6 stored colIds as canonicalized full `PTableColumnSpec` (including // annotations + `pl7.app/trace`). v7 strips down to `PTableColumnId`. | PlDataTableStateV2V6 + // v7 stored columnVisibility as the absolute hidden set. v8 stores user + // deviations from the block-defined default visibility. + | PlDataTableStateV2V7 // Normalized state | PlDataTableStateV2Normalized; @@ -190,6 +194,13 @@ export function upgradePlDataTableStateV2( if (state.version === 6) { state = migrateV6toV7(state); } + // v7 -> v8: column visibility switched from an absolute hidden set to user + // deviations from the block default. Old absolute state can't be converted + // without the per-column defaults, so reset columnVisibility (one-time loss of + // custom column show/hide); defaults apply correctly from then on. + if (state.version === 7) { + state = migrateV7toV8(state); + } return state; } @@ -242,7 +253,7 @@ function unwrapV5GridState(gridState: PlDataTableGridStateV5): PlDataTableGridSt /** Migrate v6 to v7: rewrite each colId from a full PTableColumnSpec to its * compact PTableColumnId (drops annotations/spec body, ~16× smaller per column). */ -function migrateV6toV7(state: PlDataTableStateV2V6): PlDataTableStateV2Normalized { +function migrateV6toV7(state: PlDataTableStateV2V6): PlDataTableStateV2V7 { return { version: 7, stateCache: state.stateCache.map( @@ -255,6 +266,28 @@ function migrateV6toV7(state: PlDataTableStateV2V6): PlDataTableStateV2Normalize }; } +/** Migrate v7 to v8: column visibility moved from an absolute hidden set to + * user deviations from the block-defined default visibility. The old absolute + * set cannot be converted to deviations without the per-column defaults (which + * are not available here), so reset columnVisibility and the derived + * hidden/shown pTableParams. This is a one-time reset of custom column show/hide + * choices; the block-defined defaults apply correctly afterwards. */ +function migrateV7toV8(state: PlDataTableStateV2V7): PlDataTableStateV2Normalized { + return { + version: 8, + stateCache: state.stateCache.map( + (entry): PlDataTableStateV2CacheEntry => ({ + ...entry, + gridState: { ...entry.gridState, columnVisibility: undefined }, + }), + ), + pTableParams: + state.pTableParams.sourceId === null + ? state.pTableParams + : { ...state.pTableParams, hiddenColIds: null, shownColIds: null }, + }; +} + /** Convert a v6 fat colId (canonicalized PTableColumnSpec) to a v7 compact colId * (canonicalized PTableColumnId). The row-number sentinel is a string literal, * not a spec — pass it through unchanged. */ @@ -416,7 +449,7 @@ export function createDefaultPTableParams(): PTableParamsV2 { export function createPlDataTableStateV2(): PlDataTableStateV2Normalized { return { - version: 7, + version: 8, stateCache: [], pTableParams: createDefaultPTableParams(), }; diff --git a/sdk/model/src/components/PlDataTable/typesV7.ts b/sdk/model/src/components/PlDataTable/typesV7.ts index 6e13e66717..bd4408b837 100644 --- a/sdk/model/src/components/PlDataTable/typesV7.ts +++ b/sdk/model/src/components/PlDataTable/typesV7.ts @@ -31,10 +31,18 @@ export type PlDataTableGridStateCore = { sort: "asc" | "desc"; }[]; }; - /** Includes column visibility */ + /** Includes column visibility. + * + * Stores the user's explicit overrides relative to the block-defined default + * visibility (the `pl7.app/table/visibility` annotation), NOT the absolute + * hidden set. This keeps saved state stable when the block changes a column's + * default visibility between runs (e.g. filter/ranking config flips a column + * default<->optional): unmentioned columns always follow their current default. */ columnVisibility?: { - /** All colIds which were hidden */ + /** Columns the user explicitly hid that the block default would have shown. */ hiddenColIds: PlTableColumnIdJson[]; + /** Columns the user explicitly showed that the block default would have hidden. */ + shownColIds?: PlTableColumnIdJson[]; }; }; @@ -87,21 +95,37 @@ export type PTableParamsV2 = | { sourceId: null; hiddenColIds: null; + shownColIds?: null; sorting: []; filters: null; defaultFilters: null; } | { sourceId: string; + /** User's explicit hide overrides (vs block default visibility). */ hiddenColIds: null | PTableColumnId[]; + /** User's explicit show overrides (vs block default visibility). */ + shownColIds?: null | PTableColumnId[]; sorting: PTableSorting[]; filters: null | PlDataTableFilters; defaultFilters: null | PlDataTableFilters; }; +/** Historical v7 normalized state (pre-deviation column visibility). + * + * Shape is identical to the current normalized state, but `columnVisibility` + * stored the ABSOLUTE hidden set rather than the user's deviations from the + * block-defined default visibility. The v7->v8 migration resets it (see + * state-migration.ts) so the deviation model starts clean. */ +export type PlDataTableStateV2V7 = { + version: 7; + stateCache: PlDataTableStateV2CacheEntry[]; + pTableParams: PTableParamsV2; +}; + export type PlDataTableStateV2Normalized = { /** Version for upgrades */ - version: 7; + version: 8; /** Internal states, LRU cache for 5 sourceId-s */ stateCache: PlDataTableStateV2CacheEntry[]; /** PTable params derived from the cache state for the current sourceId */ diff --git a/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue b/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue index ed5d8ecdc3..24d54bdb97 100644 --- a/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue +++ b/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue @@ -129,9 +129,8 @@ const { gridApi, gridOptions } = useGrid({ let isReloading = false; gridOptions.value.onGridPreDestroyed = (event) => { if (!isReloading) { - gridOptions.value.initialState = gridState.value = normalizeColumnVisibility( - makePartialState(event.api.getState()), - gridState.value, + gridOptions.value.initialState = gridState.value = makePartialState( + event.api.getState(), event.api, ); } @@ -141,11 +140,7 @@ gridOptions.value.onRowDoubleClicked = (event) => { if (event.data && event.data.axesKey) emit("rowDoubleClicked", event.data.axesKey); }; gridOptions.value.onStateUpdated = (event) => { - const partialState = normalizeColumnVisibility( - makePartialState(event.state), - gridState.value, - event.api, - ); + const partialState = makePartialState(event.state, event.api); // We have to keep initialState synchronized with gridState for gridState recovery after key updating. gridOptions.value.initialState = gridState.value = partialState; @@ -187,8 +182,17 @@ const sheetsSettings = computed(() => { }); gridOptions.value.initialState = gridState.value; -// Restore proper types erased by AgGrid -function makePartialState(state: GridState): PlDataTableGridStateCore { +// Restore proper types erased by AgGrid. +// columnVisibility is stored as the user's *deviations* from the block-defined +// default visibility, computed from the live grid columns (see +// computeVisibilityDeviations) rather than copied from AG Grid's absolute +// hidden-list. This keeps saved state stable across re-runs when the block +// changes a column's default visibility (e.g. filter/ranking config changed). +function makePartialState( + state: GridState, + api: GridApi, +): PlDataTableGridStateCore { + const deviations = computeVisibilityDeviations(api); return { columnOrder: state.columnOrder as | { @@ -203,57 +207,41 @@ function makePartialState(state: GridState): PlDataTableGridStateCore { }[]; } | undefined, - columnVisibility: state.columnVisibility as - | { - hiddenColIds: PlTableColumnIdJson[]; - } - | undefined, + columnVisibility: + deviations.hiddenColIds.length === 0 && deviations.shownColIds.length === 0 + ? undefined + : deviations, }; } -// AG Grid returns columnVisibility: undefined when all columns are visible. -// We need to distinguish "no state yet" (use isColumnOptional defaults) from -// "user explicitly showed all columns" (store []). This function normalizes -// the undefined from AG Grid based on the previous state. -function normalizeColumnVisibility( - partialState: PlDataTableGridStateCore, - prevState: PlDataTableGridStateCore, - api: GridApi, -): PlDataTableGridStateCore { - if (partialState.columnVisibility !== undefined) return partialState; - - if (prevState.columnVisibility !== undefined) { - // Had explicit visibility state before → user made all columns visible → store []. - return { ...partialState, columnVisibility: { hiddenColIds: [] } }; - } - - // No previous explicit state → compute defaults from current columns - // to replicate: hide: hiddenColIds?.includes(colId) ?? isColumnOptional(spec.spec) - const defaultHidden = getDefaultHiddenColIds(api); - if (defaultHidden.length > 0) { - return { ...partialState, columnVisibility: { hiddenColIds: defaultHidden } }; +// Derive the user's explicit visibility overrides by comparing each column's +// current visibility against its block-defined default (isColumnOptional on the +// spec carried in ColDef.context). Only genuine deviations are recorded, so the +// default always applies to untouched columns — including ones whose default +// flips between runs. Axes and the row-number column (no column spec) are skipped. +function computeVisibilityDeviations(api: GridApi): { + hiddenColIds: PlTableColumnIdJson[]; + shownColIds: PlTableColumnIdJson[]; +} { + const hiddenColIds: PlTableColumnIdJson[] = []; + const shownColIds: PlTableColumnIdJson[] = []; + for (const col of api.getAllGridColumns() ?? []) { + const spec = col.getColDef().context as PTableColumnSpec | undefined; + if (spec === undefined || spec.type !== "column") continue; + const colId = col.getColId() as PlTableColumnIdJson; + const isHidden = !col.isVisible(); + const isOptional = isColumnOptional(spec.spec); + if (isHidden && !isOptional) hiddenColIds.push(colId); + else if (!isHidden && isOptional) shownColIds.push(colId); } - - return partialState; -} - -function getDefaultHiddenColIds(api: GridApi): PlTableColumnIdJson[] { - const cols = api.getAllGridColumns(); - if (!cols) return []; - - return cols - .filter((col) => { - const spec = col.getColDef().context as PTableColumnSpec | undefined; - return spec !== undefined && spec.type === "column" && isColumnOptional(spec.spec); - }) - .map((col) => col.getColId() as PlTableColumnIdJson); + return { hiddenColIds, shownColIds }; } -// Normalize columnVisibility for comparison: undefined and { hiddenColIds: [] } are equivalent. +// Normalize columnVisibility for comparison: no deviations is equivalent to undefined. function stateForReloadCompare(state: PlDataTableGridStateCore): PlDataTableGridStateCore { const cv = state.columnVisibility; - const normalizedCv = !cv || cv.hiddenColIds.length === 0 ? undefined : state.columnVisibility; - return { ...state, columnVisibility: normalizedCv }; + const isEmpty = !cv || (cv.hiddenColIds.length === 0 && (cv.shownColIds?.length ?? 0) === 0); + return { ...state, columnVisibility: isEmpty ? undefined : cv }; } // Reload AgGrid when new state arrives from server @@ -262,7 +250,7 @@ watch( () => [gridApi.value, gridState.value] as const, ([gridApi, gridState]) => { if (!gridApi || gridApi.isDestroyed()) return; - const selfState = makePartialState(gridApi.getState()); + const selfState = makePartialState(gridApi.getState(), gridApi); if ( !isJsonEqual(gridState, {}) && !isJsonEqual(stateForReloadCompare(gridState), stateForReloadCompare(selfState)) @@ -429,6 +417,7 @@ watch( sheets: settings.sheets ?? [], dataRenderedTracker, hiddenColIds: gridState.value.columnVisibility?.hiddenColIds, + shownColIds: gridState.value.columnVisibility?.shownColIds, cellButtonAxisParams: { showCellButtonForAxisId: props.showCellButtonForAxisId, cellButtonInvokeRowsOnDoubleClick: props.cellButtonInvokeRowsOnDoubleClick, diff --git a/sdk/ui-vue/src/components/PlAgDataTable/sources/table-source-v2.ts b/sdk/ui-vue/src/components/PlAgDataTable/sources/table-source-v2.ts index 4994ebb3c9..d7e64111ab 100644 --- a/sdk/ui-vue/src/components/PlAgDataTable/sources/table-source-v2.ts +++ b/sdk/ui-vue/src/components/PlAgDataTable/sources/table-source-v2.ts @@ -90,6 +90,7 @@ export async function calculateGridOptions({ visibleTableHandle, dataRenderedTracker, hiddenColIds, + shownColIds, cellButtonAxisParams, }: { sheets: PlDataTableSheet[]; @@ -98,7 +99,10 @@ export async function calculateGridOptions({ fullTableHandle: PTableHandle; visibleTableHandle: PTableHandle; dataRenderedTracker: DeferredCircular>; + /** User's explicit hide overrides (vs block default visibility). */ hiddenColIds?: PlTableColumnIdJson[]; + /** User's explicit show overrides (vs block default visibility). */ + shownColIds?: PlTableColumnIdJson[]; cellButtonAxisParams?: PlAgCellButtonAxisParams; }): Promise< Pick, "columnDefs" | "serverSideDatasource"> & { @@ -129,13 +133,10 @@ export async function calculateGridOptions({ tableSpecs, ); - // default hidden columns derived from Optional annotation when no saved state - const resolvedHiddenColIds = hiddenColIds ?? computeDefaultHiddenColIds(fields, tableSpecs); - const columnDefs: ColDef[] = [ makeRowNumberColDef(), ...fields.map((field) => - makeColDef(field, tableSpecs[field], resolvedHiddenColIds, cellButtonAxisParams), + makeColDef(field, tableSpecs[field], hiddenColIds, shownColIds, cellButtonAxisParams), ), ]; @@ -233,6 +234,7 @@ export function makeColDef( iCol: number, spec: PTableColumnSpec, hiddenColIds: PlTableColumnIdJson[] | undefined, + shownColIds: PlTableColumnIdJson[] | undefined, cellButtonAxisParams?: PlAgCellButtonAxisParams, ): ColDef { const colId = canonicalizeJson(getPTableColumnId(spec)); @@ -258,7 +260,13 @@ export function makeColDef( headerName, lockPosition: spec.type === "axis" || (isLabelColumnSpec(spec.spec) && spec.spec.axesSpec.length === 1), - hide: hiddenColIds !== undefined && hiddenColIds.includes(colId), + // Visibility = block default (isColumnOptional) reconciled with the user's + // explicit overrides. The block default always applies, so a column whose + // default flips between runs (e.g. filter/ranking config changed) is shown + // or hidden per its CURRENT default unless the user explicitly toggled it. + hide: shownColIds?.includes(colId) + ? false + : (hiddenColIds?.includes(colId) ?? false) || isColumnOptional(spec.spec), valueFormatter: columnRenderingSpec.valueFormatter, headerComponent: PlAgColumnHeader, cellRendererSelector: cellButtonAxisParams?.showCellButtonForAxisId @@ -402,19 +410,6 @@ function sortIndicesByTypeAndPriority(indices: number[], tableSpecs: PTableColum return [...indices].sort((a, b) => priorityOf(b) - priorityOf(a)); } -/** Default hidden col ids built from columns marked with the Optional annotation. */ -function computeDefaultHiddenColIds( - fields: number[], - tableSpecs: PTableColumnSpec[], -): PlTableColumnIdJson[] { - return fields.reduce((acc, field) => { - const spec = tableSpecs[field]; - return spec.type === "column" && isColumnOptional(spec.spec) - ? [...acc, canonicalizeJson(getPTableColumnId(spec))] - : acc; - }, []); -} - /** Extract axis indices and specs from the visible table (always present as part of join). */ function collectVisibleAxes( visibleTableSpecs: PTableColumnSpec[], diff --git a/sdk/ui-vue/src/components/PlAgDataTable/sources/table-state-v2.ts b/sdk/ui-vue/src/components/PlAgDataTable/sources/table-state-v2.ts index 1eeb49d201..46cb20fcd0 100644 --- a/sdk/ui-vue/src/components/PlAgDataTable/sources/table-state-v2.ts +++ b/sdk/ui-vue/src/components/PlAgDataTable/sources/table-state-v2.ts @@ -237,6 +237,7 @@ function createPTableParams( return { sourceId: state.sourceId, hiddenColIds: getHiddenColIds(state.gridState.columnVisibility), + shownColIds: getShownColIds(state.gridState.columnVisibility), sorting: convertAgSortingToPTableSorting(state.gridState.sort), filters, defaultFilters, @@ -430,6 +431,12 @@ function getHiddenColIds( return state?.hiddenColIds?.map((json) => parseJson(json)) ?? null; } +function getShownColIds( + state: PlDataTableGridStateCore["columnVisibility"], +): PTableColumnId[] | null { + return state?.shownColIds?.map((json) => parseJson(json)) ?? null; +} + function makeDefaultState(): PlDataTableStateV2CacheEntryNullable { return { sourceId: null, From 96f7580f2e6b8918a01c785f15e56649c07cdfe5 Mon Sep 17 00:00:00 2001 From: Paul Newling Date: Fri, 29 May 2026 19:34:56 -0700 Subject: [PATCH 02/13] MILAB-6002: address review feedback and polish docs - makeColDef: guard isColumnOptional to column specs, restoring prior axis behaviour and matching computeVisibilityDeviations (greptile). - stateForReloadCompare: optional-chain hiddenColIds.length for a defensive read of persisted state (gemini). - Document that visibility deviations are column-scoped by design: axes have no table/visibility default, are always shown when displayed, and their hide/show is not persisted (greptile). - Tighten the changeset and the columnVisibility doc comment. --- .../milab-6002-table-visibility-deviations.md | 21 ++++++++++--------- .../src/components/PlDataTable/typesV7.ts | 12 +++++------ .../PlAgDataTable/PlAgDataTableV2.vue | 10 +++++++-- .../PlAgDataTable/sources/table-source-v2.ts | 3 ++- 4 files changed, 26 insertions(+), 20 deletions(-) diff --git a/.changeset/milab-6002-table-visibility-deviations.md b/.changeset/milab-6002-table-visibility-deviations.md index 04ad1c0a1a..22ec580671 100644 --- a/.changeset/milab-6002-table-visibility-deviations.md +++ b/.changeset/milab-6002-table-visibility-deviations.md @@ -5,14 +5,15 @@ PlDataTable: persist column visibility as user deviations from block defaults -Fixes column visibility resetting when a block re-runs with changed filter/ranking -configuration (MILAB-6002). The saved grid state previously stored the absolute -hidden-column set, which overrode the block-defined default visibility once it -existed — so a column whose default flipped between runs (e.g. a filter/ranking -column reverting to optional after the filter was removed) was not re-hidden. +Column visibility reset when a block re-ran with a changed filter/ranking +configuration (MILAB-6002). The saved grid state stored the absolute +hidden-column set, which once present overrode the block's default visibility. A +column whose default flipped between runs — e.g. a filter/ranking column +reverting to `optional` when its filter is removed — stayed visible instead of +reverting to hidden. -Column visibility is now stored as the user's explicit show/hide deviations from -the block's `pl7.app/table/visibility` default. The current default always applies -to untouched columns, and the block's full/visible table handles reconcile the same -way. Persisted state is migrated v7 -> v8 (one-time reset of custom column -show/hide; defaults apply correctly afterwards). +Column visibility is now the user's explicit show/hide deviations from the +block's `pl7.app/table/visibility` default, so the current default always applies +to untouched columns; the full and visible table handles reconcile the same way. +Persisted state migrates v7 -> v8: a one-time reset of custom column show/hide, +after which defaults apply correctly. diff --git a/sdk/model/src/components/PlDataTable/typesV7.ts b/sdk/model/src/components/PlDataTable/typesV7.ts index bd4408b837..bd3a260d2b 100644 --- a/sdk/model/src/components/PlDataTable/typesV7.ts +++ b/sdk/model/src/components/PlDataTable/typesV7.ts @@ -31,13 +31,11 @@ export type PlDataTableGridStateCore = { sort: "asc" | "desc"; }[]; }; - /** Includes column visibility. - * - * Stores the user's explicit overrides relative to the block-defined default - * visibility (the `pl7.app/table/visibility` annotation), NOT the absolute - * hidden set. This keeps saved state stable when the block changes a column's - * default visibility between runs (e.g. filter/ranking config flips a column - * default<->optional): unmentioned columns always follow their current default. */ + /** User overrides to column visibility, relative to the block-defined default + * (the `pl7.app/table/visibility` annotation) rather than the absolute hidden + * set. Keeps saved state stable when the block changes a column's default + * between runs (e.g. a filter/ranking column flips default<->optional): + * unmentioned columns follow their current default. */ columnVisibility?: { /** Columns the user explicitly hid that the block default would have shown. */ hiddenColIds: PlTableColumnIdJson[]; diff --git a/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue b/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue index 24d54bdb97..496b9f547f 100644 --- a/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue +++ b/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue @@ -218,7 +218,12 @@ function makePartialState( // current visibility against its block-defined default (isColumnOptional on the // spec carried in ColDef.context). Only genuine deviations are recorded, so the // default always applies to untouched columns — including ones whose default -// flips between runs. Axes and the row-number column (no column spec) are skipped. +// flips between runs. +// +// Scope is column specs only: the block defines per-column defaults via +// pl7.app/table/visibility, while axes have no such default and are always shown +// when displayed. Axes and the row-number column (no column spec) are therefore +// skipped — a manual hide/show of an axis is intentionally not persisted. function computeVisibilityDeviations(api: GridApi): { hiddenColIds: PlTableColumnIdJson[]; shownColIds: PlTableColumnIdJson[]; @@ -240,7 +245,8 @@ function computeVisibilityDeviations(api: GridApi): { // Normalize columnVisibility for comparison: no deviations is equivalent to undefined. function stateForReloadCompare(state: PlDataTableGridStateCore): PlDataTableGridStateCore { const cv = state.columnVisibility; - const isEmpty = !cv || (cv.hiddenColIds.length === 0 && (cv.shownColIds?.length ?? 0) === 0); + const isEmpty = + !cv || ((cv.hiddenColIds?.length ?? 0) === 0 && (cv.shownColIds?.length ?? 0) === 0); return { ...state, columnVisibility: isEmpty ? undefined : cv }; } diff --git a/sdk/ui-vue/src/components/PlAgDataTable/sources/table-source-v2.ts b/sdk/ui-vue/src/components/PlAgDataTable/sources/table-source-v2.ts index d7e64111ab..d2972494bb 100644 --- a/sdk/ui-vue/src/components/PlAgDataTable/sources/table-source-v2.ts +++ b/sdk/ui-vue/src/components/PlAgDataTable/sources/table-source-v2.ts @@ -266,7 +266,8 @@ export function makeColDef( // or hidden per its CURRENT default unless the user explicitly toggled it. hide: shownColIds?.includes(colId) ? false - : (hiddenColIds?.includes(colId) ?? false) || isColumnOptional(spec.spec), + : (hiddenColIds?.includes(colId) ?? false) || + (spec.type === "column" && isColumnOptional(spec.spec)), valueFormatter: columnRenderingSpec.valueFormatter, headerComponent: PlAgColumnHeader, cellRendererSelector: cellButtonAxisParams?.showCellButtonForAxisId From c2e034ee8bd3b95ee15b59e502e866384a344632 Mon Sep 17 00:00:00 2001 From: Paul Newling Date: Fri, 29 May 2026 20:00:04 -0700 Subject: [PATCH 03/13] MILAB-6002: share column-visibility reconciliation via resolveColumnHidden; add tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract the default-vs-override precedence into a single resolveColumnHidden helper used by both the model (computeHiddenColumns) and the UI (makeColDef), so the two cannot drift — the duplication that let this bug reproduce in two places. Move the deviation derivation out of the Vue component into a pure, testable column-visibility module. Tests cover resolveColumnHidden (precedence), computeHiddenColumns (reconciliation plus preserved sorted/filtered columns), computeVisibilityDeviations (inverse derivation), and the v7->v8 migration reset, each with a MILAB-6002 regression case. --- .../createPlDataTableV3.test.ts | 71 +++++++++++++++++ .../createPlDataTable/createPlDataTableV3.ts | 28 ++++--- .../createPlDataTable/utils.test.ts | 44 ++++++++++- .../PlDataTable/createPlDataTable/utils.ts | 28 +++++++ sdk/model/src/components/PlDataTable/index.ts | 1 + .../PlDataTable/state-migration.test.ts | 77 +++++++++++++++++++ .../PlAgDataTable/PlAgDataTableV2.vue | 39 ++++------ .../sources/column-visibility.test.ts | 56 ++++++++++++++ .../sources/column-visibility.ts | 35 +++++++++ .../PlAgDataTable/sources/table-source-v2.ts | 19 +++-- 10 files changed, 355 insertions(+), 43 deletions(-) create mode 100644 sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.test.ts create mode 100644 sdk/model/src/components/PlDataTable/state-migration.test.ts create mode 100644 sdk/ui-vue/src/components/PlAgDataTable/sources/column-visibility.test.ts create mode 100644 sdk/ui-vue/src/components/PlAgDataTable/sources/column-visibility.ts diff --git a/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.test.ts b/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.test.ts new file mode 100644 index 0000000000..a533870ab4 --- /dev/null +++ b/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.test.ts @@ -0,0 +1,71 @@ +import { + Annotation, + type PColumnSpec, + type PObjectId, + type PTableColumnId, + type PTableSorting, +} from "@milaboratories/pl-model-common"; +import { describe, expect, test } from "vitest"; +import { computeHiddenColumns } from "./createPlDataTableV3"; + +function col(id: string, visibility?: "optional" | "hidden"): { id: PObjectId; spec: PColumnSpec } { + return { + id: id as PObjectId, + spec: { + kind: "PColumn", + name: id, + valueType: "Int", + axesSpec: [], + annotations: visibility ? { [Annotation.Table.Visibility]: visibility } : {}, + } as PColumnSpec, + }; +} + +const colRef = (id: string): PTableColumnId => + ({ type: "column", id: id as PObjectId }) as PTableColumnId; + +const hiddenIds = (s: Set): string[] => [...s].sort(); + +describe("computeHiddenColumns", () => { + test("with no saved overrides, hides forced-hidden and optional columns and shows the rest", () => { + const cols = [col("visible"), col("opt", "optional"), col("hid", "hidden")]; + expect(hiddenIds(computeHiddenColumns(cols, null, null, null, null))).toEqual(["hid", "opt"]); + }); + + test("a user-hidden column (block default visible) becomes hidden", () => { + const cols = [col("a"), col("b")]; + expect(hiddenIds(computeHiddenColumns(cols, null, null, [colRef("a")], null))).toEqual(["a"]); + }); + + test("a user-shown column (block default optional) becomes visible", () => { + const cols = [col("a", "optional"), col("b", "optional")]; + expect(hiddenIds(computeHiddenColumns(cols, null, null, null, [colRef("a")]))).toEqual(["b"]); + }); + + test("show overrides win over hide overrides", () => { + const cols = [col("a")]; + expect(hiddenIds(computeHiddenColumns(cols, null, null, [colRef("a")], [colRef("a")]))).toEqual( + [], + ); + }); + + test("forced-hidden columns stay hidden even when the user showed them", () => { + const cols = [col("a", "hidden")]; + expect(hiddenIds(computeHiddenColumns(cols, null, null, null, [colRef("a")]))).toEqual(["a"]); + }); + + // MILAB-6002 regression: an untouched column follows its CURRENT default across + // re-runs, rather than being pinned by a stale saved hidden set. + test("a column whose default flips to optional is re-hidden when untouched", () => { + expect(hiddenIds(computeHiddenColumns([col("flip")], null, null, null, null))).toEqual([]); + expect( + hiddenIds(computeHiddenColumns([col("flip", "optional")], null, null, null, null)), + ).toEqual(["flip"]); + }); + + test("sorted columns are force-kept visible even when optional", () => { + const cols = [col("a", "optional")]; + const sorting = [{ column: colRef("a") }] as unknown as PTableSorting[]; + expect(hiddenIds(computeHiddenColumns(cols, sorting, null, null, null))).toEqual([]); + }); +}); diff --git a/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.ts b/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.ts index ab26acc53f..04cb451c33 100644 --- a/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.ts +++ b/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.ts @@ -28,6 +28,7 @@ import { evaluateRules, isColumnHidden, isColumnOptional, + resolveColumnHidden, withHidenAxesAnnotations, withLabelAnnotations, withTableVisualAnnotations, @@ -448,8 +449,11 @@ function buildSecondaryGroups( ]; } -/** Determine which columns should be hidden based on state or optional-column defaults. */ -function computeHiddenColumns( +/** Determine which columns should be hidden, reconciling block defaults with the + * user's explicit show/hide overrides. Sorted/filtered columns are force-kept visible. + * + * Exported for unit testing. */ +export function computeHiddenColumns( columns: { readonly id: PObjectId; readonly spec: PColumnSpec }[], sorting: Nil | PTableSorting[], filters: Nil | PlDataTableFilters, @@ -466,17 +470,17 @@ function computeHiddenColumns( .filter((s): s is PTableColumnIdColumn => s.type === "column") .map((s) => s.id), ); - // Block default visibility (isColumnHidden / isColumnOptional) reconciled with - // the user's explicit show/hide overrides. Defaults always apply to untouched - // columns, so a column whose default flips between runs (e.g. filter/ranking - // config changed) follows its current default rather than a stale saved state. + // Reconcile each column's block default with the user's explicit overrides via the + // shared resolveColumnHidden, so the model and UI (makeColDef) can never diverge. const hidden = columns - .filter((c) => { - if (isColumnHidden(c.spec)) return true; - if (userShown.has(c.id)) return false; - if (userHidden.has(c.id)) return true; - return isColumnOptional(c.spec); - }) + .filter((c) => + resolveColumnHidden({ + forcedHidden: isColumnHidden(c.spec), + optional: isColumnOptional(c.spec), + userShown: userShown.has(c.id), + userHidden: userHidden.has(c.id), + }), + ) .map((c) => c.id); const preserved = collectPreservedColumnIds(sorting, filters); diff --git a/sdk/model/src/components/PlDataTable/createPlDataTable/utils.test.ts b/sdk/model/src/components/PlDataTable/createPlDataTable/utils.test.ts index 5f366885da..c36b3744a3 100644 --- a/sdk/model/src/components/PlDataTable/createPlDataTable/utils.test.ts +++ b/sdk/model/src/components/PlDataTable/createPlDataTable/utils.test.ts @@ -1,7 +1,13 @@ import { Annotation, type PColumnSpec, type PObjectId } from "@milaboratories/pl-model-common"; import { SpecDriver } from "@milaboratories/pf-spec-driver"; import { describe, expect, test } from "vitest"; -import { deriveAllLabels, evaluateRules, type LabelableColumn, type RuleColumn } from "./utils"; +import { + deriveAllLabels, + evaluateRules, + resolveColumnHidden, + type LabelableColumn, + type RuleColumn, +} from "./utils"; import type { ColumnOrderRule, ColumnVisibilityRule } from "./createPlDataTableV3"; // --------------------------------------------------------------------------- @@ -225,3 +231,39 @@ describe("evaluateRules", () => { expect(result.get("d" as PObjectId)?.visibility).toBe("hidden"); }); }); + +// --------------------------------------------------------------------------- +// resolveColumnHidden — shared precedence for model (computeHiddenColumns) and +// UI (makeColDef). The single place the reconciliation rule lives. +// --------------------------------------------------------------------------- + +describe("resolveColumnHidden", () => { + const base = { forcedHidden: false, optional: false, userShown: false, userHidden: false }; + + test("forced-hidden columns stay hidden, even when the user showed them", () => { + expect(resolveColumnHidden({ ...base, forcedHidden: true })).toBe(true); + expect(resolveColumnHidden({ ...base, forcedHidden: true, userShown: true })).toBe(true); + }); + + test("an explicit user show wins over a hide override and over the optional default", () => { + expect(resolveColumnHidden({ ...base, userShown: true })).toBe(false); + expect(resolveColumnHidden({ ...base, userShown: true, userHidden: true })).toBe(false); + expect(resolveColumnHidden({ ...base, userShown: true, optional: true })).toBe(false); + }); + + test("an explicit user hide hides a column the default would have shown", () => { + expect(resolveColumnHidden({ ...base, userHidden: true })).toBe(true); + }); + + test("untouched columns follow their current optional default", () => { + expect(resolveColumnHidden({ ...base, optional: true })).toBe(true); + expect(resolveColumnHidden({ ...base, optional: false })).toBe(false); + }); + + // MILAB-6002 regression: a column whose default flips between runs and that the + // user never touched must follow the CURRENT default, not stale saved state. + test("a flipped default with no user override follows the new default", () => { + expect(resolveColumnHidden({ ...base, optional: false })).toBe(false); // run 1: shown + expect(resolveColumnHidden({ ...base, optional: true })).toBe(true); // run 2: re-hidden + }); +}); diff --git a/sdk/model/src/components/PlDataTable/createPlDataTable/utils.ts b/sdk/model/src/components/PlDataTable/createPlDataTable/utils.ts index 9e5ba590e8..1f78a9502b 100644 --- a/sdk/model/src/components/PlDataTable/createPlDataTable/utils.ts +++ b/sdk/model/src/components/PlDataTable/createPlDataTable/utils.ts @@ -48,6 +48,34 @@ export function getEffectiveVisibility( return undefined; } +/** + * Reconcile a column's block-defined default visibility with the user's explicit + * show/hide overrides into a single "is this column hidden" decision. + * + * This is the one place the precedence lives. Both the model (visible table handle, + * see `computeHiddenColumns`) and the UI (grid `colDef`, see `makeColDef`) call it, + * so the two can never drift — divergence between them is what made MILAB-6002 + * reproduce in two places at once. Precedence: + * 1. forced-hidden (`pl7.app/table/visibility` = "hidden") — never shown; + * 2. an explicit user override — `shown` wins over `hidden`; + * 3. otherwise the column's CURRENT `optional` default. + * + * The default is applied last on purpose: a column whose default flips between runs + * (e.g. a filter/ranking column going default<->optional) follows its current + * default rather than a stale saved hidden set — the root cause of MILAB-6002. + */ +export function resolveColumnHidden(overrides: { + forcedHidden: boolean; + optional: boolean; + userShown: boolean; + userHidden: boolean; +}): boolean { + if (overrides.forcedHidden) return true; + if (overrides.userShown) return false; + if (overrides.userHidden) return true; + return overrides.optional; +} + /** Get ordering priority for a column. Rule map lookup first, then annotation fallback. */ export function getOrderPriority( col: RuleColumn, diff --git a/sdk/model/src/components/PlDataTable/index.ts b/sdk/model/src/components/PlDataTable/index.ts index 09e803e04e..7cded0f3f9 100644 --- a/sdk/model/src/components/PlDataTable/index.ts +++ b/sdk/model/src/components/PlDataTable/index.ts @@ -31,6 +31,7 @@ export { isColumnOptional, getOrderPriority, getEffectiveVisibility, + resolveColumnHidden, } from "./createPlDataTable/utils"; export type { diff --git a/sdk/model/src/components/PlDataTable/state-migration.test.ts b/sdk/model/src/components/PlDataTable/state-migration.test.ts new file mode 100644 index 0000000000..debe5d0b93 --- /dev/null +++ b/sdk/model/src/components/PlDataTable/state-migration.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, test } from "vitest"; +import type { PObjectId, PTableColumnId } from "@milaboratories/pl-model-common"; +import { createPlDataTableStateV2, upgradePlDataTableStateV2 } from "./state-migration"; +import type { + PlDataTableStateV2CacheEntry, + PlDataTableStateV2V7, + PlTableColumnIdJson, +} from "./typesV7"; + +const jsonId = (s: string): PlTableColumnIdJson => s as PlTableColumnIdJson; +const colRef = (s: string): PTableColumnId => + ({ type: "column", id: s as PObjectId }) as PTableColumnId; + +function v7CacheEntry( + sourceId: string, + hiddenColIds: PlTableColumnIdJson[], +): PlDataTableStateV2CacheEntry { + return { + sourceId, + // v7 stored the ABSOLUTE hidden set here. + gridState: { columnVisibility: { hiddenColIds } }, + sheetsState: [], + filtersState: null, + defaultFiltersState: null, + }; +} + +describe("upgradePlDataTableStateV2 — v7 to v8 column visibility", () => { + test("fresh and empty state is created at the latest version (8)", () => { + expect(createPlDataTableStateV2().version).toBe(8); + expect(upgradePlDataTableStateV2(undefined).version).toBe(8); + }); + + // MILAB-6002: the v7 absolute hidden set can't be converted to deviations without + // per-column defaults, so it is reset once; block defaults then reapply cleanly. + test("resets the v7 absolute hidden set (grid state and derived params)", () => { + const v7: PlDataTableStateV2V7 = { + version: 7, + stateCache: [v7CacheEntry("src1", [jsonId("a"), jsonId("b")])], + pTableParams: { + sourceId: "src1", + hiddenColIds: [colRef("a"), colRef("b")], + sorting: [], + filters: null, + defaultFilters: null, + }, + }; + + const out = upgradePlDataTableStateV2(v7); + + expect(out.version).toBe(8); + expect(out.stateCache[0].gridState.columnVisibility).toBeUndefined(); + expect(out.pTableParams.hiddenColIds).toBeNull(); + expect(out.pTableParams.shownColIds).toBeNull(); + // non-visibility cache fields are preserved through the reset + expect(out.stateCache[0].sourceId).toBe("src1"); + }); + + test("null-sourceId params survive the v7->v8 reset unchanged", () => { + const v7: PlDataTableStateV2V7 = { + version: 7, + stateCache: [], + pTableParams: { + sourceId: null, + hiddenColIds: null, + sorting: [], + filters: null, + defaultFilters: null, + }, + }; + + const out = upgradePlDataTableStateV2(v7); + + expect(out.version).toBe(8); + expect(out.pTableParams.sourceId).toBeNull(); + }); +}); diff --git a/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue b/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue index 496b9f547f..087d009306 100644 --- a/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue +++ b/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue @@ -31,6 +31,10 @@ import { DeferredCircular, ensureNodeVisible } from "./sources/focus-row"; import { PlAgDataTableRowNumberColId } from "./sources/row-number"; import type { PlAgCellButtonAxisParams } from "./sources/table-source-v2"; import { calculateGridOptions } from "./sources/table-source-v2"; +import { + computeVisibilityDeviations, + type ColumnVisibilityState, +} from "./sources/column-visibility"; import { useTableState } from "./sources/table-state-v2"; import type { PlAgDataTableV2Controller, @@ -192,7 +196,7 @@ function makePartialState( state: GridState, api: GridApi, ): PlDataTableGridStateCore { - const deviations = computeVisibilityDeviations(api); + const deviations = computeVisibilityDeviations(readGridColumnVisibility(api)); return { columnOrder: state.columnOrder as | { @@ -214,32 +218,23 @@ function makePartialState( }; } -// Derive the user's explicit visibility overrides by comparing each column's -// current visibility against its block-defined default (isColumnOptional on the -// spec carried in ColDef.context). Only genuine deviations are recorded, so the -// default always applies to untouched columns — including ones whose default -// flips between runs. -// +// Map the live AG Grid columns to the minimal shape computeVisibilityDeviations needs. // Scope is column specs only: the block defines per-column defaults via -// pl7.app/table/visibility, while axes have no such default and are always shown -// when displayed. Axes and the row-number column (no column spec) are therefore -// skipped — a manual hide/show of an axis is intentionally not persisted. -function computeVisibilityDeviations(api: GridApi): { - hiddenColIds: PlTableColumnIdJson[]; - shownColIds: PlTableColumnIdJson[]; -} { - const hiddenColIds: PlTableColumnIdJson[] = []; - const shownColIds: PlTableColumnIdJson[] = []; +// pl7.app/table/visibility, while axes have no such default and are always shown when +// displayed. Axes and the row-number column (no column spec) are therefore skipped — +// a manual hide/show of an axis is intentionally not persisted. +function readGridColumnVisibility(api: GridApi): ColumnVisibilityState[] { + const states: ColumnVisibilityState[] = []; for (const col of api.getAllGridColumns() ?? []) { const spec = col.getColDef().context as PTableColumnSpec | undefined; if (spec === undefined || spec.type !== "column") continue; - const colId = col.getColId() as PlTableColumnIdJson; - const isHidden = !col.isVisible(); - const isOptional = isColumnOptional(spec.spec); - if (isHidden && !isOptional) hiddenColIds.push(colId); - else if (!isHidden && isOptional) shownColIds.push(colId); + states.push({ + colId: col.getColId() as PlTableColumnIdJson, + hidden: !col.isVisible(), + optional: isColumnOptional(spec.spec), + }); } - return { hiddenColIds, shownColIds }; + return states; } // Normalize columnVisibility for comparison: no deviations is equivalent to undefined. diff --git a/sdk/ui-vue/src/components/PlAgDataTable/sources/column-visibility.test.ts b/sdk/ui-vue/src/components/PlAgDataTable/sources/column-visibility.test.ts new file mode 100644 index 0000000000..a36aa301d1 --- /dev/null +++ b/sdk/ui-vue/src/components/PlAgDataTable/sources/column-visibility.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, test } from "vitest"; +import type { PlTableColumnIdJson } from "@platforma-sdk/model"; +import { computeVisibilityDeviations, type ColumnVisibilityState } from "./column-visibility"; + +const id = (s: string): PlTableColumnIdJson => s as PlTableColumnIdJson; + +describe("computeVisibilityDeviations", () => { + test("records no deviations when every column matches its block default", () => { + const cols: ColumnVisibilityState[] = [ + { colId: id("a"), hidden: false, optional: false }, // default-visible and shown + { colId: id("b"), hidden: true, optional: true }, // default-optional and hidden + ]; + expect(computeVisibilityDeviations(cols)).toEqual({ hiddenColIds: [], shownColIds: [] }); + }); + + test("records a hide deviation when the user hides a default-visible column", () => { + const cols: ColumnVisibilityState[] = [{ colId: id("a"), hidden: true, optional: false }]; + expect(computeVisibilityDeviations(cols)).toEqual({ + hiddenColIds: [id("a")], + shownColIds: [], + }); + }); + + test("records a show deviation when the user shows a default-optional column", () => { + const cols: ColumnVisibilityState[] = [{ colId: id("a"), hidden: false, optional: true }]; + expect(computeVisibilityDeviations(cols)).toEqual({ + hiddenColIds: [], + shownColIds: [id("a")], + }); + }); + + test("separates hide and show deviations across a mixed column set", () => { + const cols: ColumnVisibilityState[] = [ + { colId: id("keep"), hidden: false, optional: false }, + { colId: id("hideMe"), hidden: true, optional: false }, + { colId: id("showMe"), hidden: false, optional: true }, + { colId: id("stayHidden"), hidden: true, optional: true }, + ]; + expect(computeVisibilityDeviations(cols)).toEqual({ + hiddenColIds: [id("hideMe")], + shownColIds: [id("showMe")], + }); + }); + + // MILAB-6002: when the user has made no overrides, deviations are empty regardless + // of the defaults — this is what lets persisted state stay stable across re-runs. + test("an all-default column set yields empty deviations (stable across re-runs)", () => { + const cols: ColumnVisibilityState[] = [ + { colId: id("x"), hidden: true, optional: true }, + { colId: id("y"), hidden: false, optional: false }, + ]; + const deviations = computeVisibilityDeviations(cols); + expect(deviations.hiddenColIds).toHaveLength(0); + expect(deviations.shownColIds).toHaveLength(0); + }); +}); diff --git a/sdk/ui-vue/src/components/PlAgDataTable/sources/column-visibility.ts b/sdk/ui-vue/src/components/PlAgDataTable/sources/column-visibility.ts new file mode 100644 index 0000000000..693c82c30f --- /dev/null +++ b/sdk/ui-vue/src/components/PlAgDataTable/sources/column-visibility.ts @@ -0,0 +1,35 @@ +import type { PlTableColumnIdJson } from "@platforma-sdk/model"; + +/** A grid column reduced to what visibility-deviation derivation needs. */ +export type ColumnVisibilityState = { + colId: PlTableColumnIdJson; + /** Current visibility in the grid. */ + hidden: boolean; + /** Block-defined default visibility: `pl7.app/table/visibility` = "optional". */ + optional: boolean; +}; + +/** + * Derive the user's explicit visibility overrides (deviations from the block default) + * from the live grid columns. This is the inverse of the hide decision in + * `makeColDef` / `resolveColumnHidden`: only genuine deviations are recorded, so an + * untouched column always follows its CURRENT default — including when that default + * flips between runs (the MILAB-6002 fix). Persisting deviations rather than the + * absolute hidden set is what keeps saved state stable across re-runs. + * + * Callers pass real columns only. Axes and the row-number column have no + * `pl7.app/table/visibility` default, so their manual show/hide is intentionally + * not persisted and they must be excluded by the caller. + */ +export function computeVisibilityDeviations(columns: ColumnVisibilityState[]): { + hiddenColIds: PlTableColumnIdJson[]; + shownColIds: PlTableColumnIdJson[]; +} { + const hiddenColIds: PlTableColumnIdJson[] = []; + const shownColIds: PlTableColumnIdJson[] = []; + for (const c of columns) { + if (c.hidden && !c.optional) hiddenColIds.push(c.colId); + else if (!c.hidden && c.optional) shownColIds.push(c.colId); + } + return { hiddenColIds, shownColIds }; +} diff --git a/sdk/ui-vue/src/components/PlAgDataTable/sources/table-source-v2.ts b/sdk/ui-vue/src/components/PlAgDataTable/sources/table-source-v2.ts index d2972494bb..168d8c4930 100644 --- a/sdk/ui-vue/src/components/PlAgDataTable/sources/table-source-v2.ts +++ b/sdk/ui-vue/src/components/PlAgDataTable/sources/table-source-v2.ts @@ -21,6 +21,7 @@ import { isLinkerColumn as isLinkerColumnSpec, isColumnHidden, isColumnOptional, + resolveColumnHidden, matchAxisId, readAnnotation, readAnnotationJson, @@ -260,14 +261,16 @@ export function makeColDef( headerName, lockPosition: spec.type === "axis" || (isLabelColumnSpec(spec.spec) && spec.spec.axesSpec.length === 1), - // Visibility = block default (isColumnOptional) reconciled with the user's - // explicit overrides. The block default always applies, so a column whose - // default flips between runs (e.g. filter/ranking config changed) is shown - // or hidden per its CURRENT default unless the user explicitly toggled it. - hide: shownColIds?.includes(colId) - ? false - : (hiddenColIds?.includes(colId) ?? false) || - (spec.type === "column" && isColumnOptional(spec.spec)), + // Visibility = block default reconciled with the user's explicit overrides via the + // shared resolveColumnHidden (same precedence as the model's computeHiddenColumns). + // forcedHidden is false here: hidden columns are filtered out upstream by + // selectDisplayableIndices, and axes carry no table/visibility default. + hide: resolveColumnHidden({ + forcedHidden: false, + optional: spec.type === "column" && isColumnOptional(spec.spec), + userShown: shownColIds?.includes(colId) ?? false, + userHidden: hiddenColIds?.includes(colId) ?? false, + }), valueFormatter: columnRenderingSpec.valueFormatter, headerComponent: PlAgColumnHeader, cellRendererSelector: cellButtonAxisParams?.showCellButtonForAxisId From 7a1b48dbf7dcd3e574f84146e6708273f6f4c416 Mon Sep 17 00:00:00 2001 From: Paul Newling Date: Sat, 30 May 2026 10:18:10 -0700 Subject: [PATCH 04/13] MILAB-6002: set shownColIds to null in default PTable params Pairs the show-override list with the existing hiddenColIds: null in createDefaultPTableParams, so fresh state spells out both override lists explicitly. shownColIds is optional on the null-source variant, so this is a clarity-only change with no behavioural effect. --- sdk/model/src/components/PlDataTable/state-migration.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/model/src/components/PlDataTable/state-migration.ts b/sdk/model/src/components/PlDataTable/state-migration.ts index d11bea3db7..a6b6d26e3b 100644 --- a/sdk/model/src/components/PlDataTable/state-migration.ts +++ b/sdk/model/src/components/PlDataTable/state-migration.ts @@ -441,6 +441,7 @@ export function createDefaultPTableParams(): PTableParamsV2 { return { sourceId: null, hiddenColIds: null, + shownColIds: null, filters: null, defaultFilters: null, sorting: [], From 40525951166c89faf6e3f16af6c00bed40d78391 Mon Sep 17 00:00:00 2001 From: Paul Newling Date: Mon, 1 Jun 2026 10:11:30 -0700 Subject: [PATCH 05/13] MILAB-6002: address review feedback (semver, makeColDef sets, docs) - Bump @platforma-sdk/model to minor: resolveColumnHidden is a new public export. - makeColDef takes ReadonlySet lookups built once in calculateGridOptions, replacing per-column Array.includes and mirroring the model's Set-based computeHiddenColumns. - Clarify the resolveColumnHidden doc comment: only the default-vs-override precedence is shared; each caller layers preserved/forced-hidden handling on top. --- .../milab-6002-table-visibility-deviations.md | 5 ++++- .../PlDataTable/createPlDataTable/utils.ts | 11 +++++++---- .../PlAgDataTable/sources/table-source-v2.ts | 15 ++++++++++----- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/.changeset/milab-6002-table-visibility-deviations.md b/.changeset/milab-6002-table-visibility-deviations.md index 22ec580671..16cdc9b602 100644 --- a/.changeset/milab-6002-table-visibility-deviations.md +++ b/.changeset/milab-6002-table-visibility-deviations.md @@ -1,5 +1,5 @@ --- -'@platforma-sdk/model': patch +'@platforma-sdk/model': minor '@platforma-sdk/ui-vue': patch --- @@ -17,3 +17,6 @@ block's `pl7.app/table/visibility` default, so the current default always applie to untouched columns; the full and visible table handles reconcile the same way. Persisted state migrates v7 -> v8: a one-time reset of custom column show/hide, after which defaults apply correctly. + +Adds `resolveColumnHidden` to the public model API — the shared default-vs-override +precedence used by the visible table handle and the grid — so this is a `minor` bump. diff --git a/sdk/model/src/components/PlDataTable/createPlDataTable/utils.ts b/sdk/model/src/components/PlDataTable/createPlDataTable/utils.ts index 1f78a9502b..2c994b1bd3 100644 --- a/sdk/model/src/components/PlDataTable/createPlDataTable/utils.ts +++ b/sdk/model/src/components/PlDataTable/createPlDataTable/utils.ts @@ -52,10 +52,13 @@ export function getEffectiveVisibility( * Reconcile a column's block-defined default visibility with the user's explicit * show/hide overrides into a single "is this column hidden" decision. * - * This is the one place the precedence lives. Both the model (visible table handle, - * see `computeHiddenColumns`) and the UI (grid `colDef`, see `makeColDef`) call it, - * so the two can never drift — divergence between them is what made MILAB-6002 - * reproduce in two places at once. Precedence: + * This is the single place the default-vs-override *precedence* lives. The model + * (visible table handle, see `computeHiddenColumns`) and the UI (grid `colDef`, see + * `makeColDef`) share it, so that decision can't drift between them — divergence + * there is what made MILAB-6002 reproduce in two places at once. Each caller layers + * its own context on top: the model force-keeps sorted/filtered columns visible + * (`collectPreservedColumnIds`); the UI filters forced-hidden columns out upstream + * (passing `forcedHidden: false`). Precedence: * 1. forced-hidden (`pl7.app/table/visibility` = "hidden") — never shown; * 2. an explicit user override — `shown` wins over `hidden`; * 3. otherwise the column's CURRENT `optional` default. diff --git a/sdk/ui-vue/src/components/PlAgDataTable/sources/table-source-v2.ts b/sdk/ui-vue/src/components/PlAgDataTable/sources/table-source-v2.ts index 168d8c4930..b54087c18b 100644 --- a/sdk/ui-vue/src/components/PlAgDataTable/sources/table-source-v2.ts +++ b/sdk/ui-vue/src/components/PlAgDataTable/sources/table-source-v2.ts @@ -134,10 +134,15 @@ export async function calculateGridOptions({ tableSpecs, ); + // Build lookup sets once (rather than Array.includes per column), mirroring the + // Set-based reconciliation in the model's computeHiddenColumns. + const hiddenColIdSet = new Set(hiddenColIds ?? []); + const shownColIdSet = new Set(shownColIds ?? []); + const columnDefs: ColDef[] = [ makeRowNumberColDef(), ...fields.map((field) => - makeColDef(field, tableSpecs[field], hiddenColIds, shownColIds, cellButtonAxisParams), + makeColDef(field, tableSpecs[field], hiddenColIdSet, shownColIdSet, cellButtonAxisParams), ), ]; @@ -234,8 +239,8 @@ export type PlAgCellButtonAxisParams = { export function makeColDef( iCol: number, spec: PTableColumnSpec, - hiddenColIds: PlTableColumnIdJson[] | undefined, - shownColIds: PlTableColumnIdJson[] | undefined, + hiddenColIds: ReadonlySet, + shownColIds: ReadonlySet, cellButtonAxisParams?: PlAgCellButtonAxisParams, ): ColDef { const colId = canonicalizeJson(getPTableColumnId(spec)); @@ -268,8 +273,8 @@ export function makeColDef( hide: resolveColumnHidden({ forcedHidden: false, optional: spec.type === "column" && isColumnOptional(spec.spec), - userShown: shownColIds?.includes(colId) ?? false, - userHidden: hiddenColIds?.includes(colId) ?? false, + userShown: shownColIds.has(colId), + userHidden: hiddenColIds.has(colId), }), valueFormatter: columnRenderingSpec.valueFormatter, headerComponent: PlAgColumnHeader, From fb37c941a2d357360da11c88d3a6e86bd92579e1 Mon Sep 17 00:00:00 2001 From: Paul Newling Date: Tue, 9 Jun 2026 10:26:10 -0700 Subject: [PATCH 06/13] MILAB-6002: make deprecated createPlDataTableV2 deviation-aware The shared grid UI now stores column visibility as user deviations (hiddenColIds + shownColIds), and createPlDataTableV3 reads them. createPlDataTableV2 still read hiddenColIds as an absolute set and ignored shownColIds, so the ~29 blocks still on the V2 path over-included columns once a user changed visibility (worst case: a single show-override -> empty hide list -> every column fetched). Extract computeHiddenColumnsV2, reconciling each column's block default with the user's show/hide deviations via the shared resolveColumnHidden -- same precedence as the V3 model and the grid's makeColDef. forcedHidden is false, matching makeColDef and V2's historical behaviour (V2 never auto-hid visibility:"hidden" columns). Add createPlDataTableV2.test.ts: deviation interpretation, the empty-hiddenColIds regression, shownColIds honoured, the MILAB-6002 default-flip case, and a guard that forced-hidden columns stay visible (forcedHidden: false). --- .../milab-6002-table-visibility-deviations.md | 5 ++ .../createPlDataTableV2.test.ts | 75 +++++++++++++++++++ .../createPlDataTable/createPlDataTableV2.ts | 71 ++++++++++++++---- 3 files changed, 135 insertions(+), 16 deletions(-) create mode 100644 sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.test.ts diff --git a/.changeset/milab-6002-table-visibility-deviations.md b/.changeset/milab-6002-table-visibility-deviations.md index 16cdc9b602..73a69bcc18 100644 --- a/.changeset/milab-6002-table-visibility-deviations.md +++ b/.changeset/milab-6002-table-visibility-deviations.md @@ -20,3 +20,8 @@ after which defaults apply correctly. Adds `resolveColumnHidden` to the public model API — the shared default-vs-override precedence used by the visible table handle and the grid — so this is a `minor` bump. + +Also updates the deprecated `createPlDataTableV2` path to read the same deviations (and +honour `shownColIds`) via `resolveColumnHidden`, so blocks still on it stay consistent +with the shared grid UI rather than misreading the deviation lists as an absolute hidden +set — which over-included columns once visibility was customised. diff --git a/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.test.ts b/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.test.ts new file mode 100644 index 0000000000..b044eb5774 --- /dev/null +++ b/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.test.ts @@ -0,0 +1,75 @@ +import { + Annotation, + type PColumnSpec, + type PObjectId, + type PTableColumnId, +} from "@milaboratories/pl-model-common"; +import { describe, expect, test } from "vitest"; +import { computeHiddenColumnsV2 } from "./createPlDataTableV2"; + +function col(id: string, visibility?: "optional" | "hidden"): { id: PObjectId; spec: PColumnSpec } { + return { + id: id as PObjectId, + spec: { + kind: "PColumn", + name: id, + valueType: "Int", + axesSpec: [], + annotations: visibility ? { [Annotation.Table.Visibility]: visibility } : {}, + } as PColumnSpec, + }; +} + +const colRef = (id: string): PTableColumnId => + ({ type: "column", id: id as PObjectId }) as PTableColumnId; + +const hiddenIds = (s: Set): string[] => [...s].sort(); + +describe("computeHiddenColumnsV2 (deviation-aware)", () => { + test("with no overrides, hides optional columns and shows the rest", () => { + const cols = [col("visible"), col("opt", "optional")]; + expect(hiddenIds(computeHiddenColumnsV2(cols, null, null))).toEqual(["opt"]); + }); + + test("a user-hidden column (block default visible) becomes hidden", () => { + const cols = [col("a"), col("b")]; + expect(hiddenIds(computeHiddenColumnsV2(cols, [colRef("a")], null))).toEqual(["a"]); + }); + + test("a user-shown column (block default optional) becomes visible", () => { + const cols = [col("a", "optional"), col("b", "optional")]; + expect(hiddenIds(computeHiddenColumnsV2(cols, null, [colRef("a")]))).toEqual(["b"]); + }); + + // Regression for the old absolute-set reader: a non-null hide list was treated as + // "hide exactly these", so an empty list (user only showed a column) unhid EVERY + // optional column. Deviations must keep untouched optional columns hidden. + test("empty hiddenColIds with a show-override keeps other optional columns hidden", () => { + const cols = [col("opt1", "optional"), col("opt2", "optional")]; + expect(hiddenIds(computeHiddenColumnsV2(cols, [], [colRef("opt1")]))).toEqual(["opt2"]); + }); + + // MILAB-6002: an untouched column follows its CURRENT default across re-runs, + // rather than being pinned by a stale saved set. + test("a column whose default flips to optional is re-hidden when untouched", () => { + expect(hiddenIds(computeHiddenColumnsV2([col("flip")], null, null))).toEqual([]); + expect(hiddenIds(computeHiddenColumnsV2([col("flip", "optional")], null, null))).toEqual([ + "flip", + ]); + }); + + // forcedHidden is intentionally false in V2: a `visibility: "hidden"` column is treated + // like any default-visible column and never auto-hidden (V2 has never hidden these, and + // the grid filters them upstream). Guards against a regression to forcedHidden: + // isColumnHidden. + test("a forced-hidden column is not auto-hidden", () => { + expect(hiddenIds(computeHiddenColumnsV2([col("forced", "hidden")], null, null))).toEqual([]); + }); + + // Axis references in the override lists are ignored (only column overrides apply). + test("axis-type override entries are ignored", () => { + const cols = [col("a")]; + const axisRef = { type: "axis", id: { name: "x" } } as unknown as PTableColumnId; + expect(hiddenIds(computeHiddenColumnsV2(cols, [axisRef], null))).toEqual([]); + }); +}); diff --git a/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.ts b/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.ts index 2d69a6f967..8801268d64 100644 --- a/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.ts +++ b/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.ts @@ -1,6 +1,7 @@ import type { AxisId, PColumn, + PColumnSpec, PObjectId, PTableColumnId, PTableColumnIdAxis, @@ -32,7 +33,8 @@ import { getMatchingLabelColumns } from "../labels"; import { collectFilterSpecColumns } from "../../../filters/traverse"; import { isEmpty } from "es-toolkit/compat"; import { createPTableDefV2 } from "./createPTableDefV2"; -import { isColumnOptional } from "./utils"; +import { isColumnOptional, resolveColumnHidden } from "./utils"; +import type { Nil } from "@milaboratories/helpers"; /** * @deprecated This function is deprecated and will be removed in future. Please migrate to createPlDataTable with v3 options for improved column discovery and display configuration. See createPlDataTableOptionsV3 for details on the new options format and migration guidance. @@ -135,21 +137,15 @@ export function createPlDataTableV2( const pframeHandle = ctx.createPFrame(fullColumns); if (!fullHandle || !pframeHandle) return undefined; - const hiddenColumns = new Set( - ((): PObjectId[] => { - // Inner join works as a filter - all columns must be present - if (coreJoinType === "inner") return []; - - const hiddenColIds = tableStateNormalized.pTableParams.hiddenColIds; - if (hiddenColIds !== null) { - return hiddenColIds - .filter((s): s is PTableColumnIdColumn => s.type === "column") - .map((s) => s.id); - } - - return columns.filter((c) => isColumnOptional(c.spec)).map((c) => c.id); - })(), - ); + const hiddenColumns = + // Inner join works as a filter - all columns must be present. + coreJoinType === "inner" + ? new Set() + : computeHiddenColumnsV2( + columns, + tableStateNormalized.pTableParams.hiddenColIds, + tableStateNormalized.pTableParams.shownColIds, + ); // Preserve linker columns columns.filter((c) => isLinkerColumn(c.spec)).forEach((c) => hiddenColumns.delete(c.id)); @@ -209,6 +205,49 @@ export function createPlDataTableV2( } satisfies PlDataTableModel; } +/** + * Reconcile each column's block default visibility with the user's explicit show/hide + * deviations into the set of columns to hide, via the shared {@link resolveColumnHidden} + * — the same precedence the V3 model (`computeHiddenColumns`) and the UI (`makeColDef`) + * use. This keeps the deprecated V2 path from misreading the stored deviation lists as an + * absolute hidden set, which dropped block defaults once any visibility was customised. + * + * `forcedHidden` is false: V2 has never auto-hidden `visibility: "hidden"` columns, and + * the UI filters them out of the grid upstream, so this preserves V2's behaviour and + * matches `makeColDef`. The caller then force-keeps sorted, filtered, linker, and core + * columns visible. + * + * Exported for unit testing. + */ +export function computeHiddenColumnsV2( + columns: { readonly id: PObjectId; readonly spec: PColumnSpec }[], + hiddenSpecs: Nil | PTableColumnId[], + shownSpecs: Nil | PTableColumnId[], +): Set { + const userHidden = new Set( + (hiddenSpecs ?? []) + .filter((s): s is PTableColumnIdColumn => s.type === "column") + .map((s) => s.id), + ); + const userShown = new Set( + (shownSpecs ?? []) + .filter((s): s is PTableColumnIdColumn => s.type === "column") + .map((s) => s.id), + ); + return new Set( + columns + .filter((c) => + resolveColumnHidden({ + forcedHidden: false, + optional: isColumnOptional(c.spec), + userShown: userShown.has(c.id), + userHidden: userHidden.has(c.id), + }), + ) + .map((c) => c.id), + ); +} + function getAllLabelColumns( resultPool: AxisLabelProvider & ColumnProvider, ): PColumn[] | undefined { From f3a73ad314af9510b94d485cddfd80bae4443585 Mon Sep 17 00:00:00 2001 From: Paul Newling Date: Tue, 9 Jun 2026 12:15:13 -0700 Subject: [PATCH 07/13] MILAB-6002: guard saved column visibility against a not-ready grid; sort deviations - deriveColumnVisibility falls back to the prior persisted deviations when the live grid reports no columns (mounting before column defs arrive, or teardown), so a not-ready grid no longer overwrites the user's show/hide with empties. - computeVisibilityDeviations sorts both deviation arrays, so a pure column reorder no longer reads as a state change and forces a spurious grid reload. - Tests cover both, and pin the known limitation where an explicit hide is absorbed when a column's default flips to optional and back. --- .../PlAgDataTable/PlAgDataTableV2.vue | 26 ++--- .../sources/column-visibility.test.ts | 94 ++++++++++++++++++- .../sources/column-visibility.ts | 34 ++++++- 3 files changed, 138 insertions(+), 16 deletions(-) diff --git a/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue b/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue index 087d009306..6693304d56 100644 --- a/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue +++ b/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue @@ -31,10 +31,7 @@ import { DeferredCircular, ensureNodeVisible } from "./sources/focus-row"; import { PlAgDataTableRowNumberColId } from "./sources/row-number"; import type { PlAgCellButtonAxisParams } from "./sources/table-source-v2"; import { calculateGridOptions } from "./sources/table-source-v2"; -import { - computeVisibilityDeviations, - type ColumnVisibilityState, -} from "./sources/column-visibility"; +import { deriveColumnVisibility, type ColumnVisibilityState } from "./sources/column-visibility"; import { useTableState } from "./sources/table-state-v2"; import type { PlAgDataTableV2Controller, @@ -188,15 +185,16 @@ gridOptions.value.initialState = gridState.value; // Restore proper types erased by AgGrid. // columnVisibility is stored as the user's *deviations* from the block-defined -// default visibility, computed from the live grid columns (see -// computeVisibilityDeviations) rather than copied from AG Grid's absolute -// hidden-list. This keeps saved state stable across re-runs when the block -// changes a column's default visibility (e.g. filter/ranking config changed). +// default visibility, derived from the live grid columns (see +// deriveColumnVisibility) rather than copied from AG Grid's absolute hidden-list. +// This keeps saved state stable across re-runs when the block changes a column's +// default visibility (e.g. filter/ranking config changed). deriveColumnVisibility +// also falls back to the prior state when the grid has no columns yet, so a +// not-ready grid (pre-columnDefs / teardown) can't wipe saved show/hide. function makePartialState( state: GridState, api: GridApi, ): PlDataTableGridStateCore { - const deviations = computeVisibilityDeviations(readGridColumnVisibility(api)); return { columnOrder: state.columnOrder as | { @@ -211,10 +209,12 @@ function makePartialState( }[]; } | undefined, - columnVisibility: - deviations.hiddenColIds.length === 0 && deviations.shownColIds.length === 0 - ? undefined - : deviations, + // Fall back to the prior persisted deviations when the live grid reports no + // columns (mid-mount / teardown), so a not-ready grid can't wipe saved show/hide. + columnVisibility: deriveColumnVisibility( + readGridColumnVisibility(api), + gridState.value.columnVisibility, + ), }; } diff --git a/sdk/ui-vue/src/components/PlAgDataTable/sources/column-visibility.test.ts b/sdk/ui-vue/src/components/PlAgDataTable/sources/column-visibility.test.ts index a36aa301d1..9e3a451c07 100644 --- a/sdk/ui-vue/src/components/PlAgDataTable/sources/column-visibility.test.ts +++ b/sdk/ui-vue/src/components/PlAgDataTable/sources/column-visibility.test.ts @@ -1,6 +1,11 @@ import { describe, expect, test } from "vitest"; -import type { PlTableColumnIdJson } from "@platforma-sdk/model"; -import { computeVisibilityDeviations, type ColumnVisibilityState } from "./column-visibility"; +import { isJsonEqual } from "@milaboratories/helpers"; +import { resolveColumnHidden, type PlTableColumnIdJson } from "@platforma-sdk/model"; +import { + computeVisibilityDeviations, + deriveColumnVisibility, + type ColumnVisibilityState, +} from "./column-visibility"; const id = (s: string): PlTableColumnIdJson => s as PlTableColumnIdJson; @@ -53,4 +58,89 @@ describe("computeVisibilityDeviations", () => { expect(deviations.hiddenColIds).toHaveLength(0); expect(deviations.shownColIds).toHaveLength(0); }); + + // The same logical deviation set must serialise identically regardless of the live + // grid column order, otherwise the reload watch (order-sensitive isJsonEqual on the + // persisted vs current state) fires a spurious grid reload on a pure column reorder. + test("output is order-stable across a column reorder", () => { + const order1: ColumnVisibilityState[] = [ + { colId: id("a"), hidden: true, optional: false }, + { colId: id("b"), hidden: true, optional: false }, + { colId: id("c"), hidden: false, optional: true }, + { colId: id("d"), hidden: false, optional: true }, + ]; + const order2: ColumnVisibilityState[] = [order1[3], order1[1], order1[0], order1[2]]; + expect( + isJsonEqual(computeVisibilityDeviations(order1), computeVisibilityDeviations(order2)), + ).toBe(true); + expect(computeVisibilityDeviations(order1)).toEqual({ + hiddenColIds: [id("a"), id("b")], + shownColIds: [id("c"), id("d")], + }); + }); +}); + +describe("deriveColumnVisibility", () => { + const prev = { hiddenColIds: [id("X")], shownColIds: [] }; + + // MILAB-6002 #1: the grid is created without column defs and only receives them once + // calculateGridOptions resolves. A grid state event in that window (or on teardown) + // reports zero columns; deriving from an empty set must NOT wipe saved deviations. + test("an empty live-column set keeps the prior persisted deviations", () => { + expect(deriveColumnVisibility([], prev)).toEqual(prev); + expect(deriveColumnVisibility([], undefined)).toBeUndefined(); + }); + + test("a non-empty column set derives fresh deviations, ignoring prior", () => { + const cols: ColumnVisibilityState[] = [{ colId: id("a"), hidden: true, optional: false }]; + expect(deriveColumnVisibility(cols, prev)).toEqual({ + hiddenColIds: [id("a")], + shownColIds: [], + }); + }); + + test("no deviations collapse to undefined (all columns follow their default)", () => { + const cols: ColumnVisibilityState[] = [ + { colId: id("a"), hidden: false, optional: false }, + { colId: id("b"), hidden: true, optional: true }, + ]; + expect(deriveColumnVisibility(cols, prev)).toBeUndefined(); + }); +}); + +// MILAB-6002 #2 — KNOWN LIMITATION of the pure-deviation model, pinned so a future +// change is noticed. resolveColumnHidden (model) and computeVisibilityDeviations (UI) +// form the save/load cycle. When a user-hidden column's default later flips to optional, +// the override coincides with the default and is dropped (cannot be distinguished from +// "following the default"); if the default flips back the column reappears. Preserving it +// would require storing more than deviations. If the product decides hides must survive, +// update this test and the model. +describe("explicit hide is absorbed when the default flips to optional and back", () => { + const X = "X" as PlTableColumnIdJson; + + // One block run: load visibility (makeColDef -> resolveColumnHidden), optionally apply + // a user toggle, then re-derive the deviations from the resulting grid state. + function run(optional: boolean, hidden: Set, shown: Set, userHides?: boolean) { + let isHidden = resolveColumnHidden({ + forcedHidden: false, + optional, + userShown: shown.has(X), + userHidden: hidden.has(X), + }); + if (userHides !== undefined) isHidden = userHides; + const dev = computeVisibilityDeviations([{ colId: X, hidden: isHidden, optional }]); + return { isHidden, hidden: new Set(dev.hiddenColIds), shown: new Set(dev.shownColIds) }; + } + + test("the run-1 hide does not survive an optional flip-and-back (documented loss)", () => { + const r1 = run(/* optional */ false, new Set(), new Set(), /* userHides */ true); + expect([...r1.hidden]).toEqual(["X"]); // hide recorded + + const r2 = run(/* optional */ true, r1.hidden, r1.shown); // default flips optional + expect(r2.isHidden).toBe(true); // still hidden this run (userHidden wins)... + expect([...r2.hidden]).toEqual([]); // ...but the deviation is dropped (coincides with default) + + const r3 = run(/* optional */ false, r2.hidden, r2.shown); // default flips back to visible + expect(r3.isHidden).toBe(false); // current behavior: the column reappears + }); }); diff --git a/sdk/ui-vue/src/components/PlAgDataTable/sources/column-visibility.ts b/sdk/ui-vue/src/components/PlAgDataTable/sources/column-visibility.ts index 693c82c30f..6729d6b201 100644 --- a/sdk/ui-vue/src/components/PlAgDataTable/sources/column-visibility.ts +++ b/sdk/ui-vue/src/components/PlAgDataTable/sources/column-visibility.ts @@ -1,4 +1,4 @@ -import type { PlTableColumnIdJson } from "@platforma-sdk/model"; +import type { PlDataTableGridStateCore, PlTableColumnIdJson } from "@platforma-sdk/model"; /** A grid column reduced to what visibility-deviation derivation needs. */ export type ColumnVisibilityState = { @@ -31,5 +31,37 @@ export function computeVisibilityDeviations(columns: ColumnVisibilityState[]): { if (c.hidden && !c.optional) hiddenColIds.push(c.colId); else if (!c.hidden && c.optional) shownColIds.push(c.colId); } + // Canonical-sort so the same logical deviation set serialises identically + // regardless of the live grid column order. The reload watch compares persisted + // vs current state with the order-sensitive isJsonEqual, so an unsorted output + // would make a pure column reorder look like a state change and fire a spurious + // grid reload (lost scroll/selection). + hiddenColIds.sort(); + shownColIds.sort(); return { hiddenColIds, shownColIds }; } + +/** + * Reconcile the deviations derived from the live grid columns with the previously + * persisted ones into the value to store in {@link PlDataTableGridStateCore.columnVisibility}. + * + * The grid is built without column defs and only receives them once + * `calculateGridOptions` resolves; during that window (and on teardown) a grid state + * event can fire while the grid reports zero columns. Deriving from an empty column + * set there yields empty deviations, which — written back — would silently wipe the + * user's saved show/hide. An empty live-column set means "grid not ready", not "user + * cleared every override", so fall back to the prior persisted value. (MILAB-6002.) + * + * Otherwise: no deviations collapse to `undefined` (equivalent to "all columns follow + * their default"), matching how the value is read back. + */ +export function deriveColumnVisibility( + columns: ColumnVisibilityState[], + prev: PlDataTableGridStateCore["columnVisibility"], +): PlDataTableGridStateCore["columnVisibility"] { + if (columns.length === 0) return prev; + const deviations = computeVisibilityDeviations(columns); + return deviations.hiddenColIds.length === 0 && deviations.shownColIds.length === 0 + ? undefined + : deviations; +} From ced539f32a5a32d90f89e83f3356a11678f59f9a Mon Sep 17 00:00:00 2001 From: Paul Newling Date: Tue, 9 Jun 2026 12:15:18 -0700 Subject: [PATCH 08/13] MILAB-6002: fix migrateV7toV8 input aliasing; expand model test coverage - migrateV7toV8 copies pTableParams for the null-sourceId case instead of returning the input by reference, so migrated state no longer aliases its source. - Migration tests cover the full v6->v7->v8 chain, a sourceId with no matching cache entry, and the fresh-object guarantee. - A V3 test pins that a sorted column the user explicitly hid stays force-visible (preservation wins over the hide). --- .../createPlDataTableV3.test.ts | 11 +++ .../PlDataTable/state-migration.test.ts | 78 +++++++++++++++++++ .../components/PlDataTable/state-migration.ts | 2 +- 3 files changed, 90 insertions(+), 1 deletion(-) diff --git a/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.test.ts b/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.test.ts index a533870ab4..52f5cd6960 100644 --- a/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.test.ts +++ b/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV3.test.ts @@ -68,4 +68,15 @@ describe("computeHiddenColumns", () => { const sorting = [{ column: colRef("a") }] as unknown as PTableSorting[]; expect(hiddenIds(computeHiddenColumns(cols, sorting, null, null, null))).toEqual([]); }); + + // Preservation (collectPreservedColumnIds) wins over an explicit user hide: a column the + // user hid but is now sorted/filtered is force-kept visible so the active sort/filter has + // its data. Pins the precedence between resolveColumnHidden and preservation — the grid's + // makeColDef does NOT preserve, so the column is in the model's visible table yet hidden in + // the grid (intended "sort by hidden column"; see the model/UI divergence note). + test("a sorted column the user explicitly hid is still force-kept visible", () => { + const cols = [col("a")]; // block default visible + const sorting = [{ column: colRef("a") }] as unknown as PTableSorting[]; + expect(hiddenIds(computeHiddenColumns(cols, sorting, null, [colRef("a")], null))).toEqual([]); + }); }); diff --git a/sdk/model/src/components/PlDataTable/state-migration.test.ts b/sdk/model/src/components/PlDataTable/state-migration.test.ts index debe5d0b93..7dc299b0fb 100644 --- a/sdk/model/src/components/PlDataTable/state-migration.test.ts +++ b/sdk/model/src/components/PlDataTable/state-migration.test.ts @@ -6,10 +6,13 @@ import type { PlDataTableStateV2V7, PlTableColumnIdJson, } from "./typesV7"; +import type { PlDataTableStateV2V6, PlDataTableV6ColIdJson } from "./typesV6"; const jsonId = (s: string): PlTableColumnIdJson => s as PlTableColumnIdJson; const colRef = (s: string): PTableColumnId => ({ type: "column", id: s as PObjectId }) as PTableColumnId; +const v6ColId = (id: string): PlDataTableV6ColIdJson => + `{"id":"${id}","type":"column"}` as PlDataTableV6ColIdJson; function v7CacheEntry( sourceId: string, @@ -74,4 +77,79 @@ describe("upgradePlDataTableStateV2 — v7 to v8 column visibility", () => { expect(out.version).toBe(8); expect(out.pTableParams.sourceId).toBeNull(); }); + + // The null-sourceId branch used to return the input pTableParams by reference, so the + // migrated state aliased its input — a later in-place edit would corrupt the source, + // violating the workspace's "treat reactive state as immutable" rule. It must be a copy. + test("null-sourceId migration returns a fresh pTableParams (no input aliasing)", () => { + const pTableParams = { + sourceId: null, + hiddenColIds: null, + sorting: [], + filters: null, + defaultFilters: null, + } as const; + const v7: PlDataTableStateV2V7 = { version: 7, stateCache: [], pTableParams }; + + const out = upgradePlDataTableStateV2(v7); + + expect(out.pTableParams).not.toBe(pTableParams); + expect(out.pTableParams.sourceId).toBeNull(); + }); + + // The reset is keyed off pTableParams.sourceId, not a cache lookup — a sourceId with no + // matching cache entry must still reset cleanly (params nulled, all cache visibility wiped). + test("v7 sourceId with no matching cache entry still resets cleanly", () => { + const v7: PlDataTableStateV2V7 = { + version: 7, + stateCache: [v7CacheEntry("other-source", [jsonId("a")])], + pTableParams: { + sourceId: "src1", // no cache entry for src1 + hiddenColIds: [colRef("a")], + sorting: [], + filters: null, + defaultFilters: null, + }, + }; + + const out = upgradePlDataTableStateV2(v7); + + expect(out.version).toBe(8); + expect(out.pTableParams.hiddenColIds).toBeNull(); + expect(out.pTableParams.shownColIds).toBeNull(); + expect(out.stateCache[0].gridState.columnVisibility).toBeUndefined(); + }); + + // Full chain: a real upgrade enters at the persisted version and runs every migration + // in sequence. v6 (PTableColumnSpec colIds) -> v7 (PTableColumnId colIds) -> v8 (reset). + test("migrates a v6 state through v7 to v8, resetting column visibility", () => { + const v6: PlDataTableStateV2V6 = { + version: 6, + stateCache: [ + { + sourceId: "src1", + gridState: { columnVisibility: { hiddenColIds: [v6ColId("a"), v6ColId("b")] } }, + sheetsState: [], + filtersState: null, + defaultFiltersState: null, + }, + ], + pTableParams: { + sourceId: "src1", + hiddenColIds: [colRef("a"), colRef("b")], + shownColIds: null, + sorting: [], + filters: null, + defaultFilters: null, + }, + }; + + const out = upgradePlDataTableStateV2(v6); + + expect(out.version).toBe(8); + expect(out.stateCache[0].sourceId).toBe("src1"); + expect(out.stateCache[0].gridState.columnVisibility).toBeUndefined(); + expect(out.pTableParams.hiddenColIds).toBeNull(); + expect(out.pTableParams.shownColIds).toBeNull(); + }); }); diff --git a/sdk/model/src/components/PlDataTable/state-migration.ts b/sdk/model/src/components/PlDataTable/state-migration.ts index a6b6d26e3b..599c960031 100644 --- a/sdk/model/src/components/PlDataTable/state-migration.ts +++ b/sdk/model/src/components/PlDataTable/state-migration.ts @@ -283,7 +283,7 @@ function migrateV7toV8(state: PlDataTableStateV2V7): PlDataTableStateV2Normalize ), pTableParams: state.pTableParams.sourceId === null - ? state.pTableParams + ? { ...state.pTableParams } : { ...state.pTableParams, hiddenColIds: null, shownColIds: null }, }; } From dab39ba4387f1e988f3f48c80b61e564b05fae90 Mon Sep 17 00:00:00 2001 From: Paul Newling Date: Tue, 9 Jun 2026 13:19:55 -0700 Subject: [PATCH 09/13] MILAB-6002: make makePartialState's prior-state dependency explicit makePartialState read the module-scope gridState ref to supply the fallback "prior deviations". Pass it as an explicit `prev` parameter instead, so the function is a pure (state, api, prev) transform and each call site shows what it uses. The reload watch passes the watched gridState, making the "selfState falls back to gridState when the grid has no columns" relationship visible right at the comparison. --- .../PlAgDataTable/PlAgDataTableV2.vue | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue b/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue index 6693304d56..efc90bd683 100644 --- a/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue +++ b/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue @@ -133,6 +133,7 @@ gridOptions.value.onGridPreDestroyed = (event) => { gridOptions.value.initialState = gridState.value = makePartialState( event.api.getState(), event.api, + gridState.value.columnVisibility, ); } gridApi.value = null; @@ -141,7 +142,7 @@ gridOptions.value.onRowDoubleClicked = (event) => { if (event.data && event.data.axesKey) emit("rowDoubleClicked", event.data.axesKey); }; gridOptions.value.onStateUpdated = (event) => { - const partialState = makePartialState(event.state, event.api); + const partialState = makePartialState(event.state, event.api, gridState.value.columnVisibility); // We have to keep initialState synchronized with gridState for gridState recovery after key updating. gridOptions.value.initialState = gridState.value = partialState; @@ -188,12 +189,14 @@ gridOptions.value.initialState = gridState.value; // default visibility, derived from the live grid columns (see // deriveColumnVisibility) rather than copied from AG Grid's absolute hidden-list. // This keeps saved state stable across re-runs when the block changes a column's -// default visibility (e.g. filter/ranking config changed). deriveColumnVisibility -// also falls back to the prior state when the grid has no columns yet, so a -// not-ready grid (pre-columnDefs / teardown) can't wipe saved show/hide. +// default visibility (e.g. filter/ranking config changed). `prev` is the +// caller's current persisted columnVisibility: deriveColumnVisibility falls back +// to it when the grid has no columns yet, so a not-ready grid (pre-columnDefs / +// teardown) can't wipe saved show/hide. function makePartialState( state: GridState, api: GridApi, + prev: PlDataTableGridStateCore["columnVisibility"], ): PlDataTableGridStateCore { return { columnOrder: state.columnOrder as @@ -209,12 +212,7 @@ function makePartialState( }[]; } | undefined, - // Fall back to the prior persisted deviations when the live grid reports no - // columns (mid-mount / teardown), so a not-ready grid can't wipe saved show/hide. - columnVisibility: deriveColumnVisibility( - readGridColumnVisibility(api), - gridState.value.columnVisibility, - ), + columnVisibility: deriveColumnVisibility(readGridColumnVisibility(api), prev), }; } @@ -251,7 +249,7 @@ watch( () => [gridApi.value, gridState.value] as const, ([gridApi, gridState]) => { if (!gridApi || gridApi.isDestroyed()) return; - const selfState = makePartialState(gridApi.getState(), gridApi); + const selfState = makePartialState(gridApi.getState(), gridApi, gridState.columnVisibility); if ( !isJsonEqual(gridState, {}) && !isJsonEqual(stateForReloadCompare(gridState), stateForReloadCompare(selfState)) From 6000b48768ffb38250f3947061862d83cd47323d Mon Sep 17 00:00:00 2001 From: Paul Newling Date: Tue, 9 Jun 2026 13:19:59 -0700 Subject: [PATCH 10/13] MILAB-6002: align migrateV7toV8 pTableParams handling with migrateV6toV7 migrateV7toV8's null-source branch returned a shallow copy while the adjacent migrateV6toV7 passes pTableParams by reference. The copy added an inconsistency without real benefit: it only froze the top-level object, leaving the nested hiddenColIds/sorting/filters arrays aliased anyway. Both migration steps now pass pTableParams by reference. Drop the object-identity test; keep the v6->v7->v8 chain and no-matching-cache-entry coverage. --- .../PlDataTable/state-migration.test.ts | 19 ------------------- .../components/PlDataTable/state-migration.ts | 2 +- 2 files changed, 1 insertion(+), 20 deletions(-) diff --git a/sdk/model/src/components/PlDataTable/state-migration.test.ts b/sdk/model/src/components/PlDataTable/state-migration.test.ts index 7dc299b0fb..3158a58717 100644 --- a/sdk/model/src/components/PlDataTable/state-migration.test.ts +++ b/sdk/model/src/components/PlDataTable/state-migration.test.ts @@ -78,25 +78,6 @@ describe("upgradePlDataTableStateV2 — v7 to v8 column visibility", () => { expect(out.pTableParams.sourceId).toBeNull(); }); - // The null-sourceId branch used to return the input pTableParams by reference, so the - // migrated state aliased its input — a later in-place edit would corrupt the source, - // violating the workspace's "treat reactive state as immutable" rule. It must be a copy. - test("null-sourceId migration returns a fresh pTableParams (no input aliasing)", () => { - const pTableParams = { - sourceId: null, - hiddenColIds: null, - sorting: [], - filters: null, - defaultFilters: null, - } as const; - const v7: PlDataTableStateV2V7 = { version: 7, stateCache: [], pTableParams }; - - const out = upgradePlDataTableStateV2(v7); - - expect(out.pTableParams).not.toBe(pTableParams); - expect(out.pTableParams.sourceId).toBeNull(); - }); - // The reset is keyed off pTableParams.sourceId, not a cache lookup — a sourceId with no // matching cache entry must still reset cleanly (params nulled, all cache visibility wiped). test("v7 sourceId with no matching cache entry still resets cleanly", () => { diff --git a/sdk/model/src/components/PlDataTable/state-migration.ts b/sdk/model/src/components/PlDataTable/state-migration.ts index 599c960031..a6b6d26e3b 100644 --- a/sdk/model/src/components/PlDataTable/state-migration.ts +++ b/sdk/model/src/components/PlDataTable/state-migration.ts @@ -283,7 +283,7 @@ function migrateV7toV8(state: PlDataTableStateV2V7): PlDataTableStateV2Normalize ), pTableParams: state.pTableParams.sourceId === null - ? { ...state.pTableParams } + ? state.pTableParams : { ...state.pTableParams, hiddenColIds: null, shownColIds: null }, }; } From dd9ad7bb5eb2dea629eaf2ca255cb9b70e3dd664 Mon Sep 17 00:00:00 2001 From: Paul Newling Date: Tue, 9 Jun 2026 16:16:41 -0700 Subject: [PATCH 11/13] MILAB-6002: route createPlDataTableV2 through computeHiddenColumns Delegate computeHiddenColumnsV2 to the shared computeHiddenColumns so the deprecated V2 path uses the same reconciliation as the V3 model and the grid's makeColDef -- removing the duplicated logic and honouring forced-hidden (visibility:"hidden") columns in V2 as they are in V3 (previously V2 left them in the visible table). Update the V2 forced-hidden test and the changeset wording to match. --- .../milab-6002-table-visibility-deviations.md | 6 +-- .../createPlDataTableV2.test.ts | 13 +++--- .../createPlDataTable/createPlDataTableV2.ts | 41 ++++--------------- 3 files changed, 19 insertions(+), 41 deletions(-) diff --git a/.changeset/milab-6002-table-visibility-deviations.md b/.changeset/milab-6002-table-visibility-deviations.md index 73a69bcc18..b06d203d0e 100644 --- a/.changeset/milab-6002-table-visibility-deviations.md +++ b/.changeset/milab-6002-table-visibility-deviations.md @@ -21,7 +21,7 @@ after which defaults apply correctly. Adds `resolveColumnHidden` to the public model API — the shared default-vs-override precedence used by the visible table handle and the grid — so this is a `minor` bump. -Also updates the deprecated `createPlDataTableV2` path to read the same deviations (and -honour `shownColIds`) via `resolveColumnHidden`, so blocks still on it stay consistent -with the shared grid UI rather than misreading the deviation lists as an absolute hidden +Also routes the deprecated `createPlDataTableV2` path through the shared +`computeHiddenColumns`, so the ~29 blocks still on it reconcile the same deviations (and +honour `shownColIds`) as the grid instead of misreading the lists as an absolute hidden set — which over-included columns once visibility was customised. diff --git a/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.test.ts b/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.test.ts index b044eb5774..62ba259c39 100644 --- a/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.test.ts +++ b/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.test.ts @@ -58,12 +58,13 @@ describe("computeHiddenColumnsV2 (deviation-aware)", () => { ]); }); - // forcedHidden is intentionally false in V2: a `visibility: "hidden"` column is treated - // like any default-visible column and never auto-hidden (V2 has never hidden these, and - // the grid filters them upstream). Guards against a regression to forcedHidden: - // isColumnHidden. - test("a forced-hidden column is not auto-hidden", () => { - expect(hiddenIds(computeHiddenColumnsV2([col("forced", "hidden")], null, null))).toEqual([]); + // Forced-hidden (`visibility: "hidden"`) columns are dropped from the visible table, + // matching V3 and the grid (delegates to computeHiddenColumns). Guards against + // regressing V2 back to leaving them in. + test("a forced-hidden column is hidden", () => { + expect(hiddenIds(computeHiddenColumnsV2([col("forced", "hidden")], null, null))).toEqual([ + "forced", + ]); }); // Axis references in the override lists are ignored (only column overrides apply). diff --git a/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.ts b/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.ts index 8801268d64..d6ccccebf2 100644 --- a/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.ts +++ b/sdk/model/src/components/PlDataTable/createPlDataTable/createPlDataTableV2.ts @@ -33,7 +33,7 @@ import { getMatchingLabelColumns } from "../labels"; import { collectFilterSpecColumns } from "../../../filters/traverse"; import { isEmpty } from "es-toolkit/compat"; import { createPTableDefV2 } from "./createPTableDefV2"; -import { isColumnOptional, resolveColumnHidden } from "./utils"; +import { computeHiddenColumns } from "./createPlDataTableV3"; import type { Nil } from "@milaboratories/helpers"; /** @@ -206,16 +206,14 @@ export function createPlDataTableV2( } /** - * Reconcile each column's block default visibility with the user's explicit show/hide - * deviations into the set of columns to hide, via the shared {@link resolveColumnHidden} - * — the same precedence the V3 model (`computeHiddenColumns`) and the UI (`makeColDef`) - * use. This keeps the deprecated V2 path from misreading the stored deviation lists as an - * absolute hidden set, which dropped block defaults once any visibility was customised. + * V2's base hide decision: which columns to drop from the visible table, reconciling each + * column's block default with the user's show/hide deviations. Delegates to the shared + * {@link computeHiddenColumns} so the deprecated V2 path uses the exact same precedence as + * the V3 model and the grid's `makeColDef` — one implementation, no drift, and forced-hidden + * (`visibility: "hidden"`) columns are dropped here just as they are in V3. * - * `forcedHidden` is false: V2 has never auto-hidden `visibility: "hidden"` columns, and - * the UI filters them out of the grid upstream, so this preserves V2's behaviour and - * matches `makeColDef`. The caller then force-keeps sorted, filtered, linker, and core - * columns visible. + * Sort/filter preservation is skipped (both args `null`); the caller then force-keeps + * sorted, filtered, linker, and core columns visible on top of this base set. * * Exported for unit testing. */ @@ -224,28 +222,7 @@ export function computeHiddenColumnsV2( hiddenSpecs: Nil | PTableColumnId[], shownSpecs: Nil | PTableColumnId[], ): Set { - const userHidden = new Set( - (hiddenSpecs ?? []) - .filter((s): s is PTableColumnIdColumn => s.type === "column") - .map((s) => s.id), - ); - const userShown = new Set( - (shownSpecs ?? []) - .filter((s): s is PTableColumnIdColumn => s.type === "column") - .map((s) => s.id), - ); - return new Set( - columns - .filter((c) => - resolveColumnHidden({ - forcedHidden: false, - optional: isColumnOptional(c.spec), - userShown: userShown.has(c.id), - userHidden: userHidden.has(c.id), - }), - ) - .map((c) => c.id), - ); + return computeHiddenColumns(columns, null, null, hiddenSpecs, shownSpecs); } function getAllLabelColumns( From 917ae130552fb502a2b671b85024a59b8eab631d Mon Sep 17 00:00:00 2001 From: Paul Newling Date: Tue, 9 Jun 2026 16:17:00 -0700 Subject: [PATCH 12/13] MILAB-6002: drop dead optional chaining in stateForReloadCompare hiddenColIds is a required field on columnVisibility, so the ?. and ?? 0 guard was dead code (greptile). shownColIds stays optional and keeps its guard. --- sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue b/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue index efc90bd683..8ce4534baa 100644 --- a/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue +++ b/sdk/ui-vue/src/components/PlAgDataTable/PlAgDataTableV2.vue @@ -238,8 +238,7 @@ function readGridColumnVisibility(api: GridApi): ColumnVisib // Normalize columnVisibility for comparison: no deviations is equivalent to undefined. function stateForReloadCompare(state: PlDataTableGridStateCore): PlDataTableGridStateCore { const cv = state.columnVisibility; - const isEmpty = - !cv || ((cv.hiddenColIds?.length ?? 0) === 0 && (cv.shownColIds?.length ?? 0) === 0); + const isEmpty = !cv || (cv.hiddenColIds.length === 0 && (cv.shownColIds?.length ?? 0) === 0); return { ...state, columnVisibility: isEmpty ? undefined : cv }; } From effebc48581cd019187791214defd3173ed045a4 Mon Sep 17 00:00:00 2001 From: Paul Newling Date: Tue, 9 Jun 2026 20:04:38 -0700 Subject: [PATCH 13/13] MILAB-6002: tighten changeset wording Drop the vague "full and visible table handles reconcile the same way" clause -- the full table handle holds every column, so it doesn't reconcile visibility. The sentence is sharper without it. --- .changeset/milab-6002-table-visibility-deviations.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.changeset/milab-6002-table-visibility-deviations.md b/.changeset/milab-6002-table-visibility-deviations.md index b06d203d0e..7845aa99d6 100644 --- a/.changeset/milab-6002-table-visibility-deviations.md +++ b/.changeset/milab-6002-table-visibility-deviations.md @@ -14,9 +14,8 @@ reverting to hidden. Column visibility is now the user's explicit show/hide deviations from the block's `pl7.app/table/visibility` default, so the current default always applies -to untouched columns; the full and visible table handles reconcile the same way. -Persisted state migrates v7 -> v8: a one-time reset of custom column show/hide, -after which defaults apply correctly. +to untouched columns. Persisted state migrates v7 -> v8: a one-time reset of +custom column show/hide, after which defaults apply correctly. Adds `resolveColumnHidden` to the public model API — the shared default-vs-override precedence used by the visible table handle and the grid — so this is a `minor` bump.