feat(layout): add flex-stretch panels and focus mode#22
feat(layout): add flex-stretch panels and focus mode#22linniksa wants to merge 1 commit intojohannesjo:mainfrom
Conversation
a068420 to
bdd5d95
Compare
Task panels now stretch to fill available width using flex-grow instead of fixed pixel sizes. Added a focus mode toggle button in the task title bar that shows only the active task at full width. In focus mode, hidden panels use visibility:hidden (not display:none) so terminals keep their correct dimensions. Focus mode state is persisted across app restarts.
bdd5d95 to
f5e27e1
Compare
There was a problem hiding this comment.
Thank you very much for this and sorry for the late reply. Somehow I wasn't receiving notifications for PRs opened here....
Code Review
Nice implementation — the visibility: hidden + position: absolute approach for keeping terminals alive with correct dimensions is well-chosen, and the persistence/store changes follow existing patterns cleanly.
Must fix
focusMode missing from persistedSnapshot() in autosave.ts
saveState() in persistence.ts includes focusMode, but persistedSnapshot() in autosave.ts does not. This means toggling focus mode won't trigger an autosave. If the app crashes before another field changes, the state is lost. Add focusMode: store.focusMode to the snapshot.
Worth addressing
scrollIntoView fires in focus mode
The createEffect at TilingLayout.tsx:26 still calls scrollIntoView in focus mode. With panels stacked absolutely this is meaningless and could cause scroll jumps. Consider guarding:
if (!activeId || !containerRef || store.focusMode) return;minSize on PanelChild is unused
minSize: 520 on the cached PanelChild (line ~52) is no longer consumed since ResizablePanel was removed. The actual enforcement is the CSS min-width: 520px in the flex wrapper. Having "520" in two places is a maintenance risk — consider removing minSize from the cache or adding a shared constant.
Ctrl+Shift+Wheel resize silently removed
The createCtrlShiftWheelResizeHandler integration was removed but not mentioned in the PR description. This is a user-facing feature regression. Also, createCtrlShiftWheelResizeHandler in wheelZoom.ts becomes dead code (no other consumers). If the removal is intentional, note it in the description and consider cleaning up the dead export.
Nice to have
Keyboard shortcut for focus mode — the app has extensive keyboard navigation but focus mode is only togglable via the icon button.
Auto-exit focus mode with 1 task — when tasks are deleted down to 1, focus mode provides no benefit. Consider auto-disabling when taskOrder.length <= 1.
johannesjo
left a comment
There was a problem hiding this comment.
Code Review: feat(layout): add flex-stretch panels and focus mode
Summary
Replaces ResizablePanel with a plain CSS flexbox layout (flex: 1 1 0px per task panel) and adds a focus mode that shows only the active task at full width while keeping background terminals alive via visibility: hidden + absolute positioning. Focus mode state is persisted across restarts. The UX concept is solid — focus mode is a frequently requested pattern for multi-task workflows.
Issue 1: Silent removal of Ctrl+Shift+Wheel resize feature
The PR removes createCtrlShiftWheelResizeHandler and the ResizablePanelHandle integration entirely. This deletes an existing user-facing feature (column resize via Ctrl+Shift+scroll wheel) with no mention in the PR description, no replacement, and no migration path.
The ResizablePanel component import is also downgraded to type-only, meaning the component is no longer rendered. While the flex layout is a valid alternative for equal-width distribution, the ability to manually resize individual panels is lost.
Action needed: Either:
- Acknowledge the removal and explain why flex-only is preferred, or
- Provide an alternative resize mechanism (e.g., CSS resize handles, or keep
ResizablePanelas an option when focus mode is off)
Issue 2: Potential SolidJS reactivity gap in the <For> render function
Inside the <For> callback, store.focusMode is read directly in the ternary that decides which style object to apply:
...(store.focusMode && !isPlaceholder
? { position: hidden() ? 'absolute' : 'relative', ... }
: { flex: isPlaceholder ? '0 0 54px' : '1 1 0px', ... })In SolidJS, <For> does not re-run its callback when store values change — it only re-runs when the array items change. The hidden() and hidePlaceholder() helper functions are proper reactive accessors (they call store.focusMode inside a function), so their usages inside style attribute bindings will update reactively. However, the top-level ternary that switches between two entirely different style object shapes reads store.focusMode outside any signal accessor.
This means toggling focus mode may not switch the style object shape (absolute positioning vs. flex) for existing <For> nodes, since the ternary branch is evaluated once when the node is created.
Recommended fix: Wrap the entire style computation in an accessor:
<div
style={(() => {
const isHidden = store.focusMode && !isPlaceholder && child.id !== store.activeTaskId;
const isHiddenPlaceholder = store.focusMode && isPlaceholder;
return store.focusMode && !isPlaceholder
? {
position: isHidden ? 'absolute' : 'relative',
inset: '0',
width: '100%',
height: '100%',
visibility: isHidden ? 'hidden' : undefined,
'pointer-events': isHidden ? 'none' : undefined,
overflow: 'hidden',
}
: {
flex: isPlaceholder ? '0 0 54px' : '1 1 0px',
'min-width': isPlaceholder ? undefined : '520px',
height: '100%',
display: isHiddenPlaceholder ? 'none' : undefined,
overflow: 'hidden',
};
})()}
/>Or extract it into a createMemo per child to make the reactivity explicit and debuggable.
Issue 3: initialSize: 520 is now dead config
PanelChild still has initialSize: 520 set in the cache, but this was a ResizablePanel-specific property. Since ResizablePanel is no longer used, initialSize and minSize on PanelChild are dead properties. They should either be removed from the cache setup or the PanelChild type should be updated to reflect the new layout model.
Issue 4: Focus button placement and UX
The focus button is added to every task panel's title bar, which is correct. A few minor UX observations:
- No keyboard shortcut for toggling focus mode. Given the app's keyboard-first design, a shortcut (e.g.,
Cmd+Shift+For similar) would be a natural addition. - When entering focus mode,
setActiveTask(props.task.id)is called first, which is the right behavior. But when exiting focus mode, the user returns to the flex layout with the previously focused task still active — this is fine, but the scroll-into-view effect should ensure that task is visible.
Minor Items
- The
markDirtyimport andcreateEffectfor terminal re-fit on task switch is well done — this is a necessary step sincevisibility: hiddenelements don't trigger resize observers. minSizechanged from 300 to 520 — this is a meaningful UX change (panels can no longer be narrower than 520px even in non-focus mode). Worth mentioning in the PR description.- Persistence of
focusModeis clean and follows existing patterns (=== trueguard on load,|| undefinedon save).
Overall
The focus mode concept is great and the visibility: hidden approach for keeping terminals alive is the right technique. However, the silent removal of manual panel resizing is a significant UX regression that should be acknowledged, and the reactivity concern in the style ternary needs verification/fix to ensure focus mode toggling works correctly for existing panels. Please address these two items and this will be in good shape.
Generated by Claude Code
|
Thank you very much! <3 |
johannesjo
left a comment
There was a problem hiding this comment.
No new commits have been pushed since the CHANGES_REQUESTED review on 2026-04-12. Both blocking issues remain open:
- SolidJS reactivity gap — inside the
<For>callback,store.focusModeis read in a top-level ternary (not a reactive accessor). Toggling focus mode will not update the style shape (absolute vs. flex) for existing<For>nodes. - Silent removal of Ctrl+Shift+Wheel resize — a user-facing feature was removed with no replacement and no mention in the PR description.
Please address both before requesting re-review.
Summary
Task panels now stretch to fill available width using flex-grow instead of fixed pixel sizes. Added a focus mode toggle button in the task title bar that shows only the active task at full width.
Description
Flex-stretch layout (default)
Panels automatically distribute available space — 1 task fills the entire width, 2 tasks split 50/50, etc. Minimum width
(520px) is preserved; horizontal scroll appears when needed.
Minimum width is still enforced when there are many tasks:
Focus mode
A new focus button in the task title bar shows only the active task at full width. Other tasks remain alive in the background (terminals keep running). Switching tasks via sidebar or keyboard shortcuts works seamlessly while in focus mode.
Check List
testing performed
applicable. — N/A, internal UI feature