fix(web): keep the desktop sidebar resizable - #53
Conversation
Scale the main-content reservation down to half the window on narrow viewports, so a default-sized desktop window can still widen the sidebar instead of starting pinned at its cap, and measure the rail's acceptance check with the same helper as the clamp. Restore the persisted width once per storage key: the effect re-ran on every parent render (callers pass resizable options as an object literal) and overwrote a live drag with the pre-drag width.
Codex review findings on the first commit: - Hydrating once per storage key removed the accidental re-clamp the old effect performed on every render, so shrinking the window left an oversized sidebar squeezing the main content until the next drag. Split restoring from reconciling: reconciliation is idempotent, skips live drags, and keeps the user's preferred width unpersisted so a window that grows again restores it. - Reserve the odd pixel of a narrow viewport for the main content, so the sidebar stays at or below half the window. Parse the applied width strictly as pixels: the resize path only writes px, and reading the provider's "16rem" default as 16 would collapse the sidebar to its minimum.
Codex review round 1 — both findings addressedMedium — viewport changes no longer re-clamped the applied width. Correct, and it was caused by this PR: hydrating once per storage key removed a re-clamp the old effect was doing accidentally on every render. Restoring and reconciling are now separate concerns. Reconciliation is idempotent, skips live drags, and deliberately does not persist — storage keeps the width the user asked for, so a window that grows again restores it instead of keeping a narrower window's cap. Verified against the running Electron app over CDP (stored preference 700px):
Storage still reads 700 throughout. Low — odd viewport widths exceeded the half-window cap by a pixel. Fixed by reserving Also found while fixing the above: the reconcile step read the applied width with 21 tests pass, lint 0 errors, typecheck clean. |
Codex review round 2: - A sidebar that switched storage keys inherited the previous key's preferred width whenever the new key had nothing stored, so an unrelated sidebar's width leaked across. Tag the preference with the key that owns it and ignore it under any other key. - Reconciliation applied a width without consulting shouldAcceptWidth, so a caller's non-numeric constraint could be violated even though the numeric limits held. Route it through the same gate as a drag. Document that limit reconciliation only covers pixel widths: a sidebar left on the provider's rem default opts out until its first drag. That gap predates this branch, and normalising component-owned defaults would make the effect fire on every render for collapsed sidebars.
Codex review round 3: - stopResize settled against whatever options were current when the pointer came up, so options changing mid-drag could persist the result under the wrong storage key or notify the wrong callback. A drag now captures its options at pointer-down and settles against those. - Leaving a storage key and returning to it looked already-hydrated, so the key's persisted width was never reloaded. Track the observed key separately from the hydrated one, and let a drag in flight finish before restoring. - Clamping an out-of-bounds width back inside the limits is mandatory, so it no longer consults shouldAcceptWidth: a caller that rejects shrinking would otherwise stay pinned above a maxWidth that just dropped. The predicate still governs optional movement between two valid widths.
Codex review rounds 2 and 3 — all findings addressedRound 2
Round 3
Stopping the review loop here. Three passes is the cap, and the trend is clear: rounds 2 and 3 found no defect reachable from this application (one caller, one constant storage key, a predicate that always accepts shrinking). They were contract gaps in the reusable primitive, which are now closed, and further rounds would keep hardening an API surface nobody calls. Residual gaps, stated plainly: the effects have no unit tests — the web suite runs on 21 tests pass, lint 0 errors, typecheck clean. Live behaviour re-verified after each round. |
1 participant
Why
Resizing the desktop sidebar had two ways to feel dead, on top of the upstream cause that turned out to be behind the original report.
Upstream cause (already fixed, for context): builds predating pingdotgg#4655 read
window.innerWidthat render time with nothing subscribed to resize. A clamped drag changes no state, so no re-render refreshes the snapshot — a window that grew (e.g. entering full-screen) kept the old, smaller cap forever. If the sidebar already sat at that stale cap, dragging it wider did nothing. That is fixed ondevvia the merge; this PR does not re-fix it.1. The clamp is too aggressive on narrow windows.
resolveThreadSidebarMaximumWidthreserved a flat 40rem for main content, so a default-sized desktop window (~916px inner width) capped the sidebar at 276px — which is roughly where it already sits. There was ~20px of travel to the right, and below ~848px of window width the sidebar could not move at all.2. The persisted-width restore overwrote live drags. Callers pass their resizable options as an object literal, so
resolvedResizable's identity changed on every parent render, and the restoreuseLayoutEffectre-ran each time — re-reading localStorage and slamming--sidebar-widthback to the stored value. Storage is only written once a drag settles, so any unrelated re-render mid-drag snapped the sidebar back to its pre-drag width. An idle window rarely re-renders; a busy one (streaming threads) does so constantly, which is exactly when the rail feels dead.What changed
threadSidebarWidth.ts: the main-content reservation scales down to half the window below ~80rem, so the sidebar always has room to grow and main content still keeps at least half.AppSidebarLayout.tsx:shouldAcceptWidthnow measures with the same helper as the clamp, so the two cannot disagree and refuse a width the clamp just allowed.sidebar.tsx: the restore effect hydrates once per storage key.Verification
Driven against the running Electron dev app over CDP (native-input paths, not just jsdom):
647px → 547px(snapped back to the stored width)847pxholds through the re-render, releases at947pxthreadSidebarWidth.test.tscovers the new reservation rule (wide window, default-sized window, half-window cap, below-minimums floor).17 tests pass,
pnpm lintreports 0 errors,pnpm typecheckis clean.