From fa56a2b39db97fb7ff17ddb93a2906dbd16bc66b Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 26 Apr 2026 22:03:03 +0200 Subject: [PATCH] Fix new agent placement overlay --- src/renderer/src/app/App.tsx | 5 - .../workspace/commands/paneCommands.ts | 2 +- .../workspace/lib/newAgentPlacement.ts | 116 +++++------------- .../workspace/ui/NewAgentPlacementOverlay.tsx | 97 ++++++++------- .../src/workspace/hook/actions/dispatch.ts | 7 +- .../src/workspace/hook/actions/pane.ts | 14 ++- src/renderer/src/workspace/hook/index.ts | 1 + 7 files changed, 100 insertions(+), 142 deletions(-) diff --git a/src/renderer/src/app/App.tsx b/src/renderer/src/app/App.tsx index 2fc136177..e870f3795 100644 --- a/src/renderer/src/app/App.tsx +++ b/src/renderer/src/app/App.tsx @@ -257,11 +257,6 @@ export default function App() { showStatusMode={settings.showStatusMode} showWorktreeBadges={settings.showWorktreeBadges} /> - ) : activeTab ? (
diff --git a/src/renderer/src/features/workspace/commands/paneCommands.ts b/src/renderer/src/features/workspace/commands/paneCommands.ts index 47cbdeae4..58fcbe2ce 100644 --- a/src/renderer/src/features/workspace/commands/paneCommands.ts +++ b/src/renderer/src/features/workspace/commands/paneCommands.ts @@ -6,7 +6,7 @@ export const paneCommands: CommandDef[] = [ id: 'new-agent', title: 'New Agent…', keywords: ['new', 'agent', 'placement', 'claude', 'codex', 'terminal'], - when: ({ workspace }) => Boolean(workspace.activeTab && !workspace.tileTabs), + when: ({ workspace }) => Boolean(workspace.activeTab && !workspace.tileTabs && !workspace.dispatchMode), run: ({ workspace }) => workspace.startNewAgentPlacement(), }, { diff --git a/src/renderer/src/features/workspace/lib/newAgentPlacement.ts b/src/renderer/src/features/workspace/lib/newAgentPlacement.ts index c77275aba..63e8a6ce8 100644 --- a/src/renderer/src/features/workspace/lib/newAgentPlacement.ts +++ b/src/renderer/src/features/workspace/lib/newAgentPlacement.ts @@ -22,18 +22,6 @@ export type PlacementTarget = scope: 'global' } -type Center = { - x: number - y: number -} - -function centerOf(rect: Rect): Center { - return { - x: rect.x + rect.width / 2, - y: rect.y + rect.height / 2, - } -} - export function buildPlacementTargets( root: TileNode, anchorSessionId: SessionId, @@ -122,14 +110,15 @@ export function buildPlacementTargets( }, ] - // WHY generate explicit placement targets instead of inferring from - // arrow "intent": + // WHY keep explicit placement target records instead of deriving the + // commit operation directly from the key press: // - // The old model guessed from the focused pane alone, which breaks down as - // soon as the user wants to insert relative to the whole layout. Example: - // with two stacked rows, "new bottom row" should wrap the entire root, not - // try to synthesize something from the focused leaf. These targets are real - // layout operations, so the preview and commit path can stay identical. + // The placement UI shows ONE preview at a time now, but the selected + // preview still has to carry the real layout operation to commit. A + // plain arrow maps to a split-leaf target; Shift+arrow maps to a + // wrap-root target. Keeping those as addressable records means preview + // geometry and commit payload stay identical, instead of rebuilding + // "left/right/top/bottom" meaning in two different places. return dedupeTargets(targets) } @@ -146,11 +135,11 @@ function dedupeTargets(targets: PlacementTarget[]): PlacementTarget[] { // care about; dropping the rounding collision trap also drops the // silent "which sibling wins?" non-determinism. // - // We still keep a dedupe pass (rather than trusting callers) because - // wrap-root and split-leaf can generate the same id in edge cases - // where the anchor IS the root leaf and "left of focused pane" is - // visually equivalent to "new left column". Same-id targets are - // genuinely redundant and the first one wins. + // In the one-leaf case, global and local placements can be visually + // identical, but their ids remain intentionally different because + // they represent different commands from the UI's perspective + // (Shift+arrow vs plain arrow). The pass remains a cheap guard + // against accidental duplicate records from future target builders. const seen = new Set() const out: PlacementTarget[] = [] for (const target of targets) { @@ -177,70 +166,23 @@ export function defaultPlacementTargetId( ) } -export function findNearestPlacementTarget( +export function placementTargetIdForArrow( targets: PlacementTarget[], - selectedId: string | null, - direction: 'left' | 'right' | 'up' | 'down', + anchorSessionId: SessionId, + arrow: 'left' | 'right' | 'up' | 'down', + scope: 'local' | 'global', ): string | null { - const current = - targets.find(target => target.id === selectedId) ?? - targets[0] - if (!current) return null - - const currentCenter = centerOf(current.rect) - const candidates = targets - .filter(target => target.id !== current.id) - .map(target => ({ - target, - center: centerOf(target.rect), - })) - .filter(candidate => { - if (direction === 'left') return candidate.center.x < currentCenter.x - 1 - if (direction === 'right') return candidate.center.x > currentCenter.x + 1 - if (direction === 'up') return candidate.center.y < currentCenter.y - 1 - return candidate.center.y > currentCenter.y + 1 - }) - - if (candidates.length === 0) return current.id - - const ranked = candidates.sort((a, b) => { - const primaryA = primaryAxisDistance(currentCenter, a.center, direction) - const primaryB = primaryAxisDistance(currentCenter, b.center, direction) - if (primaryA !== primaryB) return primaryA - primaryB - - const crossA = crossAxisDistance(currentCenter, a.center, direction) - const crossB = crossAxisDistance(currentCenter, b.center, direction) - if (crossA !== crossB) return crossA - crossB + const direction: SplitDirection = + arrow === 'left' || arrow === 'right' ? 'vertical' : 'horizontal' + const side: 'a' | 'b' = + arrow === 'left' || arrow === 'up' ? 'a' : 'b' - return euclideanDistance(currentCenter, a.center) - euclideanDistance(currentCenter, b.center) - }) - - return ranked[0]?.target.id ?? current.id -} - -function primaryAxisDistance( - from: Center, - to: Center, - direction: 'left' | 'right' | 'up' | 'down', -): number { - if (direction === 'left') return from.x - to.x - if (direction === 'right') return to.x - from.x - if (direction === 'up') return from.y - to.y - return to.y - from.y -} - -function crossAxisDistance( - from: Center, - to: Center, - direction: 'left' | 'right' | 'up' | 'down', -): number { - return direction === 'left' || direction === 'right' - ? Math.abs(to.y - from.y) - : Math.abs(to.x - from.x) -} - -function euclideanDistance(a: Center, b: Center): number { - const dx = a.x - b.x - const dy = a.y - b.y - return Math.hypot(dx, dy) + return ( + targets.find(target => { + if (target.direction !== direction || target.side !== side) return false + if (scope === 'global') return target.kind === 'wrap-root' + return target.kind === 'split-leaf' && target.targetSessionId === anchorSessionId + })?.id ?? + defaultPlacementTargetId(targets, anchorSessionId) + ) } diff --git a/src/renderer/src/features/workspace/ui/NewAgentPlacementOverlay.tsx b/src/renderer/src/features/workspace/ui/NewAgentPlacementOverlay.tsx index da769d015..09d248d17 100644 --- a/src/renderer/src/features/workspace/ui/NewAgentPlacementOverlay.tsx +++ b/src/renderer/src/features/workspace/ui/NewAgentPlacementOverlay.tsx @@ -3,7 +3,7 @@ import { useEffect, useMemo, useRef, useState } from 'react' import { buildPlacementTargets, defaultPlacementTargetId, - findNearestPlacementTarget, + placementTargetIdForArrow, type PlacementTarget, } from '@renderer/features/workspace/lib/newAgentPlacement' import type { SessionKind } from '@renderer/workspace/types' @@ -21,6 +21,13 @@ const KIND_OPTIONS: Array<{ kind: SessionKind; label: string; description: strin { kind: 'terminal', label: 'Terminal', description: 'plain shell pane' }, ] +const ARROW_TO_DIRECTION = { + ArrowLeft: 'left', + ArrowRight: 'right', + ArrowUp: 'up', + ArrowDown: 'down', +} as const + export function NewAgentPlacementOverlay({ open, workspace, onClose }: Props) { const overlayRef = useRef(null) const [selectedIndex, setSelectedIndex] = useState(0) @@ -141,24 +148,32 @@ export function NewAgentPlacementOverlay({ open, workspace, onClose }: Props) { } return } - if (event.key === 'ArrowUp') { - event.preventDefault() - setSelectedTargetId(prev => findNearestPlacementTarget(placementTargets, prev, 'up')) - return - } - if (event.key === 'ArrowDown') { + if ( + event.key === 'ArrowLeft' || + event.key === 'ArrowRight' || + event.key === 'ArrowUp' || + event.key === 'ArrowDown' + ) { event.preventDefault() - setSelectedTargetId(prev => findNearestPlacementTarget(placementTargets, prev, 'down')) - return - } - if (event.key === 'ArrowLeft') { - event.preventDefault() - setSelectedTargetId(prev => findNearestPlacementTarget(placementTargets, prev, 'left')) - return - } - if (event.key === 'ArrowRight') { - event.preventDefault() - setSelectedTargetId(prev => findNearestPlacementTarget(placementTargets, prev, 'right')) + // WHY direct arrow mapping instead of nearest-rectangle navigation: + // + // The target set intentionally contains two different operations: + // split the focused pane, or wrap the whole root. Rendering every + // target as a clickable rectangle made those operations overlap, and + // center-distance navigation could jump from a local split to an + // unrelated outer row because a large half-screen target happened to + // be closer. Plain arrows now mean "place relative to the focused + // pane"; Shift+arrow means "place relative to the whole tab." That + // keeps the operations explicit and makes the preview the only visual + // source of truth. + const arrow = ARROW_TO_DIRECTION[event.key] + const scope = event.shiftKey ? 'global' : 'local' + setSelectedTargetId(placementTargetIdForArrow( + placementTargets, + anchorSessionId, + arrow, + scope, + )) return } if (event.key === 'Enter' && placementTarget) { @@ -196,32 +211,24 @@ export function NewAgentPlacementOverlay({ open, workspace, onClose }: Props) { ref={overlayRef} className="absolute inset-0 z-40 bg-black/20" > - {selectedKind && placementTargets.map(target => { - const active = target.id === placementTarget?.id - return ( - - ) - })} + {selectedKind && placementTarget && ( +
+ )}
@@ -231,7 +238,7 @@ export function NewAgentPlacementOverlay({ open, workspace, onClose }: Props) {
{KIND_OPTIONS.find(option => option.kind === selectedKind)?.label} placement
- Arrow keys move between real layout targets. Enter confirms. Backspace resets. + Arrows split the focused pane. Shift+arrows add an outer row or column.
Target: {placementTarget?.label ?? 'none'} diff --git a/src/renderer/src/workspace/hook/actions/dispatch.ts b/src/renderer/src/workspace/hook/actions/dispatch.ts index 9b255f483..a33583da7 100644 --- a/src/renderer/src/workspace/hook/actions/dispatch.ts +++ b/src/renderer/src/workspace/hook/actions/dispatch.ts @@ -15,6 +15,7 @@ export function useDispatchActions( setTileTabs: WorkspaceSetTileTabs, refs: WorkspaceRefs, showToast: (message: string, durationMs?: number) => void, + closeNewAgentPlacement: () => void, sessionActions: SessionActions, ): { enterDispatchMode: (scope?: DispatchModeState['scope']) => Promise @@ -107,6 +108,7 @@ export function useDispatchActions( const enterDispatchMode = useCallback( async (scope: DispatchModeState['scope'] = state.dispatchMode?.scope ?? 'project') => { + closeNewAgentPlacement() setState(prev => ({ ...prev, dispatchMode: { @@ -117,7 +119,7 @@ export function useDispatchActions( setTileTabs(null) await ensureDispatchTerminal() }, - [ensureDispatchTerminal, setState, setTileTabs, state.dispatchMode?.scope], + [closeNewAgentPlacement, ensureDispatchTerminal, setState, setTileTabs, state.dispatchMode?.scope], ) const exitDispatchMode = useCallback(() => { @@ -129,6 +131,7 @@ export function useDispatchActions( const setDispatchScope = useCallback( async (scope: DispatchModeState['scope']) => { + closeNewAgentPlacement() setState(prev => ({ ...prev, dispatchMode: { @@ -138,7 +141,7 @@ export function useDispatchActions( })) await ensureDispatchTerminal() }, - [ensureDispatchTerminal, setState], + [closeNewAgentPlacement, ensureDispatchTerminal, setState], ) const toggleDispatchTerminal = useCallback(async () => { diff --git a/src/renderer/src/workspace/hook/actions/pane.ts b/src/renderer/src/workspace/hook/actions/pane.ts index f492acd9f..89059300f 100644 --- a/src/renderer/src/workspace/hook/actions/pane.ts +++ b/src/renderer/src/workspace/hook/actions/pane.ts @@ -2,6 +2,7 @@ import { useCallback } from 'react' import type { BuriedPaneRecord, + DispatchModeState, SessionId, SessionKind, SessionMeta, @@ -40,7 +41,12 @@ import type { SessionActions } from '@renderer/workspace/hook/actions/session' // ----------------------------------------------------------------------------- export function usePaneActions( - state: { activeTabId: string; sessions: Record; tabs: Tab[] }, + state: { + activeTabId: string + dispatchMode: DispatchModeState | null + sessions: Record + tabs: Tab[] + }, setState: WorkspaceSetState, setRuntimes: WorkspaceSetRuntimes, setSpotlight: WorkspaceSetSpotlight, @@ -110,8 +116,12 @@ export function usePaneActions( const startNewAgentPlacement = useCallback(() => { const tab = state.tabs.find(t => t.id === state.activeTabId) if (!tab) return + if (state.dispatchMode) { + showToast('New Agent placement is only available in the tiled workspace') + return + } openNewAgentPlacement() - }, [openNewAgentPlacement, state.activeTabId, state.tabs]) + }, [openNewAgentPlacement, showToast, state.activeTabId, state.dispatchMode, state.tabs]) const commitNewAgentPlacement = useCallback( async (kind: SessionKind, target: PlacementTarget) => { diff --git a/src/renderer/src/workspace/hook/index.ts b/src/renderer/src/workspace/hook/index.ts index 368b96c6b..2f9b781e1 100644 --- a/src/renderer/src/workspace/hook/index.ts +++ b/src/renderer/src/workspace/hook/index.ts @@ -236,6 +236,7 @@ export function useWorkspace( setTileTabs, refs, showToast, + closeNewAgentPlacement, sessionActions, )