Skip to content

Commit 6bc4b7d

Browse files
nishasdkclaude
authored andcommitted
fix(panels): address parallel review feedback
- 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>
1 parent 6adb56e commit 6bc4b7d

4 files changed

Lines changed: 28 additions & 27 deletions

File tree

src/components/ResizablePanel.tsx

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { batch, createEffect, createMemo, createSignal, For, type JSX } from 'solid-js';
1+
import { batch, createEffect, createMemo, createSignal, For, onCleanup, type JSX } from 'solid-js';
22
import { getPanelUserSize, setPanelUserSize, deletePanelUserSize } from '../store/store';
33

44
export interface PanelChild {
@@ -20,10 +20,10 @@ export interface PanelChild {
2020
* smaller than their pinned size would be (e.g., a toolbar that collapses
2121
* when its body is empty), so a stale pin can't leave a visible gap. */
2222
noPin?: () => boolean;
23-
/** Flex-grow weight for absorbers. Defaults to 1 (equal share). Use a
24-
* smaller value to give an absorber less space than others, e.g. 0.5
25-
* for notes-files so the AI terminal gets roughly 2/3 of remaining space. */
26-
flexGrow?: number;
23+
/** Relative flex-grow weight among absorbers. Defaults to 1 (equal share).
24+
* Ignored on non-absorbers. Use a smaller value to give an absorber less
25+
* space than its siblings, e.g. 0.5 so the AI terminal gets ~2/3. */
26+
absorberWeight?: number;
2727
}
2828

2929
interface ResizablePanelProps {
@@ -43,7 +43,7 @@ interface ResizablePanelProps {
4343

4444
export function ResizablePanel(props: ResizablePanelProps) {
4545
const [draggingIdx, setDraggingIdx] = createSignal<number | null>(null);
46-
const [dragOverride, setDragOverride] = createSignal<Record<number, number>>({});
46+
const [dragOverride, setDragOverride] = createSignal<Record<string, number>>({});
4747
/** Stable per-ID refs so drag measurement survives dynamic children changes. */
4848
const wrapperRefs = new Map<string, HTMLDivElement>();
4949

@@ -99,14 +99,15 @@ export function ResizablePanel(props: ResizablePanelProps) {
9999
if (stale.length > 0) deletePanelUserSize(stale);
100100
});
101101

102-
function childStyle(child: PanelChild, idx: number): JSX.CSSProperties {
102+
function childStyle(child: PanelChild): JSX.CSSProperties {
103103
const noPin = child.noPin?.() === true;
104104
// noPin children can't be sized by drag override or persisted pin — they
105105
// stay content-sized so an empty inner state can't leave a visible gap.
106106
const key = keyFor(child.id);
107-
const pinned = noPin
107+
const raw = noPin
108108
? undefined
109-
: (dragOverride()[idx] ?? (key ? getPanelUserSize(key) : undefined));
109+
: (dragOverride()[child.id] ?? (key ? getPanelUserSize(key) : undefined));
110+
const pinned = raw !== undefined && Number.isFinite(raw) && raw > 0 ? raw : undefined;
110111
const dim = isHorizontal() ? 'width' : 'height';
111112
const minDim = isHorizontal() ? 'min-width' : 'min-height';
112113
const maxDim = isHorizontal() ? 'max-width' : 'max-height';
@@ -117,7 +118,7 @@ export function ResizablePanel(props: ResizablePanelProps) {
117118
// Persisted pins on absorbers use flex-grow ratio so they scale when the
118119
// container resizes (e.g. notes/changed-files in a horizontal split that
119120
// should stay proportional with the window).
120-
if (isAbsorber(child.id) && dragOverride()[idx] === undefined) {
121+
if (isAbsorber(child.id) && dragOverride()[child.id] === undefined) {
121122
return {
122123
flex: `${pinned} 1 0`,
123124
[minDim]: `${min}px`,
@@ -132,7 +133,7 @@ export function ResizablePanel(props: ResizablePanelProps) {
132133
};
133134
}
134135
if (isAbsorber(child.id)) {
135-
const fg = child.flexGrow ?? 1;
136+
const fg = child.absorberWeight ?? 1;
136137
return {
137138
flex: `${fg} 1 0`,
138139
[minDim]: `${min}px`,
@@ -222,12 +223,12 @@ export function ResizablePanel(props: ResizablePanelProps) {
222223
// Skip the override on noPin children so they stay content-sized; also
223224
// skip on a sole absorber adjacent to a noPin sibling, since pinning the
224225
// absorber temporarily would steal the space the noPin child can't take.
225-
const override: Record<number, number> = {};
226+
const override: Record<string, number> = {};
226227
if (!leftNoPin && !(leftIsAbs && rightNoPin && soleAbs)) {
227-
override[handleIdx] = latestLeft;
228+
override[leftChild.id] = latestLeft;
228229
}
229230
if (!rightNoPin && !(rightIsAbs && leftNoPin && soleAbs)) {
230-
override[handleIdx + 1] = latestRight;
231+
override[rightChild.id] = latestRight;
231232
}
232233
setDragOverride(override);
233234
};
@@ -300,8 +301,9 @@ export function ResizablePanel(props: ResizablePanelProps) {
300301
class="rp-cell"
301302
ref={(el) => {
302303
wrapperRefs.set(child.id, el);
304+
onCleanup(() => wrapperRefs.delete(child.id));
303305
}}
304-
style={childStyle(child, i())}
306+
style={childStyle(child)}
305307
>
306308
{child.content()}
307309
</div>

src/components/TaskNotesBody.tsx

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,6 @@ export function TaskNotesBody(props: TaskNotesBodyProps) {
7272
});
7373
});
7474

75-
// notes-files is a weighted absorber in the outer vertical panel, so its
76-
// height is layout-driven (flex ratio), not content-driven. min-height is
77-
// the floor used when the panel is fresh or unpinned.
7875
const intrinsicHeight = () => (store.focusMode ? '240px' : '140px');
7976

8077
return (

src/components/TaskPanel.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ export function TaskPanel(props: TaskPanelProps) {
347347
const notesAndFilesChild: PanelChild = {
348348
id: 'notes-files',
349349
minSize: 60,
350+
absorberWeight: 0.5,
350351
content: () => (
351352
<div style={{ height: '100%', 'min-height': '200px' }}>
352353
{isNoneGit() ? (
@@ -417,7 +418,7 @@ export function TaskPanel(props: TaskPanelProps) {
417418
<ResizablePanel
418419
direction="vertical"
419420
persistKey={`task:${props.task.id}`}
420-
absorberIds={['ai-terminal']}
421+
absorberIds={['notes-files', 'ai-terminal']}
421422
children={[
422423
notesAndFilesChild,
423424
shellSectionChild,

src/components/TilingLayout.tsx

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -588,21 +588,22 @@ export function TilingLayout() {
588588
});
589589
const showHandle = () =>
590590
!store.focusMode && !child.fixed && i() < panelChildren().length - 1;
591-
function unpinHandle() {
592-
const panels = panelChildren();
593-
const left = panels[i()];
594-
const right = panels[i() + 1];
595-
if (!left || !right) return;
596-
deletePanelUserSize([`tiling:${left.id}`, `tiling:${right.id}`]);
597-
}
598591
return (
599592
<>
600593
<div style={wrapperStyle()}>{child.content()}</div>
601594
<Show when={showHandle()}>
602595
<div
603596
class={`resize-handle resize-handle-h ${dragging() === i() ? 'dragging' : ''}`}
604597
onMouseDown={(e) => handleDragStart(i(), e)}
605-
onDblClick={unpinHandle}
598+
onDblClick={() => {
599+
if (dragging() !== null) return;
600+
const panels = panelChildren();
601+
const left = panels[i()];
602+
const right = panels[i() + 1];
603+
if (!left || !right) return;
604+
deletePanelUserSize([`tiling:${left.id}`, `tiling:${right.id}`]);
605+
requestAnimationFrame(() => updateViewportState());
606+
}}
606607
/>
607608
</Show>
608609
</>

0 commit comments

Comments
 (0)