Altinity / Altinity/altinity-sql-browser
refactor(shell): a live sidebar drag leaves the docked inspector's width stale
Nessuno ha ancora preso questa issue.
- Lingua principale
- TypeScript
- Stelle
- 8
- Fork
- 2
- Merge medio
- 1h 34m
- PR unite (30g)
- 6
Descrizione
Surfaced by ChatGPT review pass 2 of PR #596 (#586, phase 1 of #593), verified against the code.
Deliberately not fixed in #586 — see "Why deferred".
What
#586 made the docked inspector's displayed width dock-aware: its ceiling reserves
CENTRE_MIN_PX for the centre surface plus reservedPx = state.sidebarPx + 2 * HANDLE_PX
(src/ui/app-shell.ts:174). The re-clamp (app.dom.reclampInspectorWidth) runs at three moments:
- shell construction (
app-shell.ts:252), - inspector unfold (called from
showInInspector), window resize.
A live left-sidebar drag is not one of them. dragCtx.apply (src/ui/app-shell.ts:182-186)
handles the 'col' axis by setting sidebar.style.width only:
apply: (axis, value) => {
if (axis === 'col') sidebar.style.width = value + 'px';
else if (axis === 'rightInspector') inspectorHost.style.width = value + 'px';
else schemaPane.style.height = value + '%';
},
So while both panels are visible, widening the sidebar silently invalidates the reservation the
inspector's width was chosen against.
Reproduction
At a 1280px viewport:
- Open Cell, Rows or Reference and size the inspector to 600px (default sidebar 248px →
reservedPx262 → centre ≈ 418px). - Drag
.col-resizeto the sidebar's maximum 420px (dragValue'sclamp(ev.clientX, 180, 420),
src/ui/splitters.ts:108) →reservedPxbecomes 434. - The inspector stays 600px, so the centre falls to ≈ 246px — below
CENTRE_MIN_PX(320) — and
stays there until an unrelatedwindow resizeor a fold/unfold happens to re-clamp it.
Why the existing tests miss it
Both new tests avoid the live transition:
- the e2e sidebar test closes the inspector, changes the sidebar, then reopens it (so the unfold
hook does the work); - the unit test changes
state.sidebarPxwhile the inspector is folded.
Neither drags the sidebar while the inspector is open, which is the only path that exposes this.
Why deferred
It is a small, self-contained fix, but it shares a root cause with the centre-minimum policy
question (filed separately): both are about what the shell does when the row's three columns
cannot all keep their minima. Fixing the re-clamp alone would keep the centre at
CENTRE_MIN_PX only while that is feasible, so the two are best resolved together. It is also
not a regression — pre-#586 the surface was a position: fixed overlay that the sidebar's width
did not interact with at all.
Acceptance
- The
'col'apply path re-clamps the displayed inspector width, without mutating the
savedrightInspectorPxpreference (the same displayed-vs-preferred split #586 established). - A real-browser test drags
.col-resizewhile the inspector stays open and asserts the
centre againstCENTRE_MIN_PX. happy-dom evaluates no CSS layout, so a unit test cannot
stand in for this. -
npm test,npm run check:types,npm run check:arch,npm run buildpass.
Guida per i contributori
Apri la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Direzione di ricerca
Inizia in src/ui/app-shell.ts intorno a dragCtx.apply e alla logica esistente di reclamp dell’inspector, quindi esamina src/ui/splitters.ts e i test esistenti della sidebar. Esegui prima il test del browser pertinente e aggiungi la copertura per il trascinamento di .col-resize mentre l’inspector rimane aperto. È completato quando il centro rispetta CENTRE_MIN_PX, la preferenza salvata rightInspectorPx rimane invariata e tutti i controlli npm elencati passano.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- typescript
- Ambito
- frontend, testing-qa
- Tipo di issue
- Bug
- Difficoltà
- 4/5
- Tempo stimato
- 3-5 giorni
- Stato di attività
- Tranquilla
- Chiarezza
- Specificata chiaramente
- Idoneità per principianti
- 55/100