Skip to content

fix(panels): proportional absorber pins, stale-pin cleanup, double-click to reset#123

Merged
johannesjo merged 2 commits into
johannesjo:mainfrom
nishasdk:panel-layout-improvements
May 20, 2026
Merged

fix(panels): proportional absorber pins, stale-pin cleanup, double-click to reset#123
johannesjo merged 2 commits into
johannesjo:mainfrom
nishasdk:panel-layout-improvements

Conversation

@nishasdk
Copy link
Copy Markdown
Contributor

What

ResizablePanel reliability fixes

  • Switch wrapperRefs from an index-keyed array to an ID-keyed Map — drag measurement now survives dynamic children changes (e.g. the plan tab appearing mid-drag)
  • Add flexGrow prop to PanelChild so absorbers can be weighted (notes-files gets a smaller weight, leaving the AI terminal ~2/3 of the remaining space)
  • Store absorber pins as flex-grow ratios instead of pixel values so they scale proportionally when the window is resized
  • Detect and clear asymmetric absorber pins (one absorber pinned while siblings are not) — these are stale pixel values from before the child was promoted to absorber status, and would otherwise starve the other absorbers

Double-click on tiling resize handles to unpin

  • Double-clicking a resize handle between two task columns resets both panels to the default equal split without any dragging

Layout cleanup

  • Remove the 40vh max-height from TaskNotesBody — it was a workaround for content-size bubbling up the flex chain; notes-files is now a proper weighted absorber so height is layout-driven, not content-driven

Why

Pinned panels previously stored pixel values, which go stale when the window is resized or when an absorber configuration changes. Storing ratios instead keeps the layout self-healing. The double-click reset gives users a quick escape hatch without needing to drag.

Testing

  1. Pin two panels at different widths, then resize the window → proportions should hold
  2. Double-click a tiling resize handle → both panels should snap back to an equal split
  3. Open the plan tab (which adds a new absorber) → drag should measure correctly with no visual jump
  4. No layout breakage in focus mode or with long plan markdown open

PS. just cosmetic fixes! just wanted to test what submitting a PR on parallel code would look like :)

@johannesjo
Copy link
Copy Markdown
Owner

johannesjo commented May 18, 2026

Thank you very much for this! <3

I reviewed this with separate passes over the resize component, task layout wiring, and tiling reset behavior. I think there’s one blocker before this lands:

The TaskNotesBody max-height: 40vh removal relies on notes-files being a weighted absorber, but that wiring doesn’t appear to be in this PR. In TaskPanel, the stack-mode vertical panel still uses absorberIds={['ai-terminal']}, and notesAndFilesChild does not set flexGrow, so notes-files remains content-sized. With the cap removed, long plan markdown can again bubble through the flex tree and push/starve the AI terminal. Please either make notes-files an absorber with the intended smaller weight or keep the cap until that layout change is actually applied.

A few smaller follow-ups I noticed:

  • ResizablePanel now interprets persisted absorber values as flex-grow weights, but dragging between two absorbers only persists those two panels. If this component is ever used with 3+ absorbers, the untouched absorber keeps weight 1 while the dragged pair get pixel-derived weights, so it can collapse unexpectedly.
  • The drag preview state is still index-keyed, even though refs are now ID-keyed. If children change during an active drag before the dragged pair, the live override can apply to the wrong child.
  • In TilingLayout, the double-click reset deletes saved widths but doesn’t schedule updateViewportState(). If the reset changes task boundaries while the total strip width stays the same, offscreen/attention affordance state can remain stale until another scroll or layout update.

@nishasdk
Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review!

This was mostly me experimenting to learn how things fit together, so appreciate the patience!
Re: the TaskNotesBody - I think it may have ended up in a worktree somewhere while I was playing around with arena mode so will track it down.

On the follow-ups: I had similar concerns about the flex-grow weighting and index drift while writing it, but went with the simpler fix for now since I wasn't sure how far this panel pattern is intended to expand. Happy to revisit if you have a sense of where it's headed, especially with behaviour in combination with the Terminal/dev/e2e section, which I haven't really touched.

A few questions before I tinker further:

  1. What are you/ people actually using Notes for day-to-day? What's the intended purpose?
    (My own Notes usage has basically been zero except when Claude generates a Plan. I occasionally copy bits of plan/review comments in there as a reference, but I end up pasting into Obsidian because the editing experience isn't quite there yet. One thing I'd find genuinely useful: if anything you highlight in the plan or review automatically synced into Notes.)

  2. What's the practical difference between sending a prompt from Notes vs. the AI chat interface vs. "Send a Prompt"? I think I have the gist but want to confirm

  3. Is Plan currently Claude-only? I haven't been able to get another agent to trigger the Plan tab but not sure if that's by design or a prompting issue.

Would love to hear how you're thinking about these sections long-term, mostly so I can work around your direction rather than against it. For now I'm just adding things I personally want haha, but if I make a bigger structural change, is it better to gate it behind a settings toggle (like the Send a Prompt panel)?

@johannesjo
Copy link
Copy Markdown
Owner

  1. What are you/ people actually using Notes for day-to-day? What's the intended purpose?
    (My own Notes usage has basically been zero except when Claude generates a Plan. I occasionally copy bits of plan/review comments in there as a reference, but I end up pasting into Obsidian because the editing experience isn't quite there yet. One thing I'd find genuinely useful: if anything you highlight in the plan or review automatically synced into Notes.)

You can use it for everything, but personally I mostly use it for keeping track I've want to evaluate with the agent later, when current work is done.

2. What's the practical difference between sending a prompt from Notes vs. the AI chat interface vs. "Send a Prompt"? I think I have the gist but want to confirm

In terms of what gets send to the agent, there is no difference.

  1. Plan finds new md docs in plan folder. Not sure to be honest, but I think we might need to extend this to work with other agents as plan folder is read from .claude/settings.json.

@johannesjo
Copy link
Copy Markdown
Owner

For this PR I would keep the scope conservative: either restore the notes max-height or wire notes-files as a real weighted absorber. If you want to go further with the absorber work, I would also make dragOverride ID-keyed while you are there, since dynamic children are exactly the case this PR is trying to handle.

I do not think we need to make ResizablePanel a fully general layout solver yet. The current concrete need is 2 absorbers. If we later rely on 3+ absorbers, then dragging should persist the full absorber ratio vector, not just the two panels next to the handle.

Plans are not meant to be Claude-only long term, but the current implementation is Claude-biased: Claude is configured via .claude/settings.local.json, while the watcher also looks at docs/plans. Other agents can work if they write markdown there, but we probably need a clearer agent-agnostic convention.

On prompt surfaces: Notes, the prompt input, and review submission all send through the same running-agent prompt path. Notes are mainly a persistent scratchpad/backlog; the prompt box is the transient next-message composer. Inline "ask about code" is different: that is a separate one-shot Q&A flow, not the task agent session.

For bigger structural changes, I would prefer good defaults over adding settings toggles by default. Add toggles when it is a lasting workflow preference or the UI would otherwise be noisy/controversial.

…ick to reset

ResizablePanel:
- Switch wrapperRefs from an index-keyed array to an ID-keyed Map so drag
  measurement survives dynamic children changes (e.g. plan tab appearing).
- Add flexGrow prop to PanelChild for weighted absorbers (notes-files gets
  a smaller weight so the AI terminal absorbs ~2/3 of remaining space).
- Store absorber pins as flex-grow ratios instead of pixel values so pinned
  panels scale proportionally when the window is resized.
- Detect and clear asymmetric absorber pins (one absorber pinned while
  siblings are not) — these are stale pixel values that would starve siblings.

TilingLayout:
- Double-click on a tiling resize handle calls deletePanelUserSize to unpin
  both neighbours, snapping them back to the default equal split.

TaskNotesBody:
- Remove 40vh max-height cap; notes-files is now a weighted absorber so
  its height is layout-driven, not content-driven.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@nishasdk nishasdk force-pushed the panel-layout-improvements branch from 01029dd to c932215 Compare May 20, 2026 12:54
@johannesjo
Copy link
Copy Markdown
Owner

johannesjo commented May 20, 2026

Thank you very much once more! There still seem to be some problems.

Multi-Agent Review — additional findings

Ran a parallel 6-agent review (correctness, security, architecture, alternatives, performance, simplicity). The four items you (@johannesjo) already flagged hold up — restating only the new findings below to avoid noise.

Critical (new)

  1. flexGrow prop is dead API / misnamed
    src/components/ResizablePanel.tsx:23-26, 134-138 — The prop reads like a CSS pass-through but is only honored on absorbers, only as a relative weight among absorbers, and ignored on every other branch. No call site sets it (the intended notesAndFilesChild usage is missing — same root as item perf: optimize xterm.js rendering for Linux #1). Either wire it in this PR (flexGrow: 0.5 on notesAndFilesChild + add notes-files to absorberIds) or delete the prop. If kept, rename to absorberWeight so callers don't expect it to bias non-absorber children.

  2. Persisted value semantics are overloaded
    src/components/ResizablePanel.tsx:113-122 vs src/store/store.ts (setPanelUserSize JSDoc says pixels). The same store key namespace now stores pixels for non-absorbers and flex-grow ratios for absorbers. Interpretation is decided at read time by current absorberIds. The whole asymmetric-pin sweep effect exists because the units are indistinguishable downstream. Either tag the persisted value ({ unit: 'px' | 'ratio', value: number }) or split into two stores (panelUserPin for pixels, panelUserWeight for ratios). Until that lands, the bug surface around absorberIds changes is large.

  3. Asymmetric-pin heuristic misfires on legitimate transitions
    src/components/ResizablePanel.tsx:71-100. The check pinnedAbsorberCount === 1 is meant to detect a stale pixel pin left over from before a child was promoted to absorber. But it also fires when absorberIds reactively grows mid-session (plan tab opens, prompt-input toggles, a third absorber appears). In that case a legitimate post-drag ratio gets wiped. Compounds with Add evaluation of Electron migration for xterm.js performance #2.

  4. Double-click race with prior mousedowns
    src/components/TilingLayout.tsx:591-605. A dblclick is preceded by two mousedowns, both of which call handleDragStart and prime dragPreview/dragging(). unpinHandle deletes the persisted sizes but leaves the drag-state signals populated until the next mouseup, so the column flashes the drag preview width. Fix: in unpinHandle, also clear dragPreview and setDragging(null), or gate the dblclick reset on dragging() === null.

Warnings (new)

  • wrapperRefs Map leak on unmountsrc/components/ResizablePanel.tsx:300-302. SolidJS ref callback isn't invoked with null on unmount, so removed children's wrapper DOM nodes stay referenced for the panel's lifetime. With churning children (plan tab, terminals, prompt input) this is the new failure mode the ID-key migration introduces. Fix: ref={(el) => { wrapperRefs.set(child.id, el); onCleanup(() => wrapperRefs.delete(child.id)); }}.
  • setDragOverride({...}) replaces whole record every movesrc/components/ResizablePanel.tsx:190-197. Every child's reactive childStyle re-evaluates on every mousemove because they all read dragOverride(). With 5 children at ~120 Hz that's ~600 style recomputes/sec, each calling keyFor/isAbsorber/getPanelUserSize. Either rAF-coalesce in onMove or store override as per-id signals so only the adjacent pair invalidate.
  • Persisted pin not validated at readsrc/components/ResizablePanel.tsx:113-130. A corrupt store value (NaN, Infinity, negative, 0) emits broken CSS or a flex ratio that starves siblings with no UI escape hatch short of clearing storage. Add Number.isFinite(pinned) && pinned > 0 clamp on read; treat anything else as undefined.
  • TaskNotesBody.tsx:73-78 comment asserts wiring that doesn't exist — Says "notes-files is a weighted absorber in the outer vertical panel"; that's the intent, but TaskPanel.tsx:420,347 doesn't add it to absorberIds and doesn't set flexGrow. The comment becomes a lie the moment this lands. Pair with the wiring or soften to "intended to be".
  • unpinHandle declared inside <For>src/components/TilingLayout.tsx:591-597. Recreated per child per render. Inline the two-line body, or hoist.

Suggestions (new)

  • Extract unpinPair(leftId, rightId, prefix)TilingLayout.tsx:591-597 and ResizablePanel.tsx:223-228 are the same 5-line shape with different prefixes. Identical user-facing behavior diverging is a guarantee the next contributor only updates one place.
  • Split flex: '${fg} 1 0' strings into 'flex-grow' / 'flex-shrink' / 'flex-basis' — Solid's style object accepts them individually and the three branches in childStyle become uniform-shaped, easier to diff/animate.
  • Follow-up: CSS Grid with fr units would natively express weighted absorbers (<fg>fr for absorbers, minmax(<px>, auto) for pinned non-absorbers). Dissolves the pixel/ratio dual meaning and the asymmetric-pin sweep entirely. Out of scope for this PR; flagging for the broader resize rework.

What I'd merge as-is

Nothing yet — confidence high on the structural items above. The smallest viable path is still your suggested scope: wire notes-files as a weighted absorber + ID-key dragOverride, plus the leak/race/comment fixes from this pass. The store-semantics rework (#2) and the prop rename (#1) can ride along or land as a follow-up — either way they should be settled before more absorber callers appear.

- Rename flexGrow -> absorberWeight on PanelChild; wire notes-files as a
  weighted absorber (absorberWeight: 0.5) in TaskPanel and add it to
  absorberIds alongside ai-terminal. Height is now layout-driven so the
  max-height: 40vh cap on TaskNotesBody is removed.
- Add onCleanup(() => wrapperRefs.delete(child.id)) to fix Map leak on
  child unmount
- Add Number.isFinite + > 0 guard on persisted pin reads so a corrupt
  store value can't emit broken CSS
- Gate double-click reset on dragging() !== null to prevent drag-preview
  flash race; inline unpinHandle body while there
- Remove unused idx param from childStyle (side effect of ID-keyed
  dragOverride from prior commit)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@nishasdk
Copy link
Copy Markdown
Contributor Author

nishasdk commented May 20, 2026

Apologies for the force pushes earlier it seems like some changes in the origin triggered an update which then lead to rebasing to stay current with upstream. I understand that rewrites the branch history and makes incremental review harder and very sorry for the confusion. All updates from here are regular commits.

Addressed everything flagged:

Blocker (from first review)

  • Wired notes-files as a proper weighted absorber (absorberWeight: 0.5) in TaskPanel and added it to absorberIds alongside ai-terminal. With that in place the max-height: 40vh cap on TaskNotesBody is removed — height is now layout-driven rather than a workaround for content bubbling up the flex tree.

From the parallel review

  • Renamed flexGrowabsorberWeight on PanelChild so it's clear the prop only applies as a relative weight among absorbers and isn't a CSS pass-through
  • Added onCleanup(() => wrapperRefs.delete(child.id)) in the ref callback to fix the Map leak on unmount
  • Added Number.isFinite(raw) && raw > 0 guard on persisted pin reads so a corrupt store value can't emit broken CSS
  • Gated the double-click reset on dragging() !== null to avoid the drag-preview flash race; inlined the function body while there
  • dragOverride is now ID-keyed (from the first review pass) — removed the now-unused idx parameter from childStyle as a side effect

Left out of scope (as suggested)

  • 3+ absorber ratio vector — current concrete need is still 2
  • Asymmetric-pin heuristic misfire on reactive absorberIds changes — flagged, will track separately
  • Store semantics overload (pixels vs ratios in same namespace) — agreed this needs a proper fix, leaving for a follow-up before more absorber callers appear
  • setDragOverride recompute performance — not a blocker at current panel counts

@johannesjo
Copy link
Copy Markdown
Owner

Thank you very much!

@johannesjo johannesjo merged commit 6bc4b7d into johannesjo:main May 20, 2026
1 check passed
@nishasdk nishasdk deleted the panel-layout-improvements branch May 20, 2026 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants