From 8a78bff90932c183b7d532796a8408d137ede1e8 Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Thu, 3 Sep 2026 14:20:27 +0200 Subject: [PATCH 1/8] fix(ui): compact and animate shortcut pills Issue: Shortcut pills for directory and pull-request actions remained visible while the focused terminal ran a foreground process, even though their commands could not reach the shell. Fixed pill positions also left large gaps when optional pills disappeared. Solution: Make the Cmd+P and Cmd+O pills follow the foreground-process visibility rule already used by Cmd+T, and close their overlays when the terminal becomes busy. Let PillGroupComponent pack available pills right-aligned with fixed spacing and synchronize overlay geometry with eased 200 ms membership and position transitions. Add behavior and layout tests plus documentation. --- README.md | 5 +- docs/ARCHITECTURE.md | 16 +- src/main.zig | 1 + src/ui/components/expanding_overlay.zig | 26 ++- src/ui/components/pill_group.zig | 199 ++++++++++++++++++- src/ui/components/pr_dropdown.zig | 25 ++- src/ui/components/recent_folders_overlay.zig | 33 ++- src/ui/components/worktree_overlay.zig | 20 +- 8 files changed, 309 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 476b13ea..52ab05fe 100644 --- a/README.md +++ b/README.md @@ -33,8 +33,8 @@ Architect solves this with a grid view that keeps all your agents visible, with - **Dynamic grid** — starts with a single terminal in full view; press ⌘N to add a terminal after the current one, and closing terminals compacts the grid forward - **Grid view** — keep all agents visible simultaneously, expand any one to full screen - **Worktree picker** (⌘T) — quickly `cd` into git worktrees for parallel agent work on separate branches; new worktrees are created outside the repo tree (configurable via `[worktree]` in `config.toml`) with automatic post-create initialization -- **Recent folders** (⌘O) — quickly `cd` into recently visited directories with instant search filtering (start typing to narrow the list — the overlay takes the keyboard the instant it starts opening, so nothing you type is lost to the animation), substring highlighting, arrow key navigation, and ⌘1–⌘9 quick selection -- **Pull requests** (⌘P) — when the focused terminal is inside a GitHub-cloned repo, the ⌘P pill loads and shows the current branch's PR number (e.g. `#123`), scaled to fit the pill; expand it to list open PRs (via `gh pr list`) with search filtering and check out a branch with `gh pr checkout` +- **Recent folders** (⌘O) — quickly `cd` into recently visited directories with instant search filtering (start typing to narrow the list — the overlay takes the keyboard the instant it starts opening, so nothing you type is lost to the animation), substring highlighting, arrow key navigation, and ⌘1–⌘9 quick selection; its pill is hidden while the focused terminal runs a foreground process +- **Pull requests** (⌘P) — when the focused terminal is inside a GitHub-cloned repo, the ⌘P pill loads and shows the current branch's PR number (e.g. `#123`), scaled to fit the pill; expand it to list open PRs (via `gh pr list`) with search filtering and check out a branch with `gh pr checkout`; the pill is hidden while the focused terminal runs a foreground process - **Diff review comments** — click diff lines in the ⌘D overlay to leave inline comments with multiline wrapping, then send them all to a running agent (or start one) with the "Send to agent" button - **Selection-to-agent context** — release after selecting terminal text to reveal a narrow robot button anchored just outside the selection; the button follows the selection while scrolling; choose Claude, Codex, or Gemini in a centered modal with highlighted dropdown navigation, review the fully wrapped context in a scrollable field, enter wrapped multiline instructions, cancel if needed, or launch a new agent in the selected terminal's working directory with the selection submitted as its initial prompt without embedding large contexts in the shell command line - **Story viewer** — run `architect story ` to open a scrollable overlay that renders PR story files with prose text and diff-colored code blocks @@ -43,6 +43,7 @@ Architect solves this with a grid view that keeps all your agents visible, with ### Terminal Essentials - Smooth animated transitions for grid expansion, contraction, and reflow (cells and borders move/resize together) +- Context-aware shortcut pills stay right-aligned with fixed spacing, compact when a pill is unavailable, and animate into their new positions; the worktree, recent-folder, and pull-request pills are hidden while the focused terminal runs a foreground process - Wakeable idle input handling keeps typing responsive after short idle periods instead of waiting on a fixed sleep window - Keyboard navigation: ⌘+Return to expand, ⌘1–⌘0 to switch grid slots, ⌘Arrow to move focus in grid view (plays a brief wave animation on the destination terminal), ⌘N to add, ⌘W to close a terminal (restarts if it's the only terminal), ⌘T for worktrees, ⌘O for recent folders, ⌘D for repo-wide git diff (staged + unstaged + untracked), ⌘R for reader mode, ⌘/ for shortcuts; quit with ⌘Q or the window close button - Every text field (recent-folder and reader/story search, worktree name, diff comments, selection-agent instructions) behaves like a macOS text field: a blinking caret, Backspace deletes one character, ⌥Backspace the previous word, ⌘Backspace the whole field, ⌘A selects everything (the next keystroke or paste replaces it), and ⌘C/⌘V copy and paste. The selection-agent field accepts multiline instructions with ⇧↩; plain ↩ launches, while Escape or Cancel dismisses it. Pasting multi-line text into a one-line field drops the newlines, and text longer than the field fades out at its edge instead of spilling past it. Emoji are scaled to the surrounding line height wherever they appear (search fields, comments, reader mode, stories, selection-agent instructions) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 5c83f366..f03f3188 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -345,6 +345,19 @@ On Enter / click: runtime.zig dispatch: send `gh pr checkout \n` to the focused shell ``` +### Top-Right Pill Layout + +`PillGroupComponent` owns the top-right shortcut row for help, recent folders, +worktrees, and pull requests. Each overlay reports whether its pill is +currently available; the group packs available pills from right to left with a +fixed gap and applies the resulting x-coordinate to the shared +`ExpandingOverlay` geometry. When availability or the window geometry changes, +the existing pills interpolate to their new positions with the shared cubic +easing function. The group also requests frames for the short layout animation, +so the transition is visible under idle throttling. The worktree, recent-folder, +and pull-request pills are unavailable while the focused terminal has a +foreground process, because their actions send commands to that shell. + ### External Notification Path ``` @@ -482,7 +495,8 @@ Rotate: rename active file to architect-.log and continue in new | `ui/components/modal_frame.zig` | Shared chrome for centered modal dialogs: full-window darkening scrim + rounded filled/bordered panel, and the Escape/⌘W dismiss-key check. Used by `confirm_dialog.zig` and `selection_agent_overlay.zig` so their scrim/panel rendering and dismissal keys can't drift independently | `renderScrimAndPanel()`, `isDismissKey()` | `gfx/primitives`, `geom`, `c` | | `ui/components/dropdown_menu.zig` | Reusable vertical list menu: owns open/hover/keyboard-nav state and the committed `selected` index, renders its own cached item-label textures, and reports a `.selected`/`.closed` event on click or Enter/Escape so the owning component reacts (persist the pick, or act on it immediately) instead of tracking hit-testing and highlight rendering itself. Used by `selection_agent_overlay.zig`'s agent selector and `diff_overlay.zig`'s "Send to agent" menu | `DropdownMenu`, `openMenu()`, `close()`, `handleKey()`, `handleClick()`, `handleMotion()`, `itemAt()`, `itemRect()`, `render()` | `gfx/primitives`, `font_cache`, `ui/text_render`, `geom`, `c` | | `ui/components/selection_agent_overlay.zig` | Selection action form with highlighted agent selector, multiline prompt field, fully wrapped and scrollable selected-context preview, viewport-bounded context textures, cached UI text, and launch action containing the selected terminal context | `SelectionAgentOverlayComponent`, `open()`, `formatAgentPrompt()` | `ui/text_edit`, `ui/text_render`, `ui/first_frame_guard`, `ui/components/modal_frame`, `ui/components/dropdown_menu`, `ui/components/scrollbar`, `gfx/primitives`, `font_cache`, `geom`, `c` | -| `ui/components/*` | Individual overlay and widget implementations conforming to `UiComponent` vtable. Includes: help overlay, worktree picker, recent folders picker (with instant search filtering), PR dropdown, diff viewer (with inline review comments), story viewer (PR story file visualization with rich markdown, anchor badges, bezier arrows, clickable links, and Cmd+F search — uses shared markdown parser/renderer pipeline and shared search utilities), reader mode overlay (uses shared search utilities), fullscreen overlay helper (shared animation/scroll/close logic embedded by story, diff, and reader overlays), reusable aqua-style scrollbar widget, session interaction, toast, quit confirm, quit-blocking overlay, restart buttons, escape hold indicator, metrics overlay, global shortcuts, pill group, cwd bar (its clipped parent-path marquee runs only in a focused Grid view), expanding overlay helper (badge-to-panel animation; `State.isOpenOrOpening()` is the canonical "this overlay owns the keyboard and is visible" test, so input is never dropped during the expand), button, confirm dialog (shares its scrim/panel chrome and dismiss-key check with the selection-agent overlay via `ui/components/modal_frame`), marquee label, hotkey indicator, flowing line, hold gesture detector. | Each component implements the `VTable` interface; overlays toggle via keyboard shortcuts or external commands and emit `UiAction` values. | `ui/component`, `ui/types`, `anim/easing`, `font`, `metrics`, `url_matcher`, `ui/session_view_state` | +| `ui/components/*` | Individual overlay and widget implementations conforming to `UiComponent` vtable. Includes: help overlay, worktree picker, recent folders picker (with instant search filtering), PR dropdown, diff viewer (with inline review comments), story viewer (PR story file visualization with rich markdown, anchor badges, bezier arrows, clickable links, and Cmd+F search — uses shared markdown parser/renderer pipeline and shared search utilities), reader mode overlay (uses shared search utilities), fullscreen overlay helper (shared animation/scroll/close logic embedded by story, diff, and reader overlays), reusable aqua-style scrollbar widget, session interaction, toast, quit confirm, quit-blocking overlay, restart buttons, escape hold indicator, metrics overlay, global shortcuts, dynamic pill group, cwd bar (its clipped parent-path marquee runs only in a focused Grid view), expanding overlay helper (badge-to-panel animation; `State.isOpenOrOpening()` is the canonical "this overlay owns the keyboard and is visible" test, so input is never dropped during the expand), button, confirm dialog (shares its scrim/panel chrome and dismiss-key check with the selection-agent overlay via `ui/components/modal_frame`), marquee label, hotkey indicator, flowing line, hold gesture detector. | Each component implements the `VTable` interface; overlays toggle via keyboard shortcuts or external commands and emit `UiAction` values. The pill group additionally owns right-aligned placement and eased membership transitions for the top-right shortcut row. | `ui/component`, `ui/types`, `anim/easing`, `font`, `metrics`, `url_matcher`, `ui/session_view_state` | +| `ui/components/pill_group.zig` | Dynamic top-right shortcut-pill layout and overlay coordination. Packs currently available pills with fixed spacing, animates existing pills when membership or window geometry changes, and synchronizes the animated position with overlay rendering and hit-testing. | `PillGroupComponent`, `PillLayout` | `ui/components/expanding_overlay`, `ui/components/help_overlay`, `ui/components/recent_folders_overlay`, `ui/components/worktree_overlay`, `ui/components/pr_dropdown`, `ui/first_frame_guard`, `anim/easing`, `dpi` | | `ui/components/pr_dropdown.zig` | GitHub pull request picker orchestration: owns focused-repository state, input/lifecycle handling, repository-keyed worker jobs, stale-result filtering, branch badges, and checkout actions | `PRDropdownComponent` | `ui/components/pr_dropdown_model`, `ui/components/pr_dropdown_repo`, `ui/components/pr_dropdown_fetch`, `ui/components/pr_dropdown_view`, `ui/components/expanding_overlay`, `ui/components/search_utils`, `ui/text_edit`, `ui/types`, `geom`, `c` | | `ui/components/pr_dropdown_model.zig` | Pull request and fetch result types plus pure repository/result matching predicates | `PullRequest`, `FetchStatus`, `FetchResult`, `freeFetchResult()`, `prNumberForBranch()` | std | | `ui/components/pr_dropdown_repo.zig` | Synchronous repository discovery: `.git` and worktree config/HEAD resolution and GitHub origin detection | `findRepoRoot()`, `detectGithubOrigin()`, `readCurrentBranch()`, `originUrlIsGithub()` | std | diff --git a/src/main.zig b/src/main.zig index 1251c32a..9fb0fa02 100644 --- a/src/main.zig +++ b/src/main.zig @@ -72,6 +72,7 @@ test { _ = @import("ui/components/markdown_parser.zig"); _ = @import("ui/components/markdown_renderer.zig"); _ = @import("ui/components/metrics_overlay.zig"); + _ = @import("ui/components/pill_group.zig"); _ = @import("ui/components/quit_blocking_overlay.zig"); _ = @import("ui/components/recent_folders_overlay.zig"); _ = @import("ui/components/pr_dropdown.zig"); diff --git a/src/ui/components/expanding_overlay.zig b/src/ui/components/expanding_overlay.zig index 49ac186d..4adf7349 100644 --- a/src/ui/components/expanding_overlay.zig +++ b/src/ui/components/expanding_overlay.zig @@ -9,6 +9,7 @@ pub const ExpandingOverlay = struct { start_size: c_int, target_size: c_int, slot: usize, + layout_x: ?c_int = null, margin: c_int, small_size: c_int, large_size: c_int, @@ -46,6 +47,12 @@ pub const ExpandingOverlay = struct { self.content_height = height; } + /// Sets the left edge of the collapsed pill. Expanded rectangles keep the + /// same right edge, so the panel grows to the left from this position. + pub fn setLayoutX(self: *ExpandingOverlay, x: c_int) void { + self.layout_x = x; + } + pub fn startExpanding(self: *ExpandingOverlay, now: i64) void { self.state = .Expanding; self.start_time = now; @@ -91,8 +98,13 @@ pub const ExpandingOverlay = struct { _ = window_height; const margin = dpi.scale(self.margin, ui_scale); const size = self.currentSize(now, ui_scale); + const small = dpi.scale(self.small_size, ui_scale); + const large = dpi.scale(self.large_size, ui_scale); const spacing = dpi.scale(self.small_size + self.margin, ui_scale); - const x = window_width - margin - size - @as(c_int, @intCast(self.slot)) * spacing; + const x = if (self.layout_x) |collapsed_x| + collapsed_x - (size - small) + else + window_width - margin - size - @as(c_int, @intCast(self.slot)) * spacing; const y = margin; const height = blk: { @@ -100,8 +112,6 @@ pub const ExpandingOverlay = struct { break :blk size; } - const small = dpi.scale(self.small_size, ui_scale); - const large = dpi.scale(self.large_size, ui_scale); // content_height is already scaled by the overlay component const target_height = self.content_height; @@ -150,3 +160,13 @@ test "collapsing from the open state starts at full size" { try std.testing.expectEqual(@as(c_int, 400), overlay.currentSize(1000, 1.0)); try std.testing.expectEqual(@as(c_int, 40), overlay.currentSize(1200, 1.0)); } + +test "layout position keeps the expanded overlay right edge anchored" { + var overlay = ExpandingOverlay.init(0, 20, 40, 400, 200); + overlay.setLayoutX(600); + overlay.startExpanding(0); + + const rect = overlay.rect(100, 800, 800, 1.0); + try std.testing.expectEqual(@as(c_int, 420), rect.x); + try std.testing.expectEqual(@as(c_int, 640), rect.x + rect.w); +} diff --git a/src/ui/components/pill_group.zig b/src/ui/components/pill_group.zig index 02b3b5bf..dfc9a7b4 100644 --- a/src/ui/components/pill_group.zig +++ b/src/ui/components/pill_group.zig @@ -7,7 +7,126 @@ const WorktreeOverlayComponent = @import("worktree_overlay.zig").WorktreeOverlay const RecentFoldersOverlayComponent = @import("recent_folders_overlay.zig").RecentFoldersOverlayComponent; const PRDropdownComponent = @import("pr_dropdown.zig").PRDropdownComponent; +const dpi = @import("../../dpi.zig"); +const easing = @import("../../anim/easing.zig"); const ExpandingOverlay = @import("expanding_overlay.zig").ExpandingOverlay; +const FirstFrameGuard = @import("../first_frame_guard.zig").FirstFrameGuard; + +const pill_count: usize = 4; +const pill_size: c_int = 40; +const pill_margin: c_int = 20; +const pill_spacing: c_int = 20; +const pill_animation_duration_ms: i64 = 200; + +const PillKind = enum(usize) { + pull_request, + worktree, + recent_folders, + help, +}; + +const layout_order = [_]PillKind{ .help, .recent_folders, .worktree, .pull_request }; + +const PillLayout = struct { + current_x: [pill_count]c_int = [_]c_int{0} ** pill_count, + start_x: [pill_count]c_int = [_]c_int{0} ** pill_count, + target_x: [pill_count]c_int = [_]c_int{0} ** pill_count, + visible: [pill_count]bool = [_]bool{false} ** pill_count, + start_time: i64 = 0, + initialized: bool = false, + animating: bool = false, + + fn update(self: *PillLayout, now_ms: i64, window_w: c_int, ui_scale: f32, visible: [pill_count]bool) bool { + if (self.initialized) { + self.advance(now_ms); + } + + const targets = self.calculateTargets(window_w, ui_scale, visible); + if (!self.initialized) { + self.current_x = targets; + self.start_x = targets; + self.target_x = targets; + self.visible = visible; + self.initialized = true; + return false; + } + + const membership_changed = !std.mem.eql(bool, self.visible[0..], visible[0..]); + var target_changed = false; + for (0..pill_count) |idx| { + if (visible[idx] and self.target_x[idx] != targets[idx]) { + target_changed = true; + break; + } + } + + if (!membership_changed and !target_changed) return false; + + self.start_x = self.current_x; + self.target_x = targets; + self.visible = visible; + self.start_time = now_ms; + self.animating = true; + return membership_changed; + } + + fn calculateTargets(self: *const PillLayout, window_w: c_int, ui_scale: f32, visible: [pill_count]bool) [pill_count]c_int { + var targets = self.current_x; + const margin = dpi.scale(pill_margin, ui_scale); + const size = dpi.scale(pill_size, ui_scale); + const spacing = dpi.scale(pill_spacing, ui_scale); + + // Seed absent pills with their former full-row locations. This gives + // a pill that appears later a nearby origin for its entrance motion; + // subsequent updates preserve the last location while it is hidden. + if (!self.initialized) { + var default_right_edge = window_w - margin; + for (layout_order) |pill| { + const idx: usize = @intFromEnum(pill); + targets[idx] = default_right_edge - size; + default_right_edge = targets[idx] - spacing; + } + } + + var right_edge = window_w - margin; + + for (layout_order) |pill| { + const idx: usize = @intFromEnum(pill); + if (!visible[idx]) continue; + targets[idx] = right_edge - size; + right_edge = targets[idx] - spacing; + } + + return targets; + } + + fn advance(self: *PillLayout, now_ms: i64) void { + if (!self.animating) return; + + const elapsed = now_ms - self.start_time; + if (elapsed >= pill_animation_duration_ms) { + self.current_x = self.target_x; + self.animating = false; + return; + } + + const clamped_elapsed: i64 = @max(@as(i64, 0), elapsed); + const progress: f32 = @min( + @as(f32, 1.0), + @as(f32, @floatFromInt(clamped_elapsed)) / @as(f32, @floatFromInt(pill_animation_duration_ms)), + ); + const eased = easing.easeInOutCubic(progress); + + for (0..pill_count) |idx| { + const distance = self.target_x[idx] - self.start_x[idx]; + self.current_x[idx] = self.start_x[idx] + @as(c_int, @intFromFloat(@as(f32, @floatFromInt(distance)) * eased)); + } + } + + fn currentX(self: *const PillLayout, pill: PillKind) c_int { + return self.current_x[@intFromEnum(pill)]; + } +}; pub const PillGroupComponent = struct { allocator: std.mem.Allocator, @@ -15,11 +134,15 @@ pub const PillGroupComponent = struct { recent_folders: *RecentFoldersOverlayComponent, worktree: *WorktreeOverlayComponent, pr_dropdown: *PRDropdownComponent, + layout: PillLayout = .{}, + first_frame: FirstFrameGuard = .{}, last_help_state: ExpandingOverlay.State = .Closed, last_recent_folders_state: ExpandingOverlay.State = .Closed, last_worktree_state: ExpandingOverlay.State = .Closed, last_pr_state: ExpandingOverlay.State = .Closed, + pub const component_z_index: i32 = 1001; + pub fn create( allocator: std.mem.Allocator, help: *HelpOverlayComponent, @@ -39,7 +162,7 @@ pub const PillGroupComponent = struct { return UiComponent{ .ptr = comp, .vtable = &vtable, - .z_index = 999, + .z_index = component_z_index, }; } @@ -118,13 +241,39 @@ pub const PillGroupComponent = struct { } } + const visible = [pill_count]bool{ + self.pr_dropdown.pillVisible(host), + self.worktree.pillVisible(host), + self.recent_folders.pillVisible(host), + true, + }; + if (self.layout.update(host.now_ms, host.window_w, host.ui_scale, visible)) { + self.first_frame.markTransition(); + } + self.applyLayout(); + self.last_help_state = help_state; self.last_recent_folders_state = recent_folders_state; self.last_worktree_state = worktree_state; self.last_pr_state = pr_state; } - fn render(_: *anyopaque, _: *const types.UiHost, _: *c.SDL_Renderer, _: *types.UiAssets) void {} + fn applyLayout(self: *PillGroupComponent) void { + self.pr_dropdown.overlay.setLayoutX(self.layout.currentX(.pull_request)); + self.worktree.overlay.setLayoutX(self.layout.currentX(.worktree)); + self.recent_folders.overlay.setLayoutX(self.layout.currentX(.recent_folders)); + self.help.overlay.setLayoutX(self.layout.currentX(.help)); + } + + fn render(self_ptr: *anyopaque, _: *const types.UiHost, _: *c.SDL_Renderer, _: *types.UiAssets) void { + const self: *PillGroupComponent = @ptrCast(@alignCast(self_ptr)); + self.first_frame.markDrawn(); + } + + fn wantsFrame(self_ptr: *anyopaque, _: *const types.UiHost) bool { + const self: *PillGroupComponent = @ptrCast(@alignCast(self_ptr)); + return self.layout.animating or self.first_frame.wantsFrame(); + } fn deinitComp(self_ptr: *anyopaque, renderer: *c.SDL_Renderer) void { deinit(self_ptr, renderer); @@ -136,5 +285,51 @@ pub const PillGroupComponent = struct { .update = update, .render = render, .deinit = deinitComp, + .wantsFrame = wantsFrame, }; }; + +test "pill layout packs visible pills against the right edge" { + var layout: PillLayout = .{}; + const all_visible = [pill_count]bool{ true, true, true, true }; + _ = layout.update(0, 800, 1.0, all_visible); + + try std.testing.expectEqual(@as(c_int, 560), layout.currentX(.pull_request)); + try std.testing.expectEqual(@as(c_int, 620), layout.currentX(.worktree)); + try std.testing.expectEqual(@as(c_int, 680), layout.currentX(.recent_folders)); + try std.testing.expectEqual(@as(c_int, 740), layout.currentX(.help)); + + const without_worktree = [pill_count]bool{ true, false, true, true }; + try std.testing.expect(layout.update(1, 800, 1.0, without_worktree)); + try std.testing.expectEqual(@as(c_int, 620), layout.target_x[@intFromEnum(PillKind.pull_request)]); + try std.testing.expectEqual(@as(c_int, 680), layout.target_x[@intFromEnum(PillKind.recent_folders)]); +} + +test "pill layout eases remaining pills into new positions" { + var layout: PillLayout = .{}; + const all_visible = [pill_count]bool{ true, true, true, true }; + _ = layout.update(0, 800, 1.0, all_visible); + + const without_worktree = [pill_count]bool{ true, false, true, true }; + _ = layout.update(0, 800, 1.0, without_worktree); + _ = layout.update(100, 800, 1.0, without_worktree); + try std.testing.expectEqual(@as(c_int, 590), layout.currentX(.pull_request)); + try std.testing.expect(layout.animating); + + _ = layout.update(200, 800, 1.0, without_worktree); + try std.testing.expectEqual(@as(c_int, 620), layout.currentX(.pull_request)); + try std.testing.expect(!layout.animating); +} + +test "pill layout eases a newly available pill into the compact row" { + var layout: PillLayout = .{}; + const only_help = [pill_count]bool{ false, false, false, true }; + _ = layout.update(0, 800, 1.0, only_help); + + const pull_request_and_help = [pill_count]bool{ true, false, false, true }; + _ = layout.update(0, 800, 1.0, pull_request_and_help); + _ = layout.update(100, 800, 1.0, pull_request_and_help); + + try std.testing.expectEqual(@as(c_int, 620), layout.currentX(.pull_request)); + try std.testing.expectEqual(@as(c_int, 740), layout.currentX(.help)); +} diff --git a/src/ui/components/pr_dropdown.zig b/src/ui/components/pr_dropdown.zig index 8519f6fa..97aaefd7 100644 --- a/src/ui/components/pr_dropdown.zig +++ b/src/ui/components/pr_dropdown.zig @@ -159,7 +159,7 @@ pub const PRDropdownComponent = struct { // Cmd+P toggles overlay (only meaningful inside a GitHub repo) if (has_gui and !has_blocking_mod and key == c.SDLK_P) { - if (!self.is_github_repo) return false; + if (!self.pillVisible(host)) return false; if (self.overlay.state == .Open) { self.closeOverlay(host.now_ms); } else { @@ -232,7 +232,7 @@ pub const PRDropdownComponent = struct { } }, c.SDL_EVENT_MOUSE_BUTTON_DOWN => { - if (!self.is_github_repo) return false; + if (!self.pillVisible(host)) return false; const mouse_x: c_int = @intFromFloat(event.button.x); const mouse_y: c_int = @intFromFloat(event.button.y); const rect = self.overlay.rect(host.now_ms, host.window_w, host.window_h, host.ui_scale); @@ -281,11 +281,19 @@ pub const PRDropdownComponent = struct { fn hitTest(self_ptr: *anyopaque, host: *const types.UiHost, x: c_int, y: c_int) bool { const self: *PRDropdownComponent = @ptrCast(@alignCast(self_ptr)); - if (!self.is_github_repo) return false; + if (!self.pillVisible(host)) return false; const rect = self.overlay.rect(host.now_ms, host.window_w, host.window_h, host.ui_scale); return geom.containsPoint(rect, x, y); } + pub fn shouldShowPill(is_github_repo: bool, focused_busy: bool) bool { + return is_github_repo and !focused_busy; + } + + pub fn pillVisible(self: *const PRDropdownComponent, host: *const types.UiHost) bool { + return shouldShowPill(self.is_github_repo, host.focused_has_foreground_process); + } + fn update(self_ptr: *anyopaque, host: *const types.UiHost, _: *types.UiActionQueue) void { const self: *PRDropdownComponent = @ptrCast(@alignCast(self_ptr)); @@ -321,6 +329,9 @@ pub const PRDropdownComponent = struct { self.escape_pressed = false; } } + if (busy and self.overlay.state.isOpenOrOpening()) { + self.closeOverlay(host.now_ms); + } // Pick up completed background fetch results, including results that // belong to a repository that is no longer focused. @@ -347,7 +358,7 @@ pub const PRDropdownComponent = struct { fn render(self_ptr: *anyopaque, ui_host: *const types.UiHost, renderer: *c.SDL_Renderer, assets: *types.UiAssets) void { const self: *PRDropdownComponent = @ptrCast(@alignCast(self_ptr)); self.first_frame.markDrawn(); - if (!self.is_github_repo) return; + if (!self.pillVisible(ui_host)) return; const rect = self.overlay.rect(ui_host.now_ms, ui_host.window_w, ui_host.window_h, ui_host.ui_scale); const radius: c_int = 8; @@ -714,3 +725,9 @@ pub const PRDropdownComponent = struct { test "PR dropdown renders below sibling pill overlays" { try std.testing.expect(PRDropdownComponent.component_z_index < 1000); } + +test "pull request pill is hidden while the focused shell is busy" { + try std.testing.expect(PRDropdownComponent.shouldShowPill(true, false)); + try std.testing.expect(!PRDropdownComponent.shouldShowPill(true, true)); + try std.testing.expect(!PRDropdownComponent.shouldShowPill(false, false)); +} diff --git a/src/ui/components/recent_folders_overlay.zig b/src/ui/components/recent_folders_overlay.zig index f916c72e..6e86dd0e 100644 --- a/src/ui/components/recent_folders_overlay.zig +++ b/src/ui/components/recent_folders_overlay.zig @@ -162,6 +162,7 @@ pub const RecentFoldersOverlayComponent = struct { switch (event.type) { c.SDL_EVENT_MOUSE_BUTTON_DOWN => { + if (!self.pillVisible(host)) return false; const mouse_x: c_int = @intFromFloat(event.button.x); const mouse_y: c_int = @intFromFloat(event.button.y); const rect = self.overlay.rect(host.now_ms, host.window_w, host.window_h, host.ui_scale); @@ -214,6 +215,7 @@ pub const RecentFoldersOverlayComponent = struct { // Cmd+O toggles overlay if (has_gui and !has_blocking_mod and key == c.SDLK_O) { + if (!self.pillVisible(host)) return false; if (self.overlay.state.isOpenOrOpening()) { self.closeOverlay(host.now_ms); } else { @@ -303,10 +305,19 @@ pub const RecentFoldersOverlayComponent = struct { fn hitTest(self_ptr: *anyopaque, host: *const types.UiHost, x: c_int, y: c_int) bool { const self: *RecentFoldersOverlayComponent = @ptrCast(@alignCast(self_ptr)); + if (!self.pillVisible(host)) return false; const rect = self.overlay.rect(host.now_ms, host.window_w, host.window_h, host.ui_scale); return geom.containsPoint(rect, x, y); } + pub fn shouldShowPill(folder_count: usize, focused_busy: bool) bool { + return folder_count > 0 and !focused_busy; + } + + pub fn pillVisible(self: *const RecentFoldersOverlayComponent, host: *const types.UiHost) bool { + return shouldShowPill(self.all_folders.items.len, host.focused_has_foreground_process); + } + fn update(self_ptr: *anyopaque, host: *const types.UiHost, _: *types.UiActionQueue) void { const self: *RecentFoldersOverlayComponent = @ptrCast(@alignCast(self_ptr)); @@ -323,6 +334,10 @@ pub const RecentFoldersOverlayComponent = struct { } } + if (!self.pillVisible(host) and self.overlay.state.isOpenOrOpening()) { + self.closeOverlay(host.now_ms); + } + if (self.overlay.isAnimating() and self.overlay.isComplete(host.now_ms)) { self.overlay.state = switch (self.overlay.state) { .Expanding => .Open, @@ -348,7 +363,7 @@ pub const RecentFoldersOverlayComponent = struct { fn render(self_ptr: *anyopaque, ui_host: *const types.UiHost, renderer: *c.SDL_Renderer, assets: *types.UiAssets) void { const self: *RecentFoldersOverlayComponent = @ptrCast(@alignCast(self_ptr)); self.first_frame.markDrawn(); - if (self.all_folders.items.len == 0) return; + if (!self.pillVisible(ui_host)) return; const rect = self.overlay.rect(ui_host.now_ms, ui_host.window_w, ui_host.window_h, ui_host.ui_scale); const radius: c_int = 8; @@ -902,6 +917,13 @@ const TestComponent = struct { } }; +fn addTestFolder(t: *TestComponent) void { + const folders = [_]config.Persistence.RecentFolder{ + .{ .path = "/tmp/architect-test-folder", .count = 1 }, + }; + t.comp.setFolders(&folders); +} + test "Cmd+Backspace clears the search query and Alt+Backspace drops one segment" { var t = TestComponent.init(); defer t.deinit(); @@ -938,6 +960,7 @@ test "keys typed during the expand animation reach the search query" { // Cmd+O starts the expand; the overlay is .Expanding, not .Open, for the // whole animation, and must already own the keyboard. + addTestFolder(&t); try testing.expect(t.send(keyEvent(c.SDLK_O, c.SDL_KMOD_GUI), 0)); try testing.expectEqual(ExpandingOverlay.State.Expanding, t.comp.overlay.state); @@ -953,6 +976,7 @@ test "Cmd+O during the expand animation closes the overlay" { var t = TestComponent.init(); defer t.deinit(); + addTestFolder(&t); try testing.expect(t.send(keyEvent(c.SDLK_O, c.SDL_KMOD_GUI), 0)); try testing.expect(t.send(keyEvent(c.SDLK_O, c.SDL_KMOD_GUI), 50)); try testing.expectEqual(ExpandingOverlay.State.Collapsing, t.comp.overlay.state); @@ -1006,8 +1030,15 @@ test "the caret blinks while the picker is open" { defer t.deinit(); // Opening resets the blink so the caret is solid on the first frame. + addTestFolder(&t); try testing.expect(t.send(keyEvent(c.SDLK_O, c.SDL_KMOD_GUI), 1000)); try testing.expect(t.comp.search.caretVisible(1000)); try testing.expect(!t.comp.search.caretVisible(1600)); try testing.expect(t.comp.search.caretVisible(2100)); } + +test "recent folders pill is hidden while the focused shell is busy" { + try testing.expect(RecentFoldersOverlayComponent.shouldShowPill(1, false)); + try testing.expect(!RecentFoldersOverlayComponent.shouldShowPill(1, true)); + try testing.expect(!RecentFoldersOverlayComponent.shouldShowPill(0, false)); +} diff --git a/src/ui/components/worktree_overlay.zig b/src/ui/components/worktree_overlay.zig index b92ea9b9..16f21c59 100644 --- a/src/ui/components/worktree_overlay.zig +++ b/src/ui/components/worktree_overlay.zig @@ -141,7 +141,7 @@ pub const WorktreeOverlayComponent = struct { } } - if (!self.available) return false; + if (!self.available or host.focused_has_foreground_process) return false; switch (event.type) { c.SDL_EVENT_MOUSE_BUTTON_DOWN => { @@ -268,11 +268,19 @@ pub const WorktreeOverlayComponent = struct { fn hitTest(self_ptr: *anyopaque, host: *const types.UiHost, x: c_int, y: c_int) bool { const self: *WorktreeOverlayComponent = @ptrCast(@alignCast(self_ptr)); - if (!self.available) return false; + if (!self.pillVisible(host)) return false; const rect = self.overlay.rect(host.now_ms, host.window_w, host.window_h, host.ui_scale); return geom.containsPoint(rect, x, y); } + pub fn shouldShowPill(available: bool, focused_busy: bool) bool { + return available and !focused_busy; + } + + pub fn pillVisible(self: *const WorktreeOverlayComponent, host: *const types.UiHost) bool { + return shouldShowPill(self.available, host.focused_has_foreground_process); + } + fn update(self_ptr: *anyopaque, host: *const types.UiHost, _: *types.UiActionQueue) void { const self: *WorktreeOverlayComponent = @ptrCast(@alignCast(self_ptr)); @@ -346,7 +354,7 @@ pub const WorktreeOverlayComponent = struct { fn render(self_ptr: *anyopaque, ui_host: *const types.UiHost, renderer: *c.SDL_Renderer, assets: *types.UiAssets) void { const self: *WorktreeOverlayComponent = @ptrCast(@alignCast(self_ptr)); self.first_frame.markDrawn(); - if (!self.available and !self.creating and !self.confirming_removal) return; + if (!self.pillVisible(ui_host) and !self.creating and !self.confirming_removal) return; if (self.creating) { _ = self.ensureCache(renderer, ui_host.ui_scale, assets, ui_host.theme); @@ -1662,3 +1670,9 @@ test "createModalInputStyle uses the active theme colors" { try std.testing.expectEqual(@as(u8, 6), style.placeholder.b); try std.testing.expectEqual(@as(u8, 150), style.placeholder.a); } + +test "worktree pill is hidden while the focused shell is busy" { + try std.testing.expect(WorktreeOverlayComponent.shouldShowPill(true, false)); + try std.testing.expect(!WorktreeOverlayComponent.shouldShowPill(true, true)); + try std.testing.expect(!WorktreeOverlayComponent.shouldShowPill(false, false)); +} From f3eed61739fa07177023a3ec3504dbfa9ecd5a94 Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Thu, 3 Sep 2026 15:06:58 +0200 Subject: [PATCH 2/8] fix(ui): close shortcut picker input leaks Issue: Review found that open shortcut pickers could consume input during a foreground process, and the pill layout could stop rendering before its final animation coordinates were presented. Solution: Re-arm the pill group's first-frame guard when layout easing completes. Reject all events from unavailable pull-request and recent-folder pickers after Escape cleanup, so foreground-process input is not consumed. Add regression coverage and document event-boundary gating. Addresses: https://github.com/forketyfork/architect/pull/398#discussion_r3924491720 Addresses: https://github.com/forketyfork/architect/pull/398#discussion_r3924491843 Addresses: https://github.com/forketyfork/architect/pull/398#discussion_r3924491895 --- docs/ARCHITECTURE.md | 5 +- src/ui/components/pill_group.zig | 27 +++++-- src/ui/components/pr_dropdown.zig | 76 +++++++++++++++++++- src/ui/components/recent_folders_overlay.zig | 31 ++++++-- 4 files changed, 125 insertions(+), 14 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index f03f3188..99dc4eed 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -356,7 +356,10 @@ the existing pills interpolate to their new positions with the shared cubic easing function. The group also requests frames for the short layout animation, so the transition is visible under idle throttling. The worktree, recent-folder, and pull-request pills are unavailable while the focused terminal has a -foreground process, because their actions send commands to that shell. +foreground process, because their actions send commands to that shell. Their +event handlers reject input at the same boundary, before the scheduled update +collapses an already-open picker, so foreground-process input cannot be +consumed by stale UI. ### External Notification Path diff --git a/src/ui/components/pill_group.zig b/src/ui/components/pill_group.zig index dfc9a7b4..dda919b1 100644 --- a/src/ui/components/pill_group.zig +++ b/src/ui/components/pill_group.zig @@ -37,9 +37,7 @@ const PillLayout = struct { animating: bool = false, fn update(self: *PillLayout, now_ms: i64, window_w: c_int, ui_scale: f32, visible: [pill_count]bool) bool { - if (self.initialized) { - self.advance(now_ms); - } + const animation_completed = if (self.initialized) self.advance(now_ms) else false; const targets = self.calculateTargets(window_w, ui_scale, visible); if (!self.initialized) { @@ -60,7 +58,7 @@ const PillLayout = struct { } } - if (!membership_changed and !target_changed) return false; + if (!membership_changed and !target_changed) return animation_completed; self.start_x = self.current_x; self.target_x = targets; @@ -100,14 +98,14 @@ const PillLayout = struct { return targets; } - fn advance(self: *PillLayout, now_ms: i64) void { - if (!self.animating) return; + fn advance(self: *PillLayout, now_ms: i64) bool { + if (!self.animating) return false; const elapsed = now_ms - self.start_time; if (elapsed >= pill_animation_duration_ms) { self.current_x = self.target_x; self.animating = false; - return; + return true; } const clamped_elapsed: i64 = @max(@as(i64, 0), elapsed); @@ -121,6 +119,7 @@ const PillLayout = struct { const distance = self.target_x[idx] - self.start_x[idx]; self.current_x[idx] = self.start_x[idx] + @as(c_int, @intFromFloat(@as(f32, @floatFromInt(distance)) * eased)); } + return false; } fn currentX(self: *const PillLayout, pill: PillKind) c_int { @@ -321,6 +320,20 @@ test "pill layout eases remaining pills into new positions" { try std.testing.expect(!layout.animating); } +test "pill layout reports when easing reaches its final position" { + var layout: PillLayout = .{}; + const all_visible = [pill_count]bool{ true, true, true, true }; + _ = layout.update(0, 800, 1.0, all_visible); + + const without_worktree = [pill_count]bool{ true, false, true, true }; + _ = layout.update(0, 800, 1.0, without_worktree); + try std.testing.expect(layout.animating); + + try std.testing.expect(layout.update(pill_animation_duration_ms, 800, 1.0, without_worktree)); + try std.testing.expectEqual(@as(c_int, 620), layout.currentX(.pull_request)); + try std.testing.expect(!layout.animating); +} + test "pill layout eases a newly available pill into the compact row" { var layout: PillLayout = .{}; const only_help = [pill_count]bool{ false, false, false, true }; diff --git a/src/ui/components/pr_dropdown.zig b/src/ui/components/pr_dropdown.zig index 97aaefd7..6ee14f2d 100644 --- a/src/ui/components/pr_dropdown.zig +++ b/src/ui/components/pr_dropdown.zig @@ -1,5 +1,6 @@ const std = @import("std"); const c = @import("../../c.zig"); +const colors = @import("../../colors.zig"); const geom = @import("../../geom.zig"); const primitives = @import("../../gfx/primitives.zig"); const types = @import("../types.zig"); @@ -150,6 +151,8 @@ pub const PRDropdownComponent = struct { } } + if (!self.pillVisible(host)) return false; + switch (event.type) { c.SDL_EVENT_KEY_DOWN => { const key = event.key.key; @@ -159,7 +162,6 @@ pub const PRDropdownComponent = struct { // Cmd+P toggles overlay (only meaningful inside a GitHub repo) if (has_gui and !has_blocking_mod and key == c.SDLK_P) { - if (!self.pillVisible(host)) return false; if (self.overlay.state == .Open) { self.closeOverlay(host.now_ms); } else { @@ -232,7 +234,6 @@ pub const PRDropdownComponent = struct { } }, c.SDL_EVENT_MOUSE_BUTTON_DOWN => { - if (!self.pillVisible(host)) return false; const mouse_x: c_int = @intFromFloat(event.button.x); const mouse_y: c_int = @intFromFloat(event.button.y); const rect = self.overlay.rect(host.now_ms, host.window_w, host.window_h, host.ui_scale); @@ -731,3 +732,74 @@ test "pull request pill is hidden while the focused shell is busy" { try std.testing.expect(!PRDropdownComponent.shouldShowPill(true, true)); try std.testing.expect(!PRDropdownComponent.shouldShowPill(false, false)); } + +const testing = std.testing; + +fn testTheme() colors.Theme { + const base = c.SDL_Color{ .r = 0, .g = 0, .b = 0, .a = 255 }; + return .{ + .background = base, + .foreground = base, + .selection = base, + .accent = base, + .palette = [_]c.SDL_Color{base} ** 16, + }; +} + +fn testHost(now_ms: i64, focused_busy: bool, theme: *const colors.Theme) types.UiHost { + return .{ + .now_ms = now_ms, + .window_w = 1200, + .window_h = 800, + .window_focused = true, + .ui_scale = 1.0, + .grid_cols = 2, + .grid_rows = 2, + .cell_w = 8, + .cell_h = 16, + .term_cols = 80, + .term_rows = 24, + .view_mode = .Grid, + .focused_session = 0, + .focused_cwd = null, + .focused_has_foreground_process = focused_busy, + .sessions = &.{}, + .theme = theme, + }; +} + +fn keyEvent(key: c.SDL_Keycode, mod: c.SDL_Keymod) c.SDL_Event { + var event: c.SDL_Event = undefined; + event.type = c.SDL_EVENT_KEY_DOWN; + event.key.key = key; + event.key.mod = mod; + return event; +} + +fn textEvent(text: [*c]const u8) c.SDL_Event { + var event: c.SDL_Event = undefined; + event.type = c.SDL_EVENT_TEXT_INPUT; + event.text.text = text; + return event; +} + +test "open PR picker does not consume input while the focused shell is busy" { + var component: PRDropdownComponent = .{ .allocator = testing.allocator, .io = undefined }; + defer component.search_query.deinit(testing.allocator); + component.is_github_repo = true; + component.overlay.state = .Open; + try component.search_query.buf.appendSlice(testing.allocator, "before"); + + var actions = types.UiActionQueue.init(testing.allocator); + defer actions.deinit(); + var theme = testTheme(); + const host = testHost(0, true, &theme); + + var typed = textEvent("x"); + try testing.expect(!PRDropdownComponent.handleEvent(&component, &host, &typed, &actions)); + try testing.expectEqualStrings("before", component.search_query.text()); + + var erased = keyEvent(c.SDLK_BACKSPACE, 0); + try testing.expect(!PRDropdownComponent.handleEvent(&component, &host, &erased, &actions)); + try testing.expectEqualStrings("before", component.search_query.text()); +} diff --git a/src/ui/components/recent_folders_overlay.zig b/src/ui/components/recent_folders_overlay.zig index 6e86dd0e..00b97cca 100644 --- a/src/ui/components/recent_folders_overlay.zig +++ b/src/ui/components/recent_folders_overlay.zig @@ -160,9 +160,10 @@ pub const RecentFoldersOverlayComponent = struct { } } + if (!self.pillVisible(host)) return false; + switch (event.type) { c.SDL_EVENT_MOUSE_BUTTON_DOWN => { - if (!self.pillVisible(host)) return false; const mouse_x: c_int = @intFromFloat(event.button.x); const mouse_y: c_int = @intFromFloat(event.button.y); const rect = self.overlay.rect(host.now_ms, host.window_w, host.window_h, host.ui_scale); @@ -215,7 +216,6 @@ pub const RecentFoldersOverlayComponent = struct { // Cmd+O toggles overlay if (has_gui and !has_blocking_mod and key == c.SDLK_O) { - if (!self.pillVisible(host)) return false; if (self.overlay.state.isOpenOrOpening()) { self.closeOverlay(host.now_ms); } else { @@ -910,11 +910,16 @@ const TestComponent = struct { self.actions.deinit(); } - fn send(self: *TestComponent, event: c.SDL_Event, now_ms: i64) bool { - const host = testHost(now_ms, &self.theme); + fn sendWithBusy(self: *TestComponent, event: c.SDL_Event, now_ms: i64, focused_busy: bool) bool { + var host = testHost(now_ms, &self.theme); + host.focused_has_foreground_process = focused_busy; var ev = event; return RecentFoldersOverlayComponent.handleEvent(&self.comp, &host, &ev, &self.actions); } + + fn send(self: *TestComponent, event: c.SDL_Event, now_ms: i64) bool { + return self.sendWithBusy(event, now_ms, false); + } }; fn addTestFolder(t: *TestComponent) void { @@ -928,6 +933,7 @@ test "Cmd+Backspace clears the search query and Alt+Backspace drops one segment" var t = TestComponent.init(); defer t.deinit(); + addTestFolder(&t); t.comp.overlay.state = .Open; try t.comp.search.buf.appendSlice(testing.allocator, "dev/github/architect"); @@ -947,6 +953,7 @@ test "plain Backspace still deletes a single character" { var t = TestComponent.init(); defer t.deinit(); + addTestFolder(&t); t.comp.overlay.state = .Open; try t.comp.search.buf.appendSlice(testing.allocator, "arch"); @@ -995,6 +1002,7 @@ test "Cmd+A selects the query and the next keystroke replaces it" { var t = TestComponent.init(); defer t.deinit(); + addTestFolder(&t); t.comp.overlay.state = .Open; try t.comp.search.buf.appendSlice(testing.allocator, "arch"); @@ -1042,3 +1050,18 @@ test "recent folders pill is hidden while the focused shell is busy" { try testing.expect(!RecentFoldersOverlayComponent.shouldShowPill(1, true)); try testing.expect(!RecentFoldersOverlayComponent.shouldShowPill(0, false)); } + +test "open recent folders picker does not consume input while the focused shell is busy" { + var t = TestComponent.init(); + defer t.deinit(); + + addTestFolder(&t); + t.comp.overlay.state = .Open; + try t.comp.search.buf.appendSlice(testing.allocator, "before"); + + try testing.expect(!t.sendWithBusy(textEvent("x"), 0, true)); + try testing.expectEqualStrings("before", t.comp.search.text()); + + try testing.expect(!t.sendWithBusy(keyEvent(c.SDLK_BACKSPACE, 0), 0, true)); + try testing.expectEqualStrings("before", t.comp.search.text()); +} From f91a9f37cc34cf91b73699b6931f50cc83db9eac Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Thu, 3 Sep 2026 15:24:25 +0200 Subject: [PATCH 3/8] fix(worktree): clear removal modal on busy shell Issue: Copilot review found that the worktree removal confirmation modal remained visible after the focused terminal became busy, even though its input was disabled. Solution: Clear the pending removal state, including the confirmation flag and owned path, during the busy transition before collapsing the worktree overlay. Add a regression test covering modal cleanup and document the behavior. Addresses: https://github.com/forketyfork/architect/pull/398#pullrequestreview-5102369984 --- docs/ARCHITECTURE.md | 3 ++- src/ui/components/worktree_overlay.zig | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 99dc4eed..b19665cf 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -359,7 +359,8 @@ and pull-request pills are unavailable while the focused terminal has a foreground process, because their actions send commands to that shell. Their event handlers reject input at the same boundary, before the scheduled update collapses an already-open picker, so foreground-process input cannot be -consumed by stale UI. +consumed by stale UI. A busy transition also clears any pending worktree +removal confirmation before the modal is hidden. ### External Notification Path diff --git a/src/ui/components/worktree_overlay.zig b/src/ui/components/worktree_overlay.zig index 16f21c59..9e5b777c 100644 --- a/src/ui/components/worktree_overlay.zig +++ b/src/ui/components/worktree_overlay.zig @@ -294,6 +294,7 @@ pub const WorktreeOverlayComponent = struct { self.creating = false; self.escape_pressed = false; self.clearCreateInput(); + self.clearPendingRemoval(); if (self.overlay.state == .Open or self.overlay.state == .Expanding) { self.overlay.startCollapsing(host.now_ms); } @@ -1676,3 +1677,25 @@ test "worktree pill is hidden while the focused shell is busy" { try std.testing.expect(!WorktreeOverlayComponent.shouldShowPill(true, true)); try std.testing.expect(!WorktreeOverlayComponent.shouldShowPill(false, false)); } + +test "busy transition clears the removal confirmation modal" { + var component: WorktreeOverlayComponent = .{ .allocator = std.testing.allocator, .io = undefined }; + component.available = true; + component.overlay.state = .Open; + component.confirming_removal = true; + component.pending_removal_index = 0; + component.pending_removal_path = try std.testing.allocator.dupe(u8, "/tmp/architect-worktree"); + + var host: types.UiHost = undefined; + host.now_ms = 100; + host.focused_has_foreground_process = true; + var actions = types.UiActionQueue.init(std.testing.allocator); + defer actions.deinit(); + + WorktreeOverlayComponent.update(&component, &host, &actions); + + try std.testing.expect(!component.confirming_removal); + try std.testing.expectEqual(@as(?usize, null), component.pending_removal_index); + try std.testing.expect(component.pending_removal_path == null); + try std.testing.expectEqual(ExpandingOverlay.State.Collapsing, component.overlay.state); +} From 1f9b27df008b8c06281e363ec6d060e6b0b2becd Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Thu, 3 Sep 2026 16:00:31 +0200 Subject: [PATCH 4/8] fix(ui): close unavailable shortcut pickers immediately Issue: Shortcut pickers could reappear in a partially collapsed state after the focused terminal became busy, and newly available pills could overlap existing hit targets during layout changes. Solution: Force unavailable pickers to the closed state while clearing their transient input and flow state. Stage newly available pills outside the row until the existing pills finish reflowing, and cover both behaviors with regression tests and documentation. Addresses: https://github.com/forketyfork/architect/pull/398#discussion_r3925118313 --- README.md | 2 +- docs/ARCHITECTURE.md | 16 ++++--- src/ui/components/expanding_overlay.zig | 16 +++++++ src/ui/components/pill_group.zig | 46 +++++++++++++------ src/ui/components/pr_dropdown.zig | 48 ++++++++++++++++++-- src/ui/components/recent_folders_overlay.zig | 46 ++++++++++++++++--- 6 files changed, 142 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 52ab05fe..e40f2a15 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,7 @@ Architect solves this with a grid view that keeps all your agents visible, with ### Terminal Essentials - Smooth animated transitions for grid expansion, contraction, and reflow (cells and borders move/resize together) -- Context-aware shortcut pills stay right-aligned with fixed spacing, compact when a pill is unavailable, and animate into their new positions; the worktree, recent-folder, and pull-request pills are hidden while the focused terminal runs a foreground process +- Context-aware shortcut pills stay right-aligned with fixed spacing, compact when a pill is unavailable, and animate into their new positions; newly available pills enter after the existing row reflows to avoid overlapping hit targets; the worktree, recent-folder, and pull-request pills are hidden while the focused terminal runs a foreground process and any open picker closes immediately - Wakeable idle input handling keeps typing responsive after short idle periods instead of waiting on a fixed sleep window - Keyboard navigation: ⌘+Return to expand, ⌘1–⌘0 to switch grid slots, ⌘Arrow to move focus in grid view (plays a brief wave animation on the destination terminal), ⌘N to add, ⌘W to close a terminal (restarts if it's the only terminal), ⌘T for worktrees, ⌘O for recent folders, ⌘D for repo-wide git diff (staged + unstaged + untracked), ⌘R for reader mode, ⌘/ for shortcuts; quit with ⌘Q or the window close button - Every text field (recent-folder and reader/story search, worktree name, diff comments, selection-agent instructions) behaves like a macOS text field: a blinking caret, Backspace deletes one character, ⌥Backspace the previous word, ⌘Backspace the whole field, ⌘A selects everything (the next keystroke or paste replaces it), and ⌘C/⌘V copy and paste. The selection-agent field accepts multiline instructions with ⇧↩; plain ↩ launches, while Escape or Cancel dismisses it. Pasting multi-line text into a one-line field drops the newlines, and text longer than the field fades out at its edge instead of spilling past it. Emoji are scaled to the surrounding line height wherever they appear (search fields, comments, reader mode, stories, selection-agent instructions) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index b19665cf..2ea7f84d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -353,13 +353,17 @@ currently available; the group packs available pills from right to left with a fixed gap and applies the resulting x-coordinate to the shared `ExpandingOverlay` geometry. When availability or the window geometry changes, the existing pills interpolate to their new positions with the shared cubic -easing function. The group also requests frames for the short layout animation, -so the transition is visible under idle throttling. The worktree, recent-folder, -and pull-request pills are unavailable while the focused terminal has a -foreground process, because their actions send commands to that shell. Their -event handlers reject input at the same boundary, before the scheduled update +easing function. Newly available pills remain staged outside the row while +existing pills reflow, then join the row once the occupied slots are clear. +The group also requests frames for the short layout animation, so the +transition is visible under idle throttling. The worktree, recent-folder, and +pull-request pills are unavailable while the focused terminal has a foreground +process, because their actions send commands to that shell. Their event +handlers reject input at the same boundary, before the scheduled update collapses an already-open picker, so foreground-process input cannot be -consumed by stale UI. A busy transition also clears any pending worktree +consumed by stale UI. A busy or otherwise unavailable transition closes the +recent-folder and PR pickers immediately, resetting their transient state +instead of exposing a partial collapse. It also clears any pending worktree removal confirmation before the modal is hidden. ### External Notification Path diff --git a/src/ui/components/expanding_overlay.zig b/src/ui/components/expanding_overlay.zig index 4adf7349..4d258205 100644 --- a/src/ui/components/expanding_overlay.zig +++ b/src/ui/components/expanding_overlay.zig @@ -71,6 +71,12 @@ pub const ExpandingOverlay = struct { self.target_size = self.small_size; } + pub fn closeImmediately(self: *ExpandingOverlay) void { + self.state = .Closed; + self.start_size = self.small_size; + self.target_size = self.small_size; + } + pub fn isAnimating(self: *const ExpandingOverlay) bool { return self.state == .Expanding or self.state == .Collapsing; } @@ -161,6 +167,16 @@ test "collapsing from the open state starts at full size" { try std.testing.expectEqual(@as(c_int, 40), overlay.currentSize(1200, 1.0)); } +test "closeImmediately resets the overlay to its collapsed state" { + var overlay = ExpandingOverlay.init(0, 20, 40, 400, 200); + overlay.startExpanding(0); + overlay.closeImmediately(); + + try std.testing.expectEqual(ExpandingOverlay.State.Closed, overlay.state); + try std.testing.expectEqual(@as(c_int, 40), overlay.currentSize(100, 1.0)); + try std.testing.expect(!overlay.isAnimating()); +} + test "layout position keeps the expanded overlay right edge anchored" { var overlay = ExpandingOverlay.init(0, 20, 40, 400, 200); overlay.setLayoutX(600); diff --git a/src/ui/components/pill_group.zig b/src/ui/components/pill_group.zig index dda919b1..504fdc06 100644 --- a/src/ui/components/pill_group.zig +++ b/src/ui/components/pill_group.zig @@ -32,6 +32,7 @@ const PillLayout = struct { start_x: [pill_count]c_int = [_]c_int{0} ** pill_count, target_x: [pill_count]c_int = [_]c_int{0} ** pill_count, visible: [pill_count]bool = [_]bool{false} ** pill_count, + entering: [pill_count]bool = [_]bool{false} ** pill_count, start_time: i64 = 0, initialized: bool = false, animating: bool = false, @@ -62,6 +63,15 @@ const PillLayout = struct { self.start_x = self.current_x; self.target_x = targets; + const entrance_x = window_w + dpi.scale(pill_size, ui_scale); + for (0..pill_count) |idx| { + const stage = visible[idx] and (self.entering[idx] or !self.visible[idx]); + self.entering[idx] = stage; + if (stage) { + self.start_x[idx] = entrance_x; + self.current_x[idx] = entrance_x; + } + } self.visible = visible; self.start_time = now_ms; self.animating = true; @@ -74,18 +84,6 @@ const PillLayout = struct { const size = dpi.scale(pill_size, ui_scale); const spacing = dpi.scale(pill_spacing, ui_scale); - // Seed absent pills with their former full-row locations. This gives - // a pill that appears later a nearby origin for its entrance motion; - // subsequent updates preserve the last location while it is hidden. - if (!self.initialized) { - var default_right_edge = window_w - margin; - for (layout_order) |pill| { - const idx: usize = @intFromEnum(pill); - targets[idx] = default_right_edge - size; - default_right_edge = targets[idx] - spacing; - } - } - var right_edge = window_w - margin; for (layout_order) |pill| { @@ -104,6 +102,7 @@ const PillLayout = struct { const elapsed = now_ms - self.start_time; if (elapsed >= pill_animation_duration_ms) { self.current_x = self.target_x; + self.entering = [_]bool{false} ** pill_count; self.animating = false; return true; } @@ -116,6 +115,7 @@ const PillLayout = struct { const eased = easing.easeInOutCubic(progress); for (0..pill_count) |idx| { + if (self.entering[idx]) continue; const distance = self.target_x[idx] - self.start_x[idx]; self.current_x[idx] = self.start_x[idx] + @as(c_int, @intFromFloat(@as(f32, @floatFromInt(distance)) * eased)); } @@ -343,6 +343,26 @@ test "pill layout eases a newly available pill into the compact row" { _ = layout.update(0, 800, 1.0, pull_request_and_help); _ = layout.update(100, 800, 1.0, pull_request_and_help); - try std.testing.expectEqual(@as(c_int, 620), layout.currentX(.pull_request)); + try std.testing.expectEqual(@as(c_int, 840), layout.currentX(.pull_request)); try std.testing.expectEqual(@as(c_int, 740), layout.currentX(.help)); + + _ = layout.update(200, 800, 1.0, pull_request_and_help); + try std.testing.expectEqual(@as(c_int, 680), layout.currentX(.pull_request)); +} + +test "pill layout stages a newly available pill outside the occupied row" { + var layout: PillLayout = .{}; + const pull_request_and_help = [pill_count]bool{ true, false, false, true }; + _ = layout.update(0, 800, 1.0, pull_request_and_help); + + const with_recent_folders = [pill_count]bool{ true, false, true, true }; + _ = layout.update(0, 800, 1.0, with_recent_folders); + _ = layout.update(100, 800, 1.0, with_recent_folders); + + try std.testing.expectEqual(@as(c_int, 840), layout.currentX(.recent_folders)); + try std.testing.expectEqual(@as(c_int, 650), layout.currentX(.pull_request)); + + _ = layout.update(200, 800, 1.0, with_recent_folders); + try std.testing.expectEqual(@as(c_int, 680), layout.currentX(.recent_folders)); + try std.testing.expectEqual(@as(c_int, 620), layout.currentX(.pull_request)); } diff --git a/src/ui/components/pr_dropdown.zig b/src/ui/components/pr_dropdown.zig index 6ee14f2d..aee0e34e 100644 --- a/src/ui/components/pr_dropdown.zig +++ b/src/ui/components/pr_dropdown.zig @@ -317,7 +317,7 @@ pub const PRDropdownComponent = struct { // Close overlay if no longer applicable. if (!self.is_github_repo and self.overlay.state != .Closed) { - self.closeOverlay(host.now_ms); + self.closeOverlayImmediately(); } // Block while focused shell is busy with a foreground process. @@ -326,12 +326,11 @@ pub const PRDropdownComponent = struct { self.focused_busy = busy; if (busy) { self.destroyCache(); - self.hovered_entry = null; - self.escape_pressed = false; } } - if (busy and self.overlay.state.isOpenOrOpening()) { - self.closeOverlay(host.now_ms); + + if (!self.pillVisible(host) and self.overlay.state != .Closed) { + self.closeOverlayImmediately(); } // Pick up completed background fetch results, including results that @@ -432,6 +431,15 @@ pub const PRDropdownComponent = struct { self.refilter(); } + fn closeOverlayImmediately(self: *PRDropdownComponent) void { + self.overlay.closeImmediately(); + self.search_query.clear(); + self.refilter(); + self.hovered_entry = null; + self.escape_pressed = false; + self.flow_animation_start_ms = 0; + } + fn fetchIsStale(self: *PRDropdownComponent, now_ms: i64) bool { if (self.fetch_status != .ok) return true; if (self.last_fetched_repo == null) return true; @@ -803,3 +811,33 @@ test "open PR picker does not consume input while the focused shell is busy" { try testing.expect(!PRDropdownComponent.handleEvent(&component, &host, &erased, &actions)); try testing.expectEqualStrings("before", component.search_query.text()); } + +test "busy update immediately closes the PR picker" { + var component: PRDropdownComponent = .{ .allocator = testing.allocator, .io = undefined }; + defer component.search_query.deinit(testing.allocator); + + component.is_github_repo = true; + component.overlay.state = .Open; + component.escape_pressed = true; + component.hovered_entry = 0; + component.flow_animation_start_ms = 123; + try component.search_query.buf.appendSlice(testing.allocator, "before"); + + var actions = types.UiActionQueue.init(testing.allocator); + defer actions.deinit(); + var theme = testTheme(); + var host = testHost(200, true, &theme); + PRDropdownComponent.update(&component, &host, &actions); + + try testing.expectEqual(ExpandingOverlay.State.Closed, component.overlay.state); + try testing.expect(!component.overlay.isAnimating()); + try testing.expectEqualStrings("", component.search_query.text()); + try testing.expectEqual(@as(?usize, null), component.hovered_entry); + try testing.expect(!component.escape_pressed); + try testing.expectEqual(@as(i64, 0), component.flow_animation_start_ms); + + host.now_ms = 250; + host.focused_has_foreground_process = false; + PRDropdownComponent.update(&component, &host, &actions); + try testing.expectEqual(ExpandingOverlay.State.Closed, component.overlay.state); +} diff --git a/src/ui/components/recent_folders_overlay.zig b/src/ui/components/recent_folders_overlay.zig index 00b97cca..955b038d 100644 --- a/src/ui/components/recent_folders_overlay.zig +++ b/src/ui/components/recent_folders_overlay.zig @@ -303,6 +303,15 @@ pub const RecentFoldersOverlayComponent = struct { self.refilter(); } + fn closeOverlayImmediately(self: *RecentFoldersOverlayComponent) void { + self.overlay.closeImmediately(); + self.search.clear(); + self.refilter(); + self.hovered_entry = null; + self.escape_pressed = false; + self.flow_animation_start_ms = 0; + } + fn hitTest(self_ptr: *anyopaque, host: *const types.UiHost, x: c_int, y: c_int) bool { const self: *RecentFoldersOverlayComponent = @ptrCast(@alignCast(self_ptr)); if (!self.pillVisible(host)) return false; @@ -326,16 +335,11 @@ pub const RecentFoldersOverlayComponent = struct { self.focused_busy = busy; if (busy) { self.destroyCache(); - self.hovered_entry = null; - self.escape_pressed = false; - if (self.overlay.state == .Open or self.overlay.state == .Expanding) { - self.closeOverlay(host.now_ms); - } } } - if (!self.pillVisible(host) and self.overlay.state.isOpenOrOpening()) { - self.closeOverlay(host.now_ms); + if (!self.pillVisible(host) and self.overlay.state != .Closed) { + self.closeOverlayImmediately(); } if (self.overlay.isAnimating() and self.overlay.isComplete(host.now_ms)) { @@ -1065,3 +1069,31 @@ test "open recent folders picker does not consume input while the focused shell try testing.expect(!t.sendWithBusy(keyEvent(c.SDLK_BACKSPACE, 0), 0, true)); try testing.expectEqualStrings("before", t.comp.search.text()); } + +test "busy update immediately closes the recent folders picker" { + var t = TestComponent.init(); + defer t.deinit(); + + addTestFolder(&t); + t.comp.overlay.state = .Open; + t.comp.escape_pressed = true; + t.comp.hovered_entry = 0; + t.comp.flow_animation_start_ms = 123; + try t.comp.search.buf.appendSlice(testing.allocator, "before"); + + var host = testHost(200, &t.theme); + host.focused_has_foreground_process = true; + RecentFoldersOverlayComponent.update(&t.comp, &host, &t.actions); + + try testing.expectEqual(ExpandingOverlay.State.Closed, t.comp.overlay.state); + try testing.expect(!t.comp.overlay.isAnimating()); + try testing.expectEqualStrings("", t.comp.search.text()); + try testing.expectEqual(@as(?usize, null), t.comp.hovered_entry); + try testing.expect(!t.comp.escape_pressed); + try testing.expectEqual(@as(i64, 0), t.comp.flow_animation_start_ms); + + host.now_ms = 250; + host.focused_has_foreground_process = false; + RecentFoldersOverlayComponent.update(&t.comp, &host, &t.actions); + try testing.expectEqual(ExpandingOverlay.State.Closed, t.comp.overlay.state); +} From 369a4d174b29cc1d60ec6712c1dca7857b3212cc Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Thu, 3 Sep 2026 16:16:51 +0200 Subject: [PATCH 5/8] fix(ui): animate pill entrances and close busy worktrees Issue: Newly available shortcut pills snapped into place after reflow, and a worktree picker could reappear partway through its collapse when a busy terminal became idle quickly. Solution: Split pill membership transitions into reflow and eased entrance phases so staged pills animate while the layout keeps requesting frames. Close unavailable worktree overlays immediately and clear their modal, hover, input, and flow state, with regression coverage for the busy-to-idle transition. Addresses: https://github.com/forketyfork/architect/pull/398#discussion_r3925414683 Addresses: https://github.com/forketyfork/architect/pull/398#discussion_r3925414683 (suppressed worktree finding) --- docs/ARCHITECTURE.md | 11 ++-- src/ui/components/pill_group.zig | 73 +++++++++++++++++++++++--- src/ui/components/worktree_overlay.zig | 40 +++++++++----- 3 files changed, 99 insertions(+), 25 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2ea7f84d..18e80ac3 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -354,7 +354,8 @@ fixed gap and applies the resulting x-coordinate to the shared `ExpandingOverlay` geometry. When availability or the window geometry changes, the existing pills interpolate to their new positions with the shared cubic easing function. Newly available pills remain staged outside the row while -existing pills reflow, then join the row once the occupied slots are clear. +existing pills reflow, then enter in a second eased phase once the occupied +slots are clear. The group also requests frames for the short layout animation, so the transition is visible under idle throttling. The worktree, recent-folder, and pull-request pills are unavailable while the focused terminal has a foreground @@ -362,9 +363,9 @@ process, because their actions send commands to that shell. Their event handlers reject input at the same boundary, before the scheduled update collapses an already-open picker, so foreground-process input cannot be consumed by stale UI. A busy or otherwise unavailable transition closes the -recent-folder and PR pickers immediately, resetting their transient state -instead of exposing a partial collapse. It also clears any pending worktree -removal confirmation before the modal is hidden. +recent-folder, PR, and worktree pickers immediately, resetting their +transient state instead of exposing a partial collapse. It also clears any +pending worktree removal confirmation before the modal is hidden. ### External Notification Path @@ -504,7 +505,7 @@ Rotate: rename active file to architect-.log and continue in new | `ui/components/dropdown_menu.zig` | Reusable vertical list menu: owns open/hover/keyboard-nav state and the committed `selected` index, renders its own cached item-label textures, and reports a `.selected`/`.closed` event on click or Enter/Escape so the owning component reacts (persist the pick, or act on it immediately) instead of tracking hit-testing and highlight rendering itself. Used by `selection_agent_overlay.zig`'s agent selector and `diff_overlay.zig`'s "Send to agent" menu | `DropdownMenu`, `openMenu()`, `close()`, `handleKey()`, `handleClick()`, `handleMotion()`, `itemAt()`, `itemRect()`, `render()` | `gfx/primitives`, `font_cache`, `ui/text_render`, `geom`, `c` | | `ui/components/selection_agent_overlay.zig` | Selection action form with highlighted agent selector, multiline prompt field, fully wrapped and scrollable selected-context preview, viewport-bounded context textures, cached UI text, and launch action containing the selected terminal context | `SelectionAgentOverlayComponent`, `open()`, `formatAgentPrompt()` | `ui/text_edit`, `ui/text_render`, `ui/first_frame_guard`, `ui/components/modal_frame`, `ui/components/dropdown_menu`, `ui/components/scrollbar`, `gfx/primitives`, `font_cache`, `geom`, `c` | | `ui/components/*` | Individual overlay and widget implementations conforming to `UiComponent` vtable. Includes: help overlay, worktree picker, recent folders picker (with instant search filtering), PR dropdown, diff viewer (with inline review comments), story viewer (PR story file visualization with rich markdown, anchor badges, bezier arrows, clickable links, and Cmd+F search — uses shared markdown parser/renderer pipeline and shared search utilities), reader mode overlay (uses shared search utilities), fullscreen overlay helper (shared animation/scroll/close logic embedded by story, diff, and reader overlays), reusable aqua-style scrollbar widget, session interaction, toast, quit confirm, quit-blocking overlay, restart buttons, escape hold indicator, metrics overlay, global shortcuts, dynamic pill group, cwd bar (its clipped parent-path marquee runs only in a focused Grid view), expanding overlay helper (badge-to-panel animation; `State.isOpenOrOpening()` is the canonical "this overlay owns the keyboard and is visible" test, so input is never dropped during the expand), button, confirm dialog (shares its scrim/panel chrome and dismiss-key check with the selection-agent overlay via `ui/components/modal_frame`), marquee label, hotkey indicator, flowing line, hold gesture detector. | Each component implements the `VTable` interface; overlays toggle via keyboard shortcuts or external commands and emit `UiAction` values. The pill group additionally owns right-aligned placement and eased membership transitions for the top-right shortcut row. | `ui/component`, `ui/types`, `anim/easing`, `font`, `metrics`, `url_matcher`, `ui/session_view_state` | -| `ui/components/pill_group.zig` | Dynamic top-right shortcut-pill layout and overlay coordination. Packs currently available pills with fixed spacing, animates existing pills when membership or window geometry changes, and synchronizes the animated position with overlay rendering and hit-testing. | `PillGroupComponent`, `PillLayout` | `ui/components/expanding_overlay`, `ui/components/help_overlay`, `ui/components/recent_folders_overlay`, `ui/components/worktree_overlay`, `ui/components/pr_dropdown`, `ui/first_frame_guard`, `anim/easing`, `dpi` | +| `ui/components/pill_group.zig` | Dynamic top-right shortcut-pill layout and overlay coordination. Packs currently available pills with fixed spacing, animates existing pills when membership or window geometry changes, stages newly available pills during reflow, eases their entrance afterward, and synchronizes positions with overlay rendering and hit-testing. | `PillGroupComponent`, `PillLayout` | `ui/components/expanding_overlay`, `ui/components/help_overlay`, `ui/components/recent_folders_overlay`, `ui/components/worktree_overlay`, `ui/components/pr_dropdown`, `ui/first_frame_guard`, `anim/easing`, `dpi` | | `ui/components/pr_dropdown.zig` | GitHub pull request picker orchestration: owns focused-repository state, input/lifecycle handling, repository-keyed worker jobs, stale-result filtering, branch badges, and checkout actions | `PRDropdownComponent` | `ui/components/pr_dropdown_model`, `ui/components/pr_dropdown_repo`, `ui/components/pr_dropdown_fetch`, `ui/components/pr_dropdown_view`, `ui/components/expanding_overlay`, `ui/components/search_utils`, `ui/text_edit`, `ui/types`, `geom`, `c` | | `ui/components/pr_dropdown_model.zig` | Pull request and fetch result types plus pure repository/result matching predicates | `PullRequest`, `FetchStatus`, `FetchResult`, `freeFetchResult()`, `prNumberForBranch()` | std | | `ui/components/pr_dropdown_repo.zig` | Synchronous repository discovery: `.git` and worktree config/HEAD resolution and GitHub origin detection | `findRepoRoot()`, `detectGithubOrigin()`, `readCurrentBranch()`, `originUrlIsGithub()` | std | diff --git a/src/ui/components/pill_group.zig b/src/ui/components/pill_group.zig index 504fdc06..e3e62e9e 100644 --- a/src/ui/components/pill_group.zig +++ b/src/ui/components/pill_group.zig @@ -27,6 +27,12 @@ const PillKind = enum(usize) { const layout_order = [_]PillKind{ .help, .recent_folders, .worktree, .pull_request }; +const AnimationPhase = enum { + idle, + reflow, + entrance, +}; + const PillLayout = struct { current_x: [pill_count]c_int = [_]c_int{0} ** pill_count, start_x: [pill_count]c_int = [_]c_int{0} ** pill_count, @@ -36,6 +42,7 @@ const PillLayout = struct { start_time: i64 = 0, initialized: bool = false, animating: bool = false, + phase: AnimationPhase = .idle, fn update(self: *PillLayout, now_ms: i64, window_w: c_int, ui_scale: f32, visible: [pill_count]bool) bool { const animation_completed = if (self.initialized) self.advance(now_ms) else false; @@ -65,7 +72,7 @@ const PillLayout = struct { self.target_x = targets; const entrance_x = window_w + dpi.scale(pill_size, ui_scale); for (0..pill_count) |idx| { - const stage = visible[idx] and (self.entering[idx] or !self.visible[idx]); + const stage = visible[idx] and !self.visible[idx]; self.entering[idx] = stage; if (stage) { self.start_x[idx] = entrance_x; @@ -75,6 +82,7 @@ const PillLayout = struct { self.visible = visible; self.start_time = now_ms; self.animating = true; + self.phase = .reflow; return membership_changed; } @@ -100,13 +108,42 @@ const PillLayout = struct { if (!self.animating) return false; const elapsed = now_ms - self.start_time; - if (elapsed >= pill_animation_duration_ms) { - self.current_x = self.target_x; - self.entering = [_]bool{false} ** pill_count; - self.animating = false; - return true; + switch (self.phase) { + .idle => return false, + .reflow => { + if (elapsed >= pill_animation_duration_ms) { + for (0..pill_count) |idx| { + if (!self.entering[idx]) self.current_x[idx] = self.target_x[idx]; + } + + if (self.hasEnteringPills()) { + self.phase = .entrance; + self.start_time += pill_animation_duration_ms; + return self.advance(now_ms); + } + + return self.finishAnimation(); + } + + self.applyEasedPositions(now_ms, false); + }, + .entrance => { + if (elapsed >= pill_animation_duration_ms) return self.finishAnimation(); + self.applyEasedPositions(now_ms, true); + }, + } + return false; + } + + fn hasEnteringPills(self: *const PillLayout) bool { + for (self.entering) |entering| { + if (entering) return true; } + return false; + } + fn applyEasedPositions(self: *PillLayout, now_ms: i64, entering_only: bool) void { + const elapsed = now_ms - self.start_time; const clamped_elapsed: i64 = @max(@as(i64, 0), elapsed); const progress: f32 = @min( @as(f32, 1.0), @@ -115,11 +152,18 @@ const PillLayout = struct { const eased = easing.easeInOutCubic(progress); for (0..pill_count) |idx| { - if (self.entering[idx]) continue; + if (self.entering[idx] != entering_only) continue; const distance = self.target_x[idx] - self.start_x[idx]; self.current_x[idx] = self.start_x[idx] + @as(c_int, @intFromFloat(@as(f32, @floatFromInt(distance)) * eased)); } - return false; + } + + fn finishAnimation(self: *PillLayout) bool { + self.current_x = self.target_x; + self.entering = [_]bool{false} ** pill_count; + self.animating = false; + self.phase = .idle; + return true; } fn currentX(self: *const PillLayout, pill: PillKind) c_int { @@ -347,6 +391,12 @@ test "pill layout eases a newly available pill into the compact row" { try std.testing.expectEqual(@as(c_int, 740), layout.currentX(.help)); _ = layout.update(200, 800, 1.0, pull_request_and_help); + try std.testing.expectEqual(@as(c_int, 840), layout.currentX(.pull_request)); + + _ = layout.update(300, 800, 1.0, pull_request_and_help); + try std.testing.expectEqual(@as(c_int, 760), layout.currentX(.pull_request)); + + _ = layout.update(400, 800, 1.0, pull_request_and_help); try std.testing.expectEqual(@as(c_int, 680), layout.currentX(.pull_request)); } @@ -363,6 +413,13 @@ test "pill layout stages a newly available pill outside the occupied row" { try std.testing.expectEqual(@as(c_int, 650), layout.currentX(.pull_request)); _ = layout.update(200, 800, 1.0, with_recent_folders); + try std.testing.expectEqual(@as(c_int, 840), layout.currentX(.recent_folders)); + try std.testing.expectEqual(@as(c_int, 620), layout.currentX(.pull_request)); + + _ = layout.update(300, 800, 1.0, with_recent_folders); + try std.testing.expectEqual(@as(c_int, 760), layout.currentX(.recent_folders)); + + _ = layout.update(400, 800, 1.0, with_recent_folders); try std.testing.expectEqual(@as(c_int, 680), layout.currentX(.recent_folders)); try std.testing.expectEqual(@as(c_int, 620), layout.currentX(.pull_request)); } diff --git a/src/ui/components/worktree_overlay.zig b/src/ui/components/worktree_overlay.zig index 9e5b777c..1c890537 100644 --- a/src/ui/components/worktree_overlay.zig +++ b/src/ui/components/worktree_overlay.zig @@ -289,15 +289,7 @@ pub const WorktreeOverlayComponent = struct { self.focused_busy = busy; if (busy) { self.available = false; - self.destroyCache(); - self.hovered_entry = null; - self.creating = false; - self.escape_pressed = false; - self.clearCreateInput(); - self.clearPendingRemoval(); - if (self.overlay.state == .Open or self.overlay.state == .Expanding) { - self.overlay.startCollapsing(host.now_ms); - } + self.closeOverlayImmediately(); } else { self.needs_refresh = true; } @@ -347,8 +339,8 @@ pub const WorktreeOverlayComponent = struct { self.needs_refresh = false; } - if (!self.available and self.overlay.state == .Open) { - self.overlay.startCollapsing(host.now_ms); + if (!self.pillVisible(host) and self.overlay.state != .Closed) { + self.closeOverlayImmediately(); } } @@ -1044,6 +1036,21 @@ pub const WorktreeOverlayComponent = struct { } } + fn closeOverlayImmediately(self: *WorktreeOverlayComponent) void { + self.overlay.closeImmediately(); + self.destroyCache(); + self.hovered_entry = null; + self.hovered_remove_btn = null; + self.creating = false; + self.confirming_removal = false; + self.clearCreateInput(); + self.clearPendingRemoval(); + self.escape_pressed = false; + self.flow_animation_start_ms = 0; + self.modal_confirm_hovered = false; + self.modal_cancel_hovered = false; + } + fn clearPendingRemoval(self: *WorktreeOverlayComponent) void { self.confirming_removal = false; self.pending_removal_index = null; @@ -1685,6 +1692,7 @@ test "busy transition clears the removal confirmation modal" { component.confirming_removal = true; component.pending_removal_index = 0; component.pending_removal_path = try std.testing.allocator.dupe(u8, "/tmp/architect-worktree"); + component.flow_animation_start_ms = 123; var host: types.UiHost = undefined; host.now_ms = 100; @@ -1697,5 +1705,13 @@ test "busy transition clears the removal confirmation modal" { try std.testing.expect(!component.confirming_removal); try std.testing.expectEqual(@as(?usize, null), component.pending_removal_index); try std.testing.expect(component.pending_removal_path == null); - try std.testing.expectEqual(ExpandingOverlay.State.Collapsing, component.overlay.state); + try std.testing.expectEqual(ExpandingOverlay.State.Closed, component.overlay.state); + try std.testing.expect(!component.overlay.isAnimating()); + try std.testing.expectEqual(@as(i64, 0), component.flow_animation_start_ms); + + host.now_ms = 150; + host.focused_cwd = null; + host.focused_has_foreground_process = false; + WorktreeOverlayComponent.update(&component, &host, &actions); + try std.testing.expectEqual(ExpandingOverlay.State.Closed, component.overlay.state); } From beb12c2813e353d8b663d7dbe4f8e81e81540449 Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Thu, 3 Sep 2026 16:34:45 +0200 Subject: [PATCH 6/8] fix(ui): prevent animated pill hit-test overlap --- README.md | 2 +- docs/ARCHITECTURE.md | 3 +- src/ui/components/pill_group.zig | 38 +++++++++++++++++++- src/ui/components/pr_dropdown.zig | 12 +++++-- src/ui/components/recent_folders_overlay.zig | 12 +++++-- src/ui/components/worktree_overlay.zig | 12 +++++-- 6 files changed, 67 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index e40f2a15..8027561c 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,7 @@ Architect solves this with a grid view that keeps all your agents visible, with ### Terminal Essentials - Smooth animated transitions for grid expansion, contraction, and reflow (cells and borders move/resize together) -- Context-aware shortcut pills stay right-aligned with fixed spacing, compact when a pill is unavailable, and animate into their new positions; newly available pills enter after the existing row reflows to avoid overlapping hit targets; the worktree, recent-folder, and pull-request pills are hidden while the focused terminal runs a foreground process and any open picker closes immediately +- Context-aware shortcut pills stay right-aligned with fixed spacing, compact when a pill is unavailable, and animate into their new positions; newly available pills enter after the existing row reflows to avoid overlapping hit targets and remain non-interactive until they reach a clear slot; the worktree, recent-folder, and pull-request pills are hidden while the focused terminal runs a foreground process and any open picker closes immediately - Wakeable idle input handling keeps typing responsive after short idle periods instead of waiting on a fixed sleep window - Keyboard navigation: ⌘+Return to expand, ⌘1–⌘0 to switch grid slots, ⌘Arrow to move focus in grid view (plays a brief wave animation on the destination terminal), ⌘N to add, ⌘W to close a terminal (restarts if it's the only terminal), ⌘T for worktrees, ⌘O for recent folders, ⌘D for repo-wide git diff (staged + unstaged + untracked), ⌘R for reader mode, ⌘/ for shortcuts; quit with ⌘Q or the window close button - Every text field (recent-folder and reader/story search, worktree name, diff comments, selection-agent instructions) behaves like a macOS text field: a blinking caret, Backspace deletes one character, ⌥Backspace the previous word, ⌘Backspace the whole field, ⌘A selects everything (the next keystroke or paste replaces it), and ⌘C/⌘V copy and paste. The selection-agent field accepts multiline instructions with ⇧↩; plain ↩ launches, while Escape or Cancel dismisses it. Pasting multi-line text into a one-line field drops the newlines, and text longer than the field fades out at its edge instead of spilling past it. Emoji are scaled to the surrounding line height wherever they appear (search fields, comments, reader mode, stories, selection-agent instructions) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 18e80ac3..6ad82cd7 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -357,7 +357,8 @@ easing function. Newly available pills remain staged outside the row while existing pills reflow, then enter in a second eased phase once the occupied slots are clear. The group also requests frames for the short layout animation, so the -transition is visible under idle throttling. The worktree, recent-folder, and +transition is visible under idle throttling; newly entering pill hit targets are +disabled until they reach their clear slots. The worktree, recent-folder, and pull-request pills are unavailable while the focused terminal has a foreground process, because their actions send commands to that shell. Their event handlers reject input at the same boundary, before the scheduled update diff --git a/src/ui/components/pill_group.zig b/src/ui/components/pill_group.zig index e3e62e9e..dd291fef 100644 --- a/src/ui/components/pill_group.zig +++ b/src/ui/components/pill_group.zig @@ -72,7 +72,7 @@ const PillLayout = struct { self.target_x = targets; const entrance_x = window_w + dpi.scale(pill_size, ui_scale); for (0..pill_count) |idx| { - const stage = visible[idx] and !self.visible[idx]; + const stage = visible[idx] and (self.entering[idx] or !self.visible[idx]); self.entering[idx] = stage; if (stage) { self.start_x[idx] = entrance_x; @@ -169,6 +169,11 @@ const PillLayout = struct { fn currentX(self: *const PillLayout, pill: PillKind) c_int { return self.current_x[@intFromEnum(pill)]; } + + fn pillInteractive(self: *const PillLayout, pill: PillKind) bool { + const idx = @intFromEnum(pill); + return self.visible[idx] and !self.entering[idx]; + } }; pub const PillGroupComponent = struct { @@ -303,8 +308,11 @@ pub const PillGroupComponent = struct { fn applyLayout(self: *PillGroupComponent) void { self.pr_dropdown.overlay.setLayoutX(self.layout.currentX(.pull_request)); + self.pr_dropdown.setPillInteractive(self.layout.pillInteractive(.pull_request)); self.worktree.overlay.setLayoutX(self.layout.currentX(.worktree)); + self.worktree.setPillInteractive(self.layout.pillInteractive(.worktree)); self.recent_folders.overlay.setLayoutX(self.layout.currentX(.recent_folders)); + self.recent_folders.setPillInteractive(self.layout.pillInteractive(.recent_folders)); self.help.overlay.setLayoutX(self.layout.currentX(.help)); } @@ -423,3 +431,31 @@ test "pill layout stages a newly available pill outside the occupied row" { try std.testing.expectEqual(@as(c_int, 680), layout.currentX(.recent_folders)); try std.testing.expectEqual(@as(c_int, 620), layout.currentX(.pull_request)); } + +test "pill layout disables hit targets while positions are moving" { + var layout: PillLayout = .{}; + const all_visible = [pill_count]bool{ true, true, true, true }; + _ = layout.update(0, 800, 1.0, all_visible); + + const without_worktree = [pill_count]bool{ true, false, true, true }; + _ = layout.update(0, 800, 1.0, without_worktree); + try std.testing.expect(layout.pillInteractive(.pull_request)); + try std.testing.expect(layout.pillInteractive(.recent_folders)); + try std.testing.expect(layout.pillInteractive(.help)); + + _ = layout.update(pill_animation_duration_ms, 800, 1.0, without_worktree); + try std.testing.expect(layout.pillInteractive(.pull_request)); + try std.testing.expect(layout.pillInteractive(.recent_folders)); + try std.testing.expect(layout.pillInteractive(.help)); + + const with_worktree = [pill_count]bool{ true, true, true, true }; + _ = layout.update(pill_animation_duration_ms, 800, 1.0, with_worktree); + try std.testing.expect(!layout.pillInteractive(.worktree)); + try std.testing.expect(layout.pillInteractive(.pull_request)); + try std.testing.expect(layout.pillInteractive(.recent_folders)); + + _ = layout.update(pill_animation_duration_ms + 50, 800, 1.0, with_worktree); + try std.testing.expect(!layout.pillInteractive(.worktree)); + _ = layout.update(pill_animation_duration_ms * 3, 800, 1.0, with_worktree); + try std.testing.expect(layout.pillInteractive(.worktree)); +} diff --git a/src/ui/components/pr_dropdown.zig b/src/ui/components/pr_dropdown.zig index aee0e34e..a907cb62 100644 --- a/src/ui/components/pr_dropdown.zig +++ b/src/ui/components/pr_dropdown.zig @@ -84,6 +84,7 @@ pub const PRDropdownComponent = struct { cache: ?*view.Cache = null, escape_pressed: bool = false, focused_busy: bool = false, + pill_interactive: bool = true, flow_animation_start_ms: i64 = 0, pub const button_size_small: c_int = 40; @@ -143,6 +144,8 @@ pub const PRDropdownComponent = struct { fn handleEvent(self_ptr: *anyopaque, host: *const types.UiHost, event: *const c.SDL_Event, actions: *types.UiActionQueue) bool { const self: *PRDropdownComponent = @ptrCast(@alignCast(self_ptr)); + if (!self.pillVisible(host) or !self.pill_interactive) return false; + if (event.type == c.SDL_EVENT_KEY_UP and self.escape_pressed) { const key = event.key.key; if (key == c.SDLK_ESCAPE) { @@ -151,8 +154,6 @@ pub const PRDropdownComponent = struct { } } - if (!self.pillVisible(host)) return false; - switch (event.type) { c.SDL_EVENT_KEY_DOWN => { const key = event.key.key; @@ -282,7 +283,7 @@ pub const PRDropdownComponent = struct { fn hitTest(self_ptr: *anyopaque, host: *const types.UiHost, x: c_int, y: c_int) bool { const self: *PRDropdownComponent = @ptrCast(@alignCast(self_ptr)); - if (!self.pillVisible(host)) return false; + if (!self.pillVisible(host) or !self.pill_interactive) return false; const rect = self.overlay.rect(host.now_ms, host.window_w, host.window_h, host.ui_scale); return geom.containsPoint(rect, x, y); } @@ -295,6 +296,11 @@ pub const PRDropdownComponent = struct { return shouldShowPill(self.is_github_repo, host.focused_has_foreground_process); } + pub fn setPillInteractive(self: *PRDropdownComponent, interactive: bool) void { + self.pill_interactive = interactive; + if (!interactive) self.escape_pressed = false; + } + fn update(self_ptr: *anyopaque, host: *const types.UiHost, _: *types.UiActionQueue) void { const self: *PRDropdownComponent = @ptrCast(@alignCast(self_ptr)); diff --git a/src/ui/components/recent_folders_overlay.zig b/src/ui/components/recent_folders_overlay.zig index 955b038d..29fa891a 100644 --- a/src/ui/components/recent_folders_overlay.zig +++ b/src/ui/components/recent_folders_overlay.zig @@ -31,6 +31,7 @@ pub const RecentFoldersOverlayComponent = struct { hovered_entry: ?usize = null, escape_pressed: bool = false, focused_busy: bool = false, + pill_interactive: bool = true, cache: ?*Cache = null, flow_animation_start_ms: i64 = 0, @@ -152,6 +153,8 @@ pub const RecentFoldersOverlayComponent = struct { fn handleEvent(self_ptr: *anyopaque, host: *const types.UiHost, event: *const c.SDL_Event, actions: *types.UiActionQueue) bool { const self: *RecentFoldersOverlayComponent = @ptrCast(@alignCast(self_ptr)); + if (!self.pillVisible(host) or !self.pill_interactive) return false; + if (event.type == c.SDL_EVENT_KEY_UP and self.escape_pressed) { const key = event.key.key; if (key == c.SDLK_ESCAPE) { @@ -160,8 +163,6 @@ pub const RecentFoldersOverlayComponent = struct { } } - if (!self.pillVisible(host)) return false; - switch (event.type) { c.SDL_EVENT_MOUSE_BUTTON_DOWN => { const mouse_x: c_int = @intFromFloat(event.button.x); @@ -314,7 +315,7 @@ pub const RecentFoldersOverlayComponent = struct { fn hitTest(self_ptr: *anyopaque, host: *const types.UiHost, x: c_int, y: c_int) bool { const self: *RecentFoldersOverlayComponent = @ptrCast(@alignCast(self_ptr)); - if (!self.pillVisible(host)) return false; + if (!self.pillVisible(host) or !self.pill_interactive) return false; const rect = self.overlay.rect(host.now_ms, host.window_w, host.window_h, host.ui_scale); return geom.containsPoint(rect, x, y); } @@ -327,6 +328,11 @@ pub const RecentFoldersOverlayComponent = struct { return shouldShowPill(self.all_folders.items.len, host.focused_has_foreground_process); } + pub fn setPillInteractive(self: *RecentFoldersOverlayComponent, interactive: bool) void { + self.pill_interactive = interactive; + if (!interactive) self.escape_pressed = false; + } + fn update(self_ptr: *anyopaque, host: *const types.UiHost, _: *types.UiActionQueue) void { const self: *RecentFoldersOverlayComponent = @ptrCast(@alignCast(self_ptr)); diff --git a/src/ui/components/worktree_overlay.zig b/src/ui/components/worktree_overlay.zig index 1c890537..8592823f 100644 --- a/src/ui/components/worktree_overlay.zig +++ b/src/ui/components/worktree_overlay.zig @@ -37,6 +37,7 @@ pub const WorktreeOverlayComponent = struct { pending_removal_path: ?[]const u8 = null, pending_refresh_ms: i64 = 0, escape_pressed: bool = false, + pill_interactive: bool = true, create_input: text_edit.TextInput = .{ .separators = text_edit.name_separators, .max_len = create_name_max_len, @@ -133,6 +134,8 @@ pub const WorktreeOverlayComponent = struct { fn handleEvent(self_ptr: *anyopaque, host: *const types.UiHost, event: *const c.SDL_Event, actions: *types.UiActionQueue) bool { const self: *WorktreeOverlayComponent = @ptrCast(@alignCast(self_ptr)); + if (!self.pillVisible(host) or !self.pill_interactive) return false; + if (event.type == c.SDL_EVENT_KEY_UP and self.escape_pressed) { const key = event.key.key; if (key == c.SDLK_ESCAPE) { @@ -141,8 +144,6 @@ pub const WorktreeOverlayComponent = struct { } } - if (!self.available or host.focused_has_foreground_process) return false; - switch (event.type) { c.SDL_EVENT_MOUSE_BUTTON_DOWN => { if (self.creating) { @@ -268,7 +269,7 @@ pub const WorktreeOverlayComponent = struct { fn hitTest(self_ptr: *anyopaque, host: *const types.UiHost, x: c_int, y: c_int) bool { const self: *WorktreeOverlayComponent = @ptrCast(@alignCast(self_ptr)); - if (!self.pillVisible(host)) return false; + if (!self.pillVisible(host) or !self.pill_interactive) return false; const rect = self.overlay.rect(host.now_ms, host.window_w, host.window_h, host.ui_scale); return geom.containsPoint(rect, x, y); } @@ -281,6 +282,11 @@ pub const WorktreeOverlayComponent = struct { return shouldShowPill(self.available, host.focused_has_foreground_process); } + pub fn setPillInteractive(self: *WorktreeOverlayComponent, interactive: bool) void { + self.pill_interactive = interactive; + if (!interactive) self.escape_pressed = false; + } + fn update(self_ptr: *anyopaque, host: *const types.UiHost, _: *types.UiActionQueue) void { const self: *WorktreeOverlayComponent = @ptrCast(@alignCast(self_ptr)); From daded192a52ac09426d5e6693b98a920f6341f41 Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Thu, 3 Sep 2026 16:39:59 +0200 Subject: [PATCH 7/8] fix(ui): preserve pill entrance progress --- docs/ARCHITECTURE.md | 3 +++ src/ui/components/pill_group.zig | 22 ++++++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 6ad82cd7..3770150c 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -356,6 +356,9 @@ the existing pills interpolate to their new positions with the shared cubic easing function. Newly available pills remain staged outside the row while existing pills reflow, then enter in a second eased phase once the occupied slots are clear. +If another availability or geometry change arrives during an entrance, the +group keeps the entrant's current interpolated position instead of restarting +it offscreen. The group also requests frames for the short layout animation, so the transition is visible under idle throttling; newly entering pill hit targets are disabled until they reach their clear slots. The worktree, recent-folder, and diff --git a/src/ui/components/pill_group.zig b/src/ui/components/pill_group.zig index dd291fef..f51e9f0c 100644 --- a/src/ui/components/pill_group.zig +++ b/src/ui/components/pill_group.zig @@ -72,9 +72,10 @@ const PillLayout = struct { self.target_x = targets; const entrance_x = window_w + dpi.scale(pill_size, ui_scale); for (0..pill_count) |idx| { - const stage = visible[idx] and (self.entering[idx] or !self.visible[idx]); + const was_entering = self.entering[idx]; + const stage = visible[idx] and (was_entering or !self.visible[idx]); self.entering[idx] = stage; - if (stage) { + if (stage and !was_entering) { self.start_x[idx] = entrance_x; self.current_x[idx] = entrance_x; } @@ -459,3 +460,20 @@ test "pill layout disables hit targets while positions are moving" { _ = layout.update(pill_animation_duration_ms * 3, 800, 1.0, with_worktree); try std.testing.expect(layout.pillInteractive(.worktree)); } + +test "pill layout preserves an entrance position across a second layout change" { + var layout: PillLayout = .{}; + const only_help = [pill_count]bool{ false, false, false, true }; + _ = layout.update(0, 800, 1.0, only_help); + + const with_pull_request = [pill_count]bool{ true, false, false, true }; + _ = layout.update(0, 800, 1.0, with_pull_request); + _ = layout.update(250, 800, 1.0, with_pull_request); + const position_before_second_change = layout.currentX(.pull_request); + try std.testing.expect(position_before_second_change < 840); + + const with_recent_folders = [pill_count]bool{ true, false, true, true }; + _ = layout.update(250, 800, 1.0, with_recent_folders); + try std.testing.expectEqual(position_before_second_change, layout.currentX(.pull_request)); + try std.testing.expect(!layout.pillInteractive(.pull_request)); +} From 281541612e730e23573b5695e7c2f0320bf9b7e4 Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Thu, 3 Sep 2026 16:50:41 +0200 Subject: [PATCH 8/8] fix(ui): guard pill input transitions --- docs/ARCHITECTURE.md | 3 + src/ui/components/pill_group.zig | 88 +++++++++++++++++++- src/ui/components/pr_dropdown.zig | 19 +++-- src/ui/components/recent_folders_overlay.zig | 18 ++-- src/ui/components/worktree_overlay.zig | 15 ++-- 5 files changed, 126 insertions(+), 17 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 3770150c..048e429d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -370,6 +370,9 @@ consumed by stale UI. A busy or otherwise unavailable transition closes the recent-folder, PR, and worktree pickers immediately, resetting their transient state instead of exposing a partial collapse. It also clears any pending worktree removal confirmation before the modal is hidden. +The group consumes pointer events over an entering pill so an overlapping +interactive pill underneath cannot receive the click. Pending Escape releases +remain consumed even if a busy transition closes the picker before key-up. ### External Notification Path diff --git a/src/ui/components/pill_group.zig b/src/ui/components/pill_group.zig index f51e9f0c..1da63f69 100644 --- a/src/ui/components/pill_group.zig +++ b/src/ui/components/pill_group.zig @@ -1,5 +1,6 @@ const std = @import("std"); const c = @import("../../c.zig"); +const geom = @import("../../geom.zig"); const types = @import("../types.zig"); const UiComponent = @import("../component.zig").UiComponent; const HelpOverlayComponent = @import("help_overlay.zig").HelpOverlayComponent; @@ -175,6 +176,10 @@ const PillLayout = struct { const idx = @intFromEnum(pill); return self.visible[idx] and !self.entering[idx]; } + + fn isEntering(self: *const PillLayout, pill: PillKind) bool { + return self.entering[@intFromEnum(pill)]; + } }; pub const PillGroupComponent = struct { @@ -189,6 +194,7 @@ pub const PillGroupComponent = struct { last_recent_folders_state: ExpandingOverlay.State = .Closed, last_worktree_state: ExpandingOverlay.State = .Closed, last_pr_state: ExpandingOverlay.State = .Closed, + pointer_blocked: bool = false, pub const component_z_index: i32 = 1001; @@ -220,11 +226,56 @@ pub const PillGroupComponent = struct { self.allocator.destroy(self); } - fn handleEvent(_: *anyopaque, _: *const types.UiHost, _: *const c.SDL_Event, _: *types.UiActionQueue) bool { + fn handleEvent(self_ptr: *anyopaque, host: *const types.UiHost, event: *const c.SDL_Event, _: *types.UiActionQueue) bool { + const self: *PillGroupComponent = @ptrCast(@alignCast(self_ptr)); + + switch (event.type) { + c.SDL_EVENT_MOUSE_BUTTON_DOWN => { + const mouse_x: c_int = @intFromFloat(event.button.x); + const mouse_y: c_int = @intFromFloat(event.button.y); + if (self.containsEnteringPill(host, mouse_x, mouse_y)) { + self.pointer_blocked = true; + return true; + } + }, + c.SDL_EVENT_MOUSE_BUTTON_UP => { + if (self.pointer_blocked) { + self.pointer_blocked = false; + return true; + } + }, + c.SDL_EVENT_MOUSE_MOTION => { + if (self.pointer_blocked) return true; + const mouse_x: c_int = @intFromFloat(event.motion.x); + const mouse_y: c_int = @intFromFloat(event.motion.y); + return self.containsEnteringPill(host, mouse_x, mouse_y); + }, + else => {}, + } + return false; } - fn hitTest(_: *anyopaque, _: *const types.UiHost, _: c_int, _: c_int) bool { + fn hitTest(self_ptr: *anyopaque, host: *const types.UiHost, x: c_int, y: c_int) bool { + const self: *PillGroupComponent = @ptrCast(@alignCast(self_ptr)); + return self.containsEnteringPill(host, x, y); + } + + fn containsEnteringPill(self: *const PillGroupComponent, host: *const types.UiHost, x: c_int, y: c_int) bool { + const scaled_pill_size = dpi.scale(pill_size, host.ui_scale); + const scaled_pill_margin = dpi.scale(pill_margin, host.ui_scale); + + for (layout_order) |pill| { + if (!self.layout.isEntering(pill)) continue; + const rect = geom.Rect{ + .x = self.layout.currentX(pill), + .y = scaled_pill_margin, + .w = scaled_pill_size, + .h = scaled_pill_size, + }; + if (geom.containsPoint(rect, x, y)) return true; + } + return false; } @@ -477,3 +528,36 @@ test "pill layout preserves an entrance position across a second layout change" try std.testing.expectEqual(position_before_second_change, layout.currentX(.pull_request)); try std.testing.expect(!layout.pillInteractive(.pull_request)); } + +test "pill group blocks clicks on entering pills" { + var group = PillGroupComponent{ + .allocator = std.testing.allocator, + .help = undefined, + .recent_folders = undefined, + .worktree = undefined, + .pr_dropdown = undefined, + }; + group.layout.initialized = true; + group.layout.visible = [pill_count]bool{ true, false, false, true }; + group.layout.entering = [pill_count]bool{ true, false, false, false }; + group.layout.current_x = [pill_count]c_int{ 760, 0, 0, 740 }; + + var host: types.UiHost = undefined; + host.ui_scale = 1.0; + var actions = types.UiActionQueue.init(std.testing.allocator); + defer actions.deinit(); + + var event: c.SDL_Event = undefined; + event.type = c.SDL_EVENT_MOUSE_BUTTON_DOWN; + event.button.x = 770; + event.button.y = 30; + try std.testing.expect(PillGroupComponent.handleEvent(&group, &host, &event, &actions)); + try std.testing.expect(group.pointer_blocked); + + group.layout.entering = [_]bool{false} ** pill_count; + event.type = c.SDL_EVENT_MOUSE_BUTTON_UP; + event.button.x = 500; + event.button.y = 500; + try std.testing.expect(PillGroupComponent.handleEvent(&group, &host, &event, &actions)); + try std.testing.expect(!group.pointer_blocked); +} diff --git a/src/ui/components/pr_dropdown.zig b/src/ui/components/pr_dropdown.zig index a907cb62..bf320eb3 100644 --- a/src/ui/components/pr_dropdown.zig +++ b/src/ui/components/pr_dropdown.zig @@ -144,8 +144,6 @@ pub const PRDropdownComponent = struct { fn handleEvent(self_ptr: *anyopaque, host: *const types.UiHost, event: *const c.SDL_Event, actions: *types.UiActionQueue) bool { const self: *PRDropdownComponent = @ptrCast(@alignCast(self_ptr)); - if (!self.pillVisible(host) or !self.pill_interactive) return false; - if (event.type == c.SDL_EVENT_KEY_UP and self.escape_pressed) { const key = event.key.key; if (key == c.SDLK_ESCAPE) { @@ -154,6 +152,8 @@ pub const PRDropdownComponent = struct { } } + if (!self.pillVisible(host) or !self.pill_interactive) return false; + switch (event.type) { c.SDL_EVENT_KEY_DOWN => { const key = event.key.key; @@ -298,7 +298,6 @@ pub const PRDropdownComponent = struct { pub fn setPillInteractive(self: *PRDropdownComponent, interactive: bool) void { self.pill_interactive = interactive; - if (!interactive) self.escape_pressed = false; } fn update(self_ptr: *anyopaque, host: *const types.UiHost, _: *types.UiActionQueue) void { @@ -442,7 +441,6 @@ pub const PRDropdownComponent = struct { self.search_query.clear(); self.refilter(); self.hovered_entry = null; - self.escape_pressed = false; self.flow_animation_start_ms = 0; } @@ -790,6 +788,13 @@ fn keyEvent(key: c.SDL_Keycode, mod: c.SDL_Keymod) c.SDL_Event { return event; } +fn keyUpEvent(key: c.SDL_Keycode) c.SDL_Event { + var event: c.SDL_Event = undefined; + event.type = c.SDL_EVENT_KEY_UP; + event.key.key = key; + return event; +} + fn textEvent(text: [*c]const u8) c.SDL_Event { var event: c.SDL_Event = undefined; event.type = c.SDL_EVENT_TEXT_INPUT; @@ -839,9 +844,13 @@ test "busy update immediately closes the PR picker" { try testing.expect(!component.overlay.isAnimating()); try testing.expectEqualStrings("", component.search_query.text()); try testing.expectEqual(@as(?usize, null), component.hovered_entry); - try testing.expect(!component.escape_pressed); + try testing.expect(component.escape_pressed); try testing.expectEqual(@as(i64, 0), component.flow_animation_start_ms); + var escape_release = keyUpEvent(c.SDLK_ESCAPE); + try testing.expect(PRDropdownComponent.handleEvent(&component, &host, &escape_release, &actions)); + try testing.expect(!component.escape_pressed); + host.now_ms = 250; host.focused_has_foreground_process = false; PRDropdownComponent.update(&component, &host, &actions); diff --git a/src/ui/components/recent_folders_overlay.zig b/src/ui/components/recent_folders_overlay.zig index 29fa891a..42240caa 100644 --- a/src/ui/components/recent_folders_overlay.zig +++ b/src/ui/components/recent_folders_overlay.zig @@ -153,8 +153,6 @@ pub const RecentFoldersOverlayComponent = struct { fn handleEvent(self_ptr: *anyopaque, host: *const types.UiHost, event: *const c.SDL_Event, actions: *types.UiActionQueue) bool { const self: *RecentFoldersOverlayComponent = @ptrCast(@alignCast(self_ptr)); - if (!self.pillVisible(host) or !self.pill_interactive) return false; - if (event.type == c.SDL_EVENT_KEY_UP and self.escape_pressed) { const key = event.key.key; if (key == c.SDLK_ESCAPE) { @@ -163,6 +161,8 @@ pub const RecentFoldersOverlayComponent = struct { } } + if (!self.pillVisible(host) or !self.pill_interactive) return false; + switch (event.type) { c.SDL_EVENT_MOUSE_BUTTON_DOWN => { const mouse_x: c_int = @intFromFloat(event.button.x); @@ -309,7 +309,6 @@ pub const RecentFoldersOverlayComponent = struct { self.search.clear(); self.refilter(); self.hovered_entry = null; - self.escape_pressed = false; self.flow_animation_start_ms = 0; } @@ -330,7 +329,6 @@ pub const RecentFoldersOverlayComponent = struct { pub fn setPillInteractive(self: *RecentFoldersOverlayComponent, interactive: bool) void { self.pill_interactive = interactive; - if (!interactive) self.escape_pressed = false; } fn update(self_ptr: *anyopaque, host: *const types.UiHost, _: *types.UiActionQueue) void { @@ -892,6 +890,13 @@ fn keyEvent(key: c.SDL_Keycode, mod: c.SDL_Keymod) c.SDL_Event { return event; } +fn keyUpEvent(key: c.SDL_Keycode) c.SDL_Event { + var event: c.SDL_Event = undefined; + event.type = c.SDL_EVENT_KEY_UP; + event.key.key = key; + return event; +} + fn textEvent(text: [*c]const u8) c.SDL_Event { var event: c.SDL_Event = undefined; event.type = c.SDL_EVENT_TEXT_INPUT; @@ -1095,9 +1100,12 @@ test "busy update immediately closes the recent folders picker" { try testing.expect(!t.comp.overlay.isAnimating()); try testing.expectEqualStrings("", t.comp.search.text()); try testing.expectEqual(@as(?usize, null), t.comp.hovered_entry); - try testing.expect(!t.comp.escape_pressed); + try testing.expect(t.comp.escape_pressed); try testing.expectEqual(@as(i64, 0), t.comp.flow_animation_start_ms); + try testing.expect(t.sendWithBusy(keyUpEvent(c.SDLK_ESCAPE), 200, true)); + try testing.expect(!t.comp.escape_pressed); + host.now_ms = 250; host.focused_has_foreground_process = false; RecentFoldersOverlayComponent.update(&t.comp, &host, &t.actions); diff --git a/src/ui/components/worktree_overlay.zig b/src/ui/components/worktree_overlay.zig index 8592823f..a9c19d2c 100644 --- a/src/ui/components/worktree_overlay.zig +++ b/src/ui/components/worktree_overlay.zig @@ -134,8 +134,6 @@ pub const WorktreeOverlayComponent = struct { fn handleEvent(self_ptr: *anyopaque, host: *const types.UiHost, event: *const c.SDL_Event, actions: *types.UiActionQueue) bool { const self: *WorktreeOverlayComponent = @ptrCast(@alignCast(self_ptr)); - if (!self.pillVisible(host) or !self.pill_interactive) return false; - if (event.type == c.SDL_EVENT_KEY_UP and self.escape_pressed) { const key = event.key.key; if (key == c.SDLK_ESCAPE) { @@ -144,6 +142,8 @@ pub const WorktreeOverlayComponent = struct { } } + if (!self.pillVisible(host) or !self.pill_interactive) return false; + switch (event.type) { c.SDL_EVENT_MOUSE_BUTTON_DOWN => { if (self.creating) { @@ -284,7 +284,6 @@ pub const WorktreeOverlayComponent = struct { pub fn setPillInteractive(self: *WorktreeOverlayComponent, interactive: bool) void { self.pill_interactive = interactive; - if (!interactive) self.escape_pressed = false; } fn update(self_ptr: *anyopaque, host: *const types.UiHost, _: *types.UiActionQueue) void { @@ -582,7 +581,6 @@ pub const WorktreeOverlayComponent = struct { self.hovered_entry = null; self.clearCreateInput(); self.creating = false; - self.escape_pressed = false; self.setDisplayBase(cwd); @@ -1051,7 +1049,6 @@ pub const WorktreeOverlayComponent = struct { self.confirming_removal = false; self.clearCreateInput(); self.clearPendingRemoval(); - self.escape_pressed = false; self.flow_animation_start_ms = 0; self.modal_confirm_hovered = false; self.modal_cancel_hovered = false; @@ -1696,6 +1693,7 @@ test "busy transition clears the removal confirmation modal" { component.available = true; component.overlay.state = .Open; component.confirming_removal = true; + component.escape_pressed = true; component.pending_removal_index = 0; component.pending_removal_path = try std.testing.allocator.dupe(u8, "/tmp/architect-worktree"); component.flow_animation_start_ms = 123; @@ -1714,6 +1712,13 @@ test "busy transition clears the removal confirmation modal" { try std.testing.expectEqual(ExpandingOverlay.State.Closed, component.overlay.state); try std.testing.expect(!component.overlay.isAnimating()); try std.testing.expectEqual(@as(i64, 0), component.flow_animation_start_ms); + try std.testing.expect(component.escape_pressed); + + var escape_release: c.SDL_Event = undefined; + escape_release.type = c.SDL_EVENT_KEY_UP; + escape_release.key.key = c.SDLK_ESCAPE; + try std.testing.expect(WorktreeOverlayComponent.handleEvent(&component, &host, &escape_release, &actions)); + try std.testing.expect(!component.escape_pressed); host.now_ms = 150; host.focused_cwd = null;