From b4e7604dd1792771213ad8a5fb0aaa415d8f1f5f Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Wed, 23 Sep 2026 22:18:52 -0400 Subject: [PATCH] Keep the selection on its rows when the sort moves them A highlight is a rectangle of grid coordinates. Post a decrease on a grid sorted by sales descending and the customer drops down the list, while the highlight holds the coordinate and comes to rest over whoever took that position. `slices` is value-based, so the panel goes on writing to the customer it always meant -- the grid is the only thing that lies, which is the worst place for the lie to be. The rectangles are re-derived from the slices now rather than tracked: after a post or an undo, sweep the grid the way a drag does (slicesFromRegion, quietly) and keep the cells whose slice is still selected. That also survives the column axis shifting, which it does on almost every post -- a new segment column group moves every x by one. - Bounded at 5,000 displayed rows. The sweep reads what the pivot shows, not the grain, so at any ordinary depth it is a handful; fully expanded it is the whole traversal and not worth a highlight. - Past the bound, or on any failure, the highlight is cleared rather than left in place: losing it says "I don't know where it went", which is true, and leaving it says "it is here", which is not. - A slice may now land on more cells than it did -- one that never named the segment column matches the customer under the old segment and under the adjustment's own. That is what the slice covers. Co-Authored-By: Claude Opus 5 --- ui/src/views/Forecast.jsx | 69 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 64 insertions(+), 5 deletions(-) diff --git a/ui/src/views/Forecast.jsx b/ui/src/views/Forecast.jsx index 60f3c99..b07fad1 100644 --- a/ui/src/views/Forecast.jsx +++ b/ui/src/views/Forecast.jsx @@ -293,6 +293,9 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio const tableNameRef = useRef(null) // { x, y } of the body cell the last mousedown landed on, or null const downCellRef = useRef(null) + // Past this many displayed rows a relocate gives up and clears instead -- + // see relocateAreas. + const MAX_RELOCATE_ROWS = 5000 // the region a drag is building, as a Perspective ViewWindow; committed on mouseup const regionRef = useRef(null) // sliceKey -> the grid rectangles that produced it. Kept parallel to `slices` so @@ -363,7 +366,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio // attaches to a click. Row dimensions come from the view's __ROW_PATH__ (raw values, // so dates stay epoch millis rather than whatever the grid formatted them as), and // column dimensions from the split_by segments of the column name. - async function slicesFromRegion(win) { + async function slicesFromRegion(win, { quiet = false } = {}) { const viewer = viewerRef.current if (!viewer) return [] const cfg = await viewer.save() @@ -398,7 +401,9 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio out.push({ slice, area: { x0: c, x1: c, y0: y, y1: y } }) } } - if (blankCols) { + // Quiet for a relocate, which sweeps the whole grid rather than a region the + // user dragged: the blanks it passes over are not cells anyone asked for. + if (blankCols && !quiet) { flash(`${blankCols} cell${blankCols > 1 ? 's' : ''} skipped: a blank column-axis value cannot be isolated`, 'error') } return out @@ -408,20 +413,69 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio // list on every mousedown — so a ctrl-click selection built up over several clicks // would only ever show the last cell. Push the full set back after each change and // redraw, which gets the native highlight for every selected cell for free. - useEffect(() => { - slicesRef.current = slices + function paintAreas() { const grid = gridRef.current const state = grid?.model?._selection_state if (!state) return - const live = new Set(slices.map(sliceKey)) + const live = new Set(slicesRef.current.map(sliceKey)) for (const k of [...areasRef.current.keys()]) { if (!live.has(k)) areasRef.current.delete(k) } state.selected_areas = [...areasRef.current.values()].flat() state.dirty = true grid.regular_table?.draw({ preserve_width: true })?.catch?.(() => {}) + } + + useEffect(() => { + slicesRef.current = slices + paintAreas() }, [slices]) + // A highlight is a rectangle of grid coordinates, and an adjustment moves the + // rows under it: post a decrease on a grid sorted by sales descending and the + // customer drops down the list, while the highlight stays on the coordinate + // and now sits over whoever took that position. `slices` is value-based and + // still names the right customer, so the panel goes on writing to the row it + // always meant -- the grid is the only thing that lies, which is the worst + // place for the lie to be. + // + // So the rectangles are re-derived from the slices rather than tracked: sweep + // the grid the way a drag does, keep the cells whose slice is still selected. + // That also survives the column axis moving, which it does on almost every + // post -- a new segment column group shifts every x by one. + // + // One consequence worth expecting: a slice can legitimately land on more cells + // than it did. A selection that never named the segment column now matches the + // customer's cell under the old segment *and* under the adjustment's own, and + // both are highlighted. That is what the slice covers. + async function relocateAreas() { + const viewer = viewerRef.current + if (!viewer || !slicesRef.current.length) return + try { + await viewer.flush() + const view = await viewer.getView() + const n = await view.num_rows() + // The sweep reads the rows the pivot is displaying, not the grain, so at + // any ordinary depth it is a handful. Fully expanded it is the whole + // traversal, and a slow redraw of a highlight is not worth having. + if (!n || n > MAX_RELOCATE_ROWS) { areasRef.current.clear(); paintAreas(); return } + const found = await slicesFromRegion({ start_row: 0, end_row: n }, { quiet: true }) + const live = new Set(slicesRef.current.map(sliceKey)) + const next = new Map() + for (const { slice, area } of found) { + const k = sliceKey(slice) + if (!live.has(k)) continue + next.set(k, [...(next.get(k) || []), area]) + } + areasRef.current = next + } catch { + // Losing the highlight says "I don't know where it went", which is true. + // Leaving it where it was says "it is here", which is not. + areasRef.current.clear() + } + paintAreas() + } + function onDragStart(e) { e.preventDefault() const vertical = dock === 'bottom' @@ -1732,6 +1786,8 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio // effect that normally recounts is keyed on `slices`, and none of these // change the selection. fetchCurrentTotals(slices) + // The rows just moved under the highlight — see relocateAreas. + relocateAreas() if (op === 'scale') { setScaleInputs({}); setScaleNote('') } if (op === 'recode') { setRecodeNote('') } if (op === 'clone') { setCloneNote(''); setCloneScale('1') } @@ -1947,6 +2003,9 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio window.dispatchEvent(new CustomEvent('pf-data-changed')) refreshLogMeta(versionId) if (slicesRef.current.length) fetchCurrentTotals(slicesRef.current) + // Undo moves the rows back up the sort as surely as the post moved them + // down, and drops the adjustment's column group with them. + relocateAreas() flash(`Undone — ${data.rows_deleted} rows removed`) } catch (err) { flash(err.message, 'error')