diff --git a/.github/swift-file-length-budget.tsv b/.github/swift-file-length-budget.tsv index cca9d8a55ab1..29a3dbe35053 100644 --- a/.github/swift-file-length-budget.tsv +++ b/.github/swift-file-length-budget.tsv @@ -3,7 +3,7 @@ # Reduce counts as files shrink. CI fails if tracked files exceed this budget. 35469 CLI/cmux.swift 17854 Sources/AppDelegate.swift -16274 Sources/ContentView.swift +16252 Sources/ContentView.swift 15095 Sources/TerminalController.swift 13161 Sources/Workspace.swift 12501 Sources/GhosttyTerminalView.swift diff --git a/Resources/Localizable.xcstrings b/Resources/Localizable.xcstrings index d1ca090fa4d1..150a42d3caab 100644 --- a/Resources/Localizable.xcstrings +++ b/Resources/Localizable.xcstrings @@ -197014,6 +197014,23 @@ } } }, + "sidebar.checklist.removeItemTooltip": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Remove item" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "項目を削除" + } + } + } + }, "sidebar.checklist.uncheckTooltip": { "extractionState": "manual", "localizations": { diff --git a/Sources/ChecklistPopoverPointerTracking.swift b/Sources/ChecklistPopoverPointerTracking.swift new file mode 100644 index 000000000000..316027786fae --- /dev/null +++ b/Sources/ChecklistPopoverPointerTracking.swift @@ -0,0 +1,103 @@ +import AppKit +import SwiftUI +/// Item-row frames in ``SidebarWorkspaceChecklistPopover``'s pointer +/// coordinate space, keyed by item id. Feeds the geometry-derived +/// `hoveredItemId` (see its doc comment). +struct ChecklistPopoverRowFramesKey: PreferenceKey { + static var defaultValue: [UUID: CGRect] { [:] } + static func reduce(value: inout [UUID: CGRect], nextValue: () -> [UUID: CGRect]) { + value.merge(nextValue()) { _, new in new } + } +} + +/// AppKit-owned pointer tracking for the checklist popover: one +/// NSTrackingArea on a background NSView that fills the popover content, +/// reporting the pointer's location in that view's (flipped, top-left +/// origin) coordinates — directly comparable to the row frames collected in +/// the popover's named coordinate space. `nil` means the pointer left the +/// popover. +/// +/// Why not SwiftUI `.onContinuousHover`/`.onHover`: their tracking areas are +/// torn down and recreated whenever the owning view updates, so the first +/// mouse event after a checklist mutation can be a spurious `.ended` from +/// the stale area with no follow-up `.active` until the NEXT event — the +/// pointer-resting delete-x dropout. This view's backing NSView persists +/// across SwiftUI content updates, so its tracking area only changes with +/// geometry (`updateTrackingAreas`), never with content. +/// +/// Also seeds the location from `NSEvent.mouseLocation` at window attach, so +/// a popover that (re)presents underneath an already-resting pointer knows +/// where it is before any mouse-moved arrives. +struct PopoverPointerTracker: NSViewRepresentable { + let onPointerChange: @MainActor (CGPoint?) -> Void + + final class TrackerView: NSView { + var onPointerChange: (@MainActor (CGPoint?) -> Void)? + + // SwiftUI's named coordinate space is top-left origin; matching that + // here keeps reported points directly comparable to the row frames + // collected via preference. + override var isFlipped: Bool { true } + + // Tracking areas fire from geometry alone; this view must never win + // hit testing over the SwiftUI buttons/fields/gestures it sits + // behind (same contract as `HoverTrackingNSView`). + override func hitTest(_ point: NSPoint) -> NSView? { nil } + + override func updateTrackingAreas() { + super.updateTrackingAreas() + trackingAreas.forEach(removeTrackingArea) + addTrackingArea(NSTrackingArea( + rect: .zero, + // `.activeAlways` so hover keeps working when the terminal + // pane steals key/main status back from the popover window. + options: [.mouseEnteredAndExited, .mouseMoved, .activeAlways, .inVisibleRect], + owner: self, + userInfo: nil + )) + } + + override func viewDidMoveToWindow() { + super.viewDidMoveToWindow() + guard window != nil else { return } + updateTrackingAreas() + reportCurrentPointerLocation() + } + + override func mouseEntered(with event: NSEvent) { + report(event) + } + + override func mouseMoved(with event: NSEvent) { + report(event) + } + + override func mouseExited(with event: NSEvent) { + onPointerChange?(nil) + } + + private func report(_ event: NSEvent) { + let local = convert(event.locationInWindow, from: nil) + onPointerChange?(local) + } + + /// Reads the pointer position directly (no event needed) — used to + /// seed hover state when the popover attaches under a resting pointer. + private func reportCurrentPointerLocation() { + guard let window else { return } + let windowPoint = window.convertPoint(fromScreen: NSEvent.mouseLocation) + let local = convert(windowPoint, from: nil) + onPointerChange?(bounds.contains(local) ? local : nil) + } + } + + func makeNSView(context: Context) -> NSView { + let view = TrackerView() + view.onPointerChange = onPointerChange + return view + } + + func updateNSView(_ nsView: NSView, context: Context) { + (nsView as? TrackerView)?.onPointerChange = onPointerChange + } +} diff --git a/Sources/ChecklistSummaryPopoverModifier.swift b/Sources/ChecklistSummaryPopoverModifier.swift new file mode 100644 index 000000000000..964bca91defa --- /dev/null +++ b/Sources/ChecklistSummaryPopoverModifier.swift @@ -0,0 +1,67 @@ +import AppKit +import CmuxWorkspaces +import SwiftUI +/// Attaches the checklist popover to the section container (not just the +/// summary line — see the call site comment in ``SidebarWorkspaceChecklistSection/body``) +/// via `SidebarWorkspaceTodoPopoverHost` (a real NSPopover, not SwiftUI's +/// native `.popover()`): the checklist popover embeds a first-responder +/// TextField (the add/edit fields), and SwiftUI's native `.popover` does not +/// make its window key in cmux's focus-managed environment, so keystrokes +/// fall through to the terminal. +/// +/// The anchor NSView is pinned to a fixed 1×1pt corner via `.overlay` rather +/// than spanning the whole section via `.background()`: an +/// NSViewRepresentable stacked as a `.background()` behind the full row was +/// found to suppress `.onHover` tracking for the item rows underneath it +/// (the hover-reveal delete "x" stopped appearing reliably). Shrinking its +/// footprint to a single corner point removes it from the row's +/// hit-testing/hover area while keeping one stable, always-present anchor +/// across the 0→1 item transition. +struct ChecklistSummaryPopoverModifier: ViewModifier { + @Binding var isPresented: Bool + let model: SidebarWorkspaceChecklistPopoverModel + let actions: SidebarWorkspaceChecklistActions + let onConsumeAddFieldActivation: () -> Void + let onPopoverPresentedChange: @MainActor (Bool) -> Void + + func body(content: Content) -> some View { + content + // `.overlay(alignment: .topTrailing)` positions the anchor within + // `content`'s own bounding box. For a zero-item workspace in + // popover style, `content` (the section's VStack) renders no + // children at all, so its natural size is 0×0 — topTrailing then + // collapses to a single point at the VStack's own position, which + // its leading-aligned parent places at the row's LEFT edge, not + // the right edge. `maxWidth: .infinity` forces this container to + // always claim the row's full width regardless of content, so + // the anchor's trailing edge always matches the row's actual + // right edge, whether or not any items exist yet. + .frame(maxWidth: .infinity, alignment: .leading) + .overlay(alignment: .topTrailing) { + SidebarWorkspaceTodoPopoverHost( + isPresented: $isPresented, + model: model, + minWidth: 320, + maxHeight: 520, + preferredEdge: .maxX, + // A context-menu/palette "Add Checklist Item…" bump is an + // explicit present request: it clears the host's external- + // dismissal latch so the popover can re-present even if + // the container's earlier `false` write hasn't landed yet. + presentationRequestToken: model.addFieldActivationToken + ) { model, close in + SidebarWorkspaceChecklistPopover( + model: model, + actions: actions, + onConsumeAddFieldActivation: onConsumeAddFieldActivation, + onClose: { + close() + onPopoverPresentedChange(false) + } + ) + } + .frame(width: 1, height: 1) + .allowsHitTesting(false) + } + } +} diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index ee2d29e24c09..00f434658333 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -10543,12 +10543,8 @@ struct VerticalTabsSidebar: View { } .onReceive(NotificationCenter.default.publisher(for: .workspaceChecklistAddItemRequested)) { notification in guard let workspaceId = notification.userInfo?[WorkspaceTodoActions.workspaceIdUserInfoKey] as? UUID, - let workspace = tabManager.tabs.first(where: { $0.id == workspaceId }) else { return } - // Popover style routes the add request into the checklist popover - // (armed add field); empty checklists keep the inline ghost row - // because there is no summary line to anchor a popover to. - if WorkspaceTodoFeature.checklistStyle == .popover, - !workspace.todoState.checklist.isEmpty { + tabManager.tabs.contains(where: { $0.id == workspaceId }) else { return } + if WorkspaceTodoFeature.checklistStyle == .popover { statusPopoverWorkspaceId = nil checklistPopoverWorkspaceId = workspaceId } else { @@ -10651,6 +10647,10 @@ struct VerticalTabsSidebar: View { } .onChange(of: tabManager.selectedTabId) { _, _ in requestSelectedWorkspaceScroll(scrollProxy, renderContext: renderContext) + // Workspace switches produce no outside click for .transient auto-dismiss; close popovers explicitly. + if let dismissed = checklistPopoverWorkspaceId { checklistAddFieldActivationTokens[dismissed] = nil } + checklistPopoverWorkspaceId = nil + statusPopoverWorkspaceId = nil } .onChange(of: renderContext.workspaceIds) { oldWorkspaceIds, newWorkspaceIds in guard shouldRequestSelectedWorkspaceScrollAfterWorkspaceIdsChange( @@ -13836,12 +13836,10 @@ struct TabItemView: View, Equatable { .lineLimit(1) } - // Checklist summary line + inline expansion. Rendered while the - // workspace-todos feature is on and there is either content or a - // pending "Add Checklist Item…" request (which needs the add - // field visible on an empty checklist). - if workspaceSnapshot.taskStatus != nil, - !workspaceSnapshot.checklistItems.isEmpty || checklistAddFieldActivationToken > 0 { + // Rendered whenever there is content, a pending add request, or an OPEN + // popover — unmounting dismantles the popover's anchor mid-presentation. + if !workspaceSnapshot.checklistItems.isEmpty || checklistAddFieldActivationToken > 0 + || isChecklistPopoverPresented { SidebarWorkspaceChecklistSection( items: workspaceSnapshot.checklistItems, completedCount: workspaceSnapshot.checklistCompletedCount, diff --git a/Sources/Panels/WorkspaceTodoPanel.swift b/Sources/Panels/WorkspaceTodoPanel.swift index 8702b6d4bf15..aa12ea3f9f22 100644 --- a/Sources/Panels/WorkspaceTodoPanel.swift +++ b/Sources/Panels/WorkspaceTodoPanel.swift @@ -31,6 +31,15 @@ final class WorkspaceTodoPanel: Panel, ObservableObject { /// Token incremented to trigger the focus flash animation. @Published private(set) var focusFlashToken: Int = 0 + /// Bumped when an open-or-focus entry point (checklist popover footer, + /// palette, CLI) lands on this pane, so the add field re-arms even when + /// the pane was ALREADY focused and `isFocused` never transitions. + @Published private(set) var addFieldArmToken: Int = 0 + + func armAddField() { + addFieldArmToken += 1 + } + init(workspace: Workspace) { self.id = UUID() self.workspace = workspace diff --git a/Sources/Panels/WorkspaceTodoPanelView.swift b/Sources/Panels/WorkspaceTodoPanelView.swift index 4f028c499c1d..eb0094fb27af 100644 --- a/Sources/Panels/WorkspaceTodoPanelView.swift +++ b/Sources/Panels/WorkspaceTodoPanelView.swift @@ -24,7 +24,8 @@ struct WorkspaceTodoPanelView: View { WorkspaceTodoPaneContent( workspace: workspace, todoState: workspace.todoState, - isFocused: isFocused + isFocused: isFocused, + addFieldArmToken: panel.addFieldArmToken ) } else { Text(String( @@ -50,6 +51,8 @@ private struct WorkspaceTodoPaneContent: View { @ObservedObject var workspace: Workspace @ObservedObject var todoState: WorkspaceTodoState let isFocused: Bool + /// Open-or-focus bump; re-arms the add field when `isFocused` doesn't transition. + let addFieldArmToken: Int @State private var isStatusPopoverPresented = false @State private var pendingItemText = "" @@ -89,30 +92,40 @@ private struct WorkspaceTodoPaneContent: View { .padding(.vertical, 10) Divider() let ordered = SidebarWorkspaceChecklistDisplayPolicy.orderedItems(todoState.checklist) - ScrollView(.vertical) { - VStack(alignment: .leading, spacing: 3) { - if ordered.isEmpty { - Text(String( - localized: "workspaceTodoPane.emptyChecklist", - defaultValue: "No checklist items yet." - )) - .font(.system(size: Self.itemFontSize)) - .foregroundColor(.secondary) - .padding(.vertical, 4) - } - ForEach(Array(ordered.enumerated()), id: \.element.id) { index, item in - itemRow(item, displayIndex: index) + ScrollViewReader { proxy in + ScrollView(.vertical) { + VStack(alignment: .leading, spacing: 3) { + if ordered.isEmpty { + Text(String( + localized: "workspaceTodoPane.emptyChecklist", + defaultValue: "No checklist items yet." + )) + .font(.system(size: Self.itemFontSize)) + .foregroundColor(.secondary) + .padding(.vertical, 4) + } + ForEach(Array(ordered.enumerated()), id: \.element.id) { index, item in + itemRow(item, displayIndex: index) + .id(item.id) + } } + .padding(.horizontal, 14) + .padding(.vertical, 8) + .frame(maxWidth: .infinity, alignment: .leading) + } + .focusable(!ordered.isEmpty) + .focused($itemsFocused) + .onKeyPress(.upArrow) { moveHighlight(-1, in: ordered) } + .onKeyPress(.downArrow) { moveHighlight(1, in: ordered) } + .onKeyPress { press in handleItemsKeyPress(press, ordered: ordered) } + // `anchor: nil` scrolls the minimal distance needed to bring + // the highlighted row fully into view — a no-op if it's + // already visible. + .onChange(of: highlightedItemId) { _, newValue in + guard let newValue else { return } + proxy.scrollTo(newValue, anchor: nil) } - .padding(.horizontal, 14) - .padding(.vertical, 8) - .frame(maxWidth: .infinity, alignment: .leading) } - .focusable(!ordered.isEmpty) - .focused($itemsFocused) - .onKeyPress(.upArrow) { moveHighlight(-1, in: ordered) } - .onKeyPress(.downArrow) { moveHighlight(1, in: ordered) } - .onKeyPress { press in handleItemsKeyPress(press, ordered: ordered) } Divider() addItemRow .padding(.horizontal, 14) @@ -124,6 +137,7 @@ private struct WorkspaceTodoPaneContent: View { .onChange(of: isFocused) { _, focused in if focused, editingItemId == nil { addFieldFocused = true } } + .onChange(of: addFieldArmToken) { _, _ in if editingItemId == nil { addFieldFocused = true } } .onChange(of: editFieldFocused) { _, focused in if !focused { finishItemEditOnFocusLoss() } } @@ -382,6 +396,16 @@ private struct WorkspaceTodoPaneItemRow: View { private var isCompleted: Bool { item.state == .completed } + /// Distance above a text line's baseline to its optical vertical center + /// (`(ascender + descender) / 2`), so the checkbox's + /// `.alignmentGuide(.firstTextBaseline)` centers on the item text's FIRST + /// line specifically — not the whole multi-line block, and not the + /// baseline itself. + private var firstLineCenterOffset: CGFloat { + let font = NSFont.systemFont(ofSize: itemFontSize) + return (font.ascender + font.descender) / 2 + } + var body: some View { HStack(alignment: .firstTextBaseline, spacing: 7) { Button { @@ -395,6 +419,7 @@ private struct WorkspaceTodoPaneItemRow: View { .contentShape(Rectangle()) } .buttonStyle(.plain) + .alignmentGuide(.firstTextBaseline) { $0[VerticalAlignment.center] + firstLineCenterOffset } .safeHelp( isCompleted ? String(localized: "sidebar.checklist.uncheckTooltip", defaultValue: "Mark as pending") @@ -413,11 +438,22 @@ private struct WorkspaceTodoPaneItemRow: View { .onExitCommand(perform: actions.cancelEdit) .accessibilityIdentifier("WorkspaceTodoPaneEditItemField") } else { + // No `lineLimit` — items wrap across multiple lines. Without + // `.fixedSize(horizontal: false, ...)` Text can report its + // ideal (unwrapped) single-line width as accepted inside this + // HStack + Spacer + ScrollView nesting, so long items overflow + // past the pane's edge instead of wrapping (see the sidebar's + // matching fix in SidebarWorkspaceChecklistView.swift / + // SidebarWorkspaceChecklistPopover.swift). The checkbox above + // aligns to this Text's FIRST line only (`.firstTextBaseline`, + // offset by `firstLineCenterOffset`), not the whole block. Text(item.text) .font(.system(size: itemFontSize)) .foregroundColor(isCompleted ? .secondary : .primary) .strikethrough(isCompleted) .opacity(isCompleted ? 0.6 : 1) + .multilineTextAlignment(.leading) + .fixedSize(horizontal: false, vertical: true) .contentShape(Rectangle()) .onTapGesture { actions.beginEdit() } } diff --git a/Sources/SidebarWorkspaceChecklistPopover.swift b/Sources/SidebarWorkspaceChecklistPopover.swift index 1f26980cc32e..4c9fa1ee9b67 100644 --- a/Sources/SidebarWorkspaceChecklistPopover.swift +++ b/Sources/SidebarWorkspaceChecklistPopover.swift @@ -1,3 +1,4 @@ +import AppKit import CmuxWorkspaces import SwiftUI @@ -16,17 +17,17 @@ struct SidebarWorkspaceChecklistPopoverModel: Equatable { /// The checklist popover anchored to a workspace row's summary line /// (`sidebar.beta.workspaceTodos.checklistStyle` = `popover`): header with /// the workspace title and progress, the ordered item rows (completed sink -/// below unchecked, clamped at 7 with an in-place "… N more"), a ghost add -/// row whose TextField commits on Enter and re-arms, and an "Open as Pane" -/// footer. Hosted in a real NSPopover so the TextField can take first -/// responder (see `SidebarWorkspaceTodoPopoverHost`). +/// below unchecked, viewport capped at ``visibleRowCount`` rows and +/// scrollable beyond that), a ghost add row whose TextField commits on +/// Enter and re-arms, and an "Open as Pane" footer. Hosted in a real +/// NSPopover so the TextField can take first responder (see +/// `SidebarWorkspaceTodoPopoverHost`). struct SidebarWorkspaceChecklistPopover: View { let model: SidebarWorkspaceChecklistPopoverModel let actions: SidebarWorkspaceChecklistActions let onConsumeAddFieldActivation: () -> Void let onClose: @MainActor () -> Void - @State private var showsAllItems = false @State private var pendingItemText = "" @FocusState private var addFieldFocused: Bool @State private var editingItemId: UUID? @@ -36,41 +37,127 @@ struct SidebarWorkspaceChecklistPopover: View { /// Return toggles it when the add field is empty, and Cmd+Return always /// toggles it between completed and pending. @State private var highlightedItemId: UUID? + /// Pointer position in ``Self/pointerSpaceName`` space (nil = outside). + /// A REFERENCE box, not `@State` value storage: mouse-moved arrives per + /// pixel, and a CGPoint state write per event would rebuild every + /// non-lazy row at mouse frequency. Mutating the box invalidates + /// nothing; only `hoveredItemId` (row-granular) drives renders. + private final class PointerLocationBox { + var location: CGPoint? + } + + @State private var pointerLocation = PointerLocationBox() + /// Item under the pointer (reveals the trailing delete button). Derived + /// from pointer position + row geometry — never from per-row hover + /// events, which die when content recreates or rows reflow under a + /// resting pointer. Written only when the hovered ROW changes. + @State private var hoveredItemId: UUID? + /// Row frames in ``Self/pointerSpaceName`` space (via preference); update + /// on scroll/reflow so hover self-corrects under a resting pointer. + @State private var itemRowFrames: [UUID: CGRect] = [:] + + private static let pointerSpaceName = "checklistPopoverPointerSpace" + + private func rederiveHover(frames: [UUID: CGRect]) { + let hovered = pointerLocation.location.flatMap { point in + frames.first { $0.value.contains(point) }?.key + } + if hovered != hoveredItemId { + hoveredItemId = hovered + } + } private static let itemFontSize: CGFloat = 13 /// Checkbox glyphs draw at 13pt (the inline row's base is 8pt·scale). private static let checkboxPointSize: CGFloat = 13 + /// Single-line row height estimate (`itemFontSize` plus the row's own + /// `.padding(.vertical, 2)` on both edges plus a little line-height + /// headroom), used to cap the item list's scrollable viewport at + /// ``visibleRowCount`` rows instead of the previous flat 460pt cap + /// (≈23 rows). + private static let itemRowHeightEstimate: CGFloat = itemFontSize + 6 + private static let visibleRowCount = 6 + private static let rowSpacing: CGFloat = 2 + + /// Distance above a text line's baseline to its optical vertical center + /// (`(ascender + descender) / 2`), so the checkbox/remove button + /// `.alignmentGuide(.firstTextBaseline)` centers on the item text's + /// FIRST line specifically — not the whole multi-line block, and not the + /// baseline itself. + private var firstLineCenterOffset: CGFloat { + let font = NSFont.systemFont(ofSize: Self.itemFontSize) + return (font.ascender + font.descender) / 2 + } + + /// Content height for `count` rows, capped at ``visibleRowCount`` rows — + /// short lists get exactly their own height (no dead space), longer + /// lists get the 6-row cap and scroll for the rest. + private func scrollViewportHeight(forItemCount count: Int) -> CGFloat { + guard count > 0 else { return 0 } + let visibleCount = min(count, Self.visibleRowCount) + return Self.itemRowHeightEstimate * CGFloat(visibleCount) + + Self.rowSpacing * CGFloat(visibleCount - 1) + } + var body: some View { let ordered = SidebarWorkspaceChecklistDisplayPolicy.orderedItems(model.items) - let clamped = SidebarWorkspaceChecklistDisplayPolicy.clampedItems( - ordered, - showsAllItems: showsAllItems - ) VStack(alignment: .leading, spacing: 0) { header .padding(.horizontal, 12) .padding(.top, 10) .padding(.bottom, 6) - ScrollView(.vertical) { - VStack(alignment: .leading, spacing: 2) { - ForEach(clamped.visible) { item in - itemRow(item) + if !ordered.isEmpty { + ScrollViewReader { proxy in + ScrollView(.vertical) { + VStack(alignment: .leading, spacing: 2) { + ForEach(ordered) { item in + itemRow(item) + .id(item.id) + } + } + .padding(.horizontal, 8) } - if clamped.hiddenCount > 0 { - moreRow(hiddenCount: clamped.hiddenCount) + .frame(height: scrollViewportHeight(forItemCount: ordered.count)) + // `anchor: nil` scrolls the minimal distance needed to + // bring the highlighted row fully into view — a no-op if + // it's already visible, matching arrow-key nav that + // should only move the viewport when it must. + .onChange(of: highlightedItemId) { _, newValue in + guard let newValue else { return } + proxy.scrollTo(newValue, anchor: nil) } - addItemRow(visible: clamped.visible) } + } + addItemRow(visible: ordered) .padding(.horizontal, 8) .padding(.bottom, 6) - } - .frame(maxHeight: 460) Divider() footer } .frame(width: 320, alignment: .leading) - .background(toggleHighlightedShortcutButton(visible: clamped.visible)) + .coordinateSpace(name: Self.pointerSpaceName) + // AppKit-owned pointer tracking (see PopoverPointerTracker's doc for + // why SwiftUI hover modifiers can't be used here: their tracking + // areas churn with content updates and drop the first post-mutation + // event as a spurious "ended"). + .background(PopoverPointerTracker { location in + pointerLocation.location = location + rederiveHover(frames: itemRowFrames) + }) + .onPreferenceChange(ChecklistPopoverRowFramesKey.self) { frames in + itemRowFrames = frames + rederiveHover(frames: frames) + } + .background(toggleHighlightedShortcutButton(visible: ordered)) + // Without this, the popover's window only gets promoted to key once, + // at `popoverDidShow` — if the terminal-backed pane grabs key window + // status back afterward (see `PopoverKeyWindowElevator`'s doc + // comment), `.onContinuousHover`'s tracking areas stop firing + // (SwiftUI hover tracking is gated on `.activeInKeyWindow`), so the + // remove-item "x" stops revealing on hover. `SidebarWorkspaceStatusPopover` + // already carries this same fix for its own popover. + .background(PopoverKeyWindowElevator()) .onAppear { addFieldFocused = true } .onChange(of: editFieldFocused) { _, focused in if !focused { finishItemEditOnFocusLoss() } @@ -117,6 +204,7 @@ struct SidebarWorkspaceChecklistPopover: View { .contentShape(Rectangle()) } .buttonStyle(.plain) + .alignmentGuide(.firstTextBaseline) { $0[VerticalAlignment.center] + firstLineCenterOffset } .safeHelp( isCompleted ? String(localized: "sidebar.checklist.uncheckTooltip", defaultValue: "Mark as pending") @@ -135,17 +223,23 @@ struct SidebarWorkspaceChecklistPopover: View { .onExitCommand(perform: cancelItemEdit) .accessibilityIdentifier("SidebarChecklistPopoverEditItemField") } else { + // No `lineLimit` — items wrap across multiple lines. The + // checkbox/remove button align to this Text's FIRST line + // only (`.firstTextBaseline`, offset by + // `firstLineCenterOffset`), not the whole wrapped block. Text(item.text) .font(.system(size: Self.itemFontSize)) .foregroundColor(isCompleted ? .secondary : .primary) .strikethrough(isCompleted) .opacity(isCompleted ? 0.6 : 1) - .lineLimit(3) - .truncationMode(.tail) + .multilineTextAlignment(.leading) + .fixedSize(horizontal: false, vertical: true) .contentShape(Rectangle()) .onTapGesture { beginItemEdit(item) } } Spacer(minLength: 0) + removeItemButton(for: item) + .alignmentGuide(.firstTextBaseline) { $0[VerticalAlignment.center] + firstLineCenterOffset } } .padding(.horizontal, 4) .padding(.vertical, 2) @@ -154,7 +248,26 @@ struct SidebarWorkspaceChecklistPopover: View { .fill(highlightedItemId == item.id ? Color.primary.opacity(0.08) : Color.clear) ) .contentShape(Rectangle()) - .onTapGesture { highlightedItemId = item.id } + .onTapGesture { + // Highlighting a row while the add field holds an in-progress + // draft would leave both a "focused" item and a "focused" field + // on screen at once, making Return's outcome ambiguous — only + // set the highlight when there is no draft to disambiguate. + guard pendingItemText.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { return } + highlightedItemId = item.id + } + // Hover is derived at the container level from pointer position + + // this row's reported frame (see `hoveredItemId`'s doc comment) — + // per-row hover callbacks die when the backing view is recreated + // while the pointer rests in place. + .background( + GeometryReader { proxy in + Color.clear.preference( + key: ChecklistPopoverRowFramesKey.self, + value: [item.id: proxy.frame(in: .named(Self.pointerSpaceName))] + ) + } + ) .contextMenu { Button(String(localized: "sidebar.checklist.editItem", defaultValue: "Edit")) { beginItemEdit(item) @@ -179,28 +292,26 @@ struct SidebarWorkspaceChecklistPopover: View { } } - private func moreRow(hiddenCount: Int) -> some View { - Button { - showsAllItems = true + /// Trailing hover-reveal delete affordance, in addition to the row's + /// context-menu "Remove" entry. Always laid out at a fixed size (only + /// `.opacity`/`.allowsHitTesting` toggle) so the row's height never jumps + /// when the pointer enters/leaves. + private func removeItemButton(for item: WorkspaceChecklistItem) -> some View { + let isHovered = hoveredItemId == item.id + return Button { + actions.removeItem(item.id) } label: { - Text( - String( - format: String( - localized: "sidebar.checklist.moreItems", - defaultValue: "… %lld more" - ), - locale: .current, - hiddenCount - ) - ) - .font(.system(size: Self.itemFontSize)) - .foregroundColor(.secondary) - .contentShape(Rectangle()) + CmuxSystemSymbolImage(systemName: "xmark.circle.fill", pointSize: Self.checkboxPointSize - 2) + .foregroundColor(.secondary) + .frame(width: Self.checkboxPointSize + 6, height: Self.checkboxPointSize + 6, alignment: .center) + .contentShape(Rectangle()) } .buttonStyle(.plain) - .padding(.horizontal, 4) - .padding(.vertical, 2) - .accessibilityIdentifier("SidebarChecklistPopoverMoreRow") + .safeHelp(String(localized: "sidebar.checklist.removeItemTooltip", defaultValue: "Remove item")) + .opacity(isHovered ? 1 : 0) + .allowsHitTesting(isHovered) + .accessibilityHidden(!isHovered) + .accessibilityIdentifier("SidebarChecklistPopoverRemoveItemButton") } // MARK: Add-item row (always armed — typing needs zero extra clicks) @@ -226,8 +337,16 @@ struct SidebarWorkspaceChecklistPopover: View { .onKeyPress(.upArrow) { moveHighlight(-1, in: visible) } .onKeyPress(.downArrow) { moveHighlight(1, in: visible) } .onKeyPress(.return) { handleAddFieldReturn(visible: visible) } + .onKeyPress(.delete) { handleAddFieldDelete(visible: visible) } .onSubmit(commitPendingItem) .onExitCommand(perform: cancelPendingItem) + .onChange(of: pendingItemText) { _, newValue in + // A highlighted item plus live typed text is the ambiguous + // dual-focus state Return can't resolve visually — as soon + // as the draft becomes non-empty, drop the highlight. + guard !newValue.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { return } + highlightedItemId = nil + } .accessibilityIdentifier("SidebarChecklistPopoverAddItemField") } .padding(.horizontal, 4) @@ -239,6 +358,12 @@ struct SidebarWorkspaceChecklistPopover: View { /// Moves the highlight up/down through the visible items, clamping at the /// ends. private func moveHighlight(_ delta: Int, in visible: [WorkspaceChecklistItem]) -> KeyPress.Result { + // Browsing (arrow-key highlight) and typing a new item are mutually + // exclusive: only move the highlight while the draft is empty, so a + // highlighted item and live typed text never coexist. + guard pendingItemText.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { + return .ignored + } guard !visible.isEmpty else { return .ignored } let currentIndex = visible.firstIndex(where: { $0.id == highlightedItemId }) ?? (delta > 0 ? -1 : visible.count) @@ -257,6 +382,19 @@ struct SidebarWorkspaceChecklistPopover: View { return .handled } + /// Backspace with an empty draft removes the highlighted item — a + /// keyboard-driven delete alongside the row's hover "x" and context-menu + /// "Remove", for browsing-mode (Up/Down-highlighted) deletion without + /// reaching for the mouse. + private func handleAddFieldDelete(visible: [WorkspaceChecklistItem]) -> KeyPress.Result { + guard pendingItemText.isEmpty else { return .ignored } + guard let id = highlightedItemId, + visible.contains(where: { $0.id == id }) else { return .ignored } + actions.removeItem(id) + highlightedItemId = nil + return .handled + } + /// A zero-size button that binds the configured shortcut to toggling the highlighted /// item. A `.keyboardShortcut` fires even while the add field is focused /// (a plain TextField only consumes bare Return via `onSubmit`), so the @@ -337,8 +475,11 @@ struct SidebarWorkspaceChecklistPopover: View { private var footer: some View { Button { - actions.openPane() + // Close FIRST: NSPopover teardown restores the parent window's + // previous first responder, which would clobber the pane focus / + // armed add field that openPane() sets up. onClose() + actions.openPane() } label: { HStack(spacing: 6) { CmuxSystemSymbolImage(systemName: "rectangle.split.2x1", pointSize: 11) diff --git a/Sources/SidebarWorkspaceChecklistView.swift b/Sources/SidebarWorkspaceChecklistView.swift index d3f90acc62e8..06260c82ef9f 100644 --- a/Sources/SidebarWorkspaceChecklistView.swift +++ b/Sources/SidebarWorkspaceChecklistView.swift @@ -1,3 +1,4 @@ +import AppKit import CmuxWorkspaces import SwiftUI @@ -67,9 +68,10 @@ struct SidebarWorkspaceChecklistSection: View { /// "Add Checklist Item…" asks this row to arm and focus its add field. let addFieldActivationToken: Int /// Whether the `sidebar.beta.workspaceTodos.checklistStyle` setting is - /// `popover`: the summary line opens an anchored checklist popover - /// instead of the inline expansion. Empty checklists keep the inline - /// ghost add row either way (no summary line to anchor to). + /// `popover`: the summary line (or, for an empty checklist with no + /// summary line yet, the ghost "Add item" row) opens an anchored + /// checklist popover instead of the inline expansion — including for a + /// workspace's very first item. let usesPopoverPresentation: Bool let isPopoverPresented: Bool let primaryColor: Color @@ -82,16 +84,24 @@ struct SidebarWorkspaceChecklistSection: View { let onConsumeAddFieldActivation: () -> Void let actions: SidebarWorkspaceChecklistActions - @State private var showsAllItems = false @State private var isAddingItem = false /// Bumped after each add to recreate the AppKit add field (which re-focuses /// and clears itself on appear). @State private var inlineAddGeneration = 0 @State private var editingItemId: UUID? + /// The item currently under the pointer, used to reveal the trailing + /// delete button. A single id (not a per-row `@State`) is enough because + /// only one row can be hovered at a time; mirrors `editingItemId`. + @State private var hoveredItemId: UUID? - /// Popover presentation only applies once a summary line exists. + /// Whether taps and the "Add Checklist Item…" activation token should + /// route to the anchored popover instead of the inline expansion. Equal + /// to `usesPopoverPresentation` regardless of `totalCount`, so a + /// workspace's very first checklist item also opens the popover in + /// `.popover` style — the popover anchor lives on the outer container + /// below, which is present whether or not a summary line exists yet. private var presentsPopover: Bool { - usesPopoverPresentation && totalCount > 0 + usesPopoverPresentation } var body: some View { @@ -99,10 +109,46 @@ struct SidebarWorkspaceChecklistSection: View { if totalCount > 0 { summaryLine } - if (isExpanded && !presentsPopover) || totalCount == 0 { + // In popover style, "Add Checklist Item…" opens the popover + // directly (see the container's `.workspaceChecklistAddItemRequested` + // handler) — the row itself shows nothing inline (no ghost + // "Add item" affordance) until an item actually exists, at + // which point `summaryLine` above is the small status preview. + if !presentsPopover, isExpanded || totalCount == 0 { expandedList } } + // The popover anchor is hosted here (not on `summaryLine` alone) so + // the same backing NSView anchors the popover across the 0→1 item + // transition — this outer container is always present (even when + // popover style renders neither `summaryLine` nor `expandedList` + // while `totalCount == 0`), so the anchor never needs to move; + // re-anchoring to a freshly created view would close and immediately + // reopen the popover. The anchor is a fixed 1×1pt corner overlay + // (see `ChecklistSummaryPopoverModifier`), so it has real, stable + // bounds regardless of whether this VStack has any content. + .modifier(ChecklistSummaryPopoverModifier( + isPresented: presentsPopover + ? Binding(get: { isPopoverPresented }, set: { presented in + onPopoverPresentedChange(presented) + // Any close consumes a pending add-field activation: a + // dismissed first-item popover must not leave the + // workspace in stale "add requested" state (which also + // keeps the empty section mounted invisibly). + if !presented { onConsumeAddFieldActivation() } + }) + : .constant(false), + model: SidebarWorkspaceChecklistPopoverModel( + workspaceTitle: workspaceTitle, + items: items, + completedCount: completedCount, + totalCount: totalCount, + addFieldActivationToken: addFieldActivationToken + ), + actions: actions, + onConsumeAddFieldActivation: onConsumeAddFieldActivation, + onPopoverPresentedChange: onPopoverPresentedChange + )) .task(id: addFieldActivationToken) { // In popover presentation the container routes the token into the // checklist popover instead; arming the (hidden) inline field @@ -154,21 +200,6 @@ struct SidebarWorkspaceChecklistSection: View { ? String(localized: "sidebar.checklist.collapseTooltip", defaultValue: "Hide checklist items") : String(localized: "sidebar.checklist.expandTooltip", defaultValue: "Show checklist items")) ) - .modifier(ChecklistSummaryPopoverModifier( - isPresented: presentsPopover - ? Binding(get: { isPopoverPresented }, set: { onPopoverPresentedChange($0) }) - : .constant(false), - model: SidebarWorkspaceChecklistPopoverModel( - workspaceTitle: workspaceTitle, - items: items, - completedCount: completedCount, - totalCount: totalCount, - addFieldActivationToken: addFieldActivationToken - ), - actions: actions, - onConsumeAddFieldActivation: onConsumeAddFieldActivation, - onPopoverPresentedChange: onPopoverPresentedChange - )) .accessibilityIdentifier("SidebarChecklistSummaryLine") } @@ -177,22 +208,40 @@ struct SidebarWorkspaceChecklistSection: View { @ViewBuilder private var expandedList: some View { let ordered = SidebarWorkspaceChecklistDisplayPolicy.orderedItems(items) - let clamped = SidebarWorkspaceChecklistDisplayPolicy.clampedItems( - ordered, - showsAllItems: showsAllItems - ) VStack(alignment: .leading, spacing: 2) { - ForEach(clamped.visible) { item in - checklistItemRow(item) - } - if clamped.hiddenCount > 0 { - moreRow(hiddenCount: clamped.hiddenCount) + if !ordered.isEmpty { + ScrollView(.vertical) { + VStack(alignment: .leading, spacing: 2) { + ForEach(ordered) { item in + checklistItemRow(item) + } + } + } + .frame(height: scrollViewportHeight(forItemCount: ordered.count)) } addItemRow } .padding(.leading, 2) } + /// Single-line row height estimate (matches the add/edit field's own + /// `11 * fontScale + 4` sizing), used to cap the expanded list's + /// scrollable viewport at ``visibleRowCount`` rows instead of letting an + /// arbitrarily long checklist grow the sidebar row without bound. + private var itemRowHeightEstimate: CGFloat { 11 * fontScale + 4 } + private static let visibleRowCount = 6 + private static let rowSpacing: CGFloat = 2 + + /// Content height for `count` rows, capped at ``visibleRowCount`` rows — + /// short lists get exactly their own height (no dead space), longer + /// lists get the 6-row cap and scroll for the rest. + private func scrollViewportHeight(forItemCount count: Int) -> CGFloat { + guard count > 0 else { return 0 } + let visibleCount = min(count, Self.visibleRowCount) + return itemRowHeightEstimate * CGFloat(visibleCount) + + Self.rowSpacing * CGFloat(visibleCount - 1) + } + private func checklistItemRow(_ item: WorkspaceChecklistItem) -> some View { let isCompleted = item.state == .completed return HStack(alignment: .firstTextBaseline, spacing: 4) { @@ -207,6 +256,7 @@ struct SidebarWorkspaceChecklistSection: View { .contentShape(Rectangle()) } .buttonStyle(.plain) + .alignmentGuide(.firstTextBaseline) { $0[VerticalAlignment.center] + firstLineCenterOffset } .safeHelp( isCompleted ? String(localized: "sidebar.checklist.uncheckTooltip", defaultValue: "Mark as pending") @@ -225,17 +275,42 @@ struct SidebarWorkspaceChecklistSection: View { .frame(height: 11 * fontScale + 4) .accessibilityIdentifier("SidebarChecklistEditItemField") } else { + // No `lineLimit` — items wrap across multiple lines. The + // checkbox/remove button above/below align to this Text's + // FIRST line only (`.firstTextBaseline`, offset by + // `firstLineCenterOffset`), not the whole wrapped block. Text(item.text) .font(itemFont) .foregroundColor(isCompleted ? secondaryColor : primaryColor) .strikethrough(isCompleted) .opacity(isCompleted ? 0.6 : 1) - .lineLimit(2) - .truncationMode(.tail) + .multilineTextAlignment(.leading) + .fixedSize(horizontal: false, vertical: true) .contentShape(Rectangle()) .onTapGesture { beginItemEdit(item) } } Spacer(minLength: 0) + removeItemButton(for: item) + .alignmentGuide(.firstTextBaseline) { $0[VerticalAlignment.center] + firstLineCenterOffset } + } + .contentShape(Rectangle()) + // `.onContinuousHover` rather than `.onHover`: `.onHover` only fires + // on the `mouseEntered`/`mouseExited` edge, so if this row's backing + // view gets recreated (e.g. a sidebar re-render under this section) + // while the pointer is already inside it, no new `mouseEntered` + // arrives and the hover state gets stuck off — the "barely or never + // appears" symptom. Continuous hover re-derives the phase from the + // current pointer location on every move, so it self-corrects within + // one frame regardless of view-identity churn. + .onContinuousHover { phase in + switch phase { + case .active: + hoveredItemId = item.id + case .ended: + if hoveredItemId == item.id { + hoveredItemId = nil + } + } } .contextMenu { Button(String(localized: "sidebar.checklist.editItem", defaultValue: "Edit")) { @@ -261,26 +336,40 @@ struct SidebarWorkspaceChecklistSection: View { } } - private func moreRow(hiddenCount: Int) -> some View { - Button { - showsAllItems = true + /// Distance above a text line's baseline to its optical vertical center + /// (`(ascender + descender) / 2`), so the checkbox/remove button + /// `.alignmentGuide(.firstTextBaseline)` centers on the item text's + /// FIRST line specifically — not the whole multi-line block, and not the + /// baseline itself. `itemFont` is `magnifiedFont(scaledFontSize(10))` + /// (see `ContentView.magnifiedFont`); approximated here as + /// `10 * fontScale` since the global magnification percent isn't + /// threaded down to this view — close enough for an alignment offset. + private var firstLineCenterOffset: CGFloat { + let font = NSFont.systemFont(ofSize: 10 * fontScale) + return (font.ascender + font.descender) / 2 + } + + /// Trailing hover-reveal delete affordance, in addition to the row's + /// context-menu "Remove" entry. Always laid out at a fixed size (only + /// `.opacity`/`.allowsHitTesting` toggle) so the row's height never jumps + /// when the pointer enters/leaves — same reserved-space technique as the + /// workspace row's hover close button (`SidebarWorkspaceTrailingStatusSlot`). + private func removeItemButton(for item: WorkspaceChecklistItem) -> some View { + let isHovered = hoveredItemId == item.id + return Button { + actions.removeItem(item.id) } label: { - Text( - String( - format: String( - localized: "sidebar.checklist.moreItems", - defaultValue: "… %lld more" - ), - locale: .current, - hiddenCount - ) - ) - .font(itemFont) - .foregroundColor(secondaryColor) - .contentShape(Rectangle()) + CmuxSystemSymbolImage(magnified: "xmark.circle.fill", pointSize: 9 * fontScale) + .foregroundColor(secondaryColor) + .frame(width: 9 * fontScale + 8, height: 9 * fontScale + 8, alignment: .center) + .contentShape(Rectangle()) } .buttonStyle(.plain) - .accessibilityIdentifier("SidebarChecklistMoreRow") + .safeHelp(String(localized: "sidebar.checklist.removeItemTooltip", defaultValue: "Remove item")) + .opacity(isHovered ? 1 : 0) + .allowsHitTesting(isHovered) + .accessibilityHidden(!isHovered) + .accessibilityIdentifier("SidebarChecklistRemoveItemButton") } // MARK: Add-item row @@ -313,7 +402,11 @@ struct SidebarWorkspaceChecklistSection: View { } } else { Button { - isAddingItem = true + if presentsPopover { + onPopoverPresentedChange(!isPopoverPresented) + } else { + isAddingItem = true + } } label: { HStack(spacing: 4) { CmuxSystemSymbolImage(magnified: "plus", pointSize: 7 * fontScale) @@ -360,44 +453,3 @@ struct SidebarWorkspaceChecklistSection: View { editingItemId = nil } } - -/// Attaches the checklist popover to the summary line with SwiftUI's native -/// `.popover` (not the NSPopover host): an embedded NSViewRepresentable inside -/// a `.onHover`-tracked sidebar row suppresses the row's hover tracking, which -/// hid the hover-close "x". The add field takes first responder via -/// `@FocusState` set on appear inside `SidebarWorkspaceChecklistPopover`. -private struct ChecklistSummaryPopoverModifier: ViewModifier { - @Binding var isPresented: Bool - let model: SidebarWorkspaceChecklistPopoverModel - let actions: SidebarWorkspaceChecklistActions - let onConsumeAddFieldActivation: () -> Void - let onPopoverPresentedChange: @MainActor (Bool) -> Void - - // The checklist popover embeds a first-responder TextField (the add / edit - // fields). SwiftUI's native `.popover` does not make its window key in - // cmux's focus-managed environment, so keystrokes fall through to the - // terminal. Host it in a real NSPopover (which takes key) instead. This - // anchor only exists on rows that actually have a checklist summary line, - // so it does not touch the every-row hover path the status glyph uses. - func body(content: Content) -> some View { - content.background( - SidebarWorkspaceTodoPopoverHost( - isPresented: $isPresented, - model: model, - minWidth: 320, - maxHeight: 520, - preferredEdge: .maxX - ) { model, close in - SidebarWorkspaceChecklistPopover( - model: model, - actions: actions, - onConsumeAddFieldActivation: onConsumeAddFieldActivation, - onClose: { - close() - onPopoverPresentedChange(false) - } - ) - } - ) - } -} diff --git a/Sources/SidebarWorkspaceTodoPopoverHost.swift b/Sources/SidebarWorkspaceTodoPopoverHost.swift index 721bd28d905c..8b5a720b2f2a 100644 --- a/Sources/SidebarWorkspaceTodoPopoverHost.swift +++ b/Sources/SidebarWorkspaceTodoPopoverHost.swift @@ -139,6 +139,12 @@ struct SidebarWorkspaceTodoPopoverHost: var minWidth: CGFloat = 200 var maxHeight: CGFloat = 480 var preferredEdge: NSRectEdge = .maxX + /// Explicit "user asked for this popover" signal (e.g. the checklist + /// add-field activation token). A change clears the external-dismissal + /// latch below, so a context-menu/palette request can always re-present + /// even while the latch is waiting for the container to acknowledge an + /// AppKit-side close. + var presentationRequestToken: Int = 0 /// Builds the popover body from the latest model; the second argument /// closes the popover (footer buttons, Return/Esc handling). let content: (Model, @escaping @MainActor () -> Void) -> PopoverContent @@ -147,9 +153,36 @@ struct SidebarWorkspaceTodoPopoverHost: Coordinator(isPresented: $isPresented) } + // NOTE: the coordinator's `isPresented` binding is REFRESHED on every + // `updateNSView` (see below). The section builds that binding from its + // per-render value snapshot, so a binding captured only at + // `makeCoordinator` time reads a frozen value forever — a coordinator + // created while the popover was hidden then saw `isPresented == false` + // at `popoverDidClose` even when the container still said shown, skipped + // the `false` write-back, and left the container state stuck `true`. + // Every later model change then re-presented the popover with no user + // action (the "popover opens while typing in the todo pane" bug). + + /// Anchor view for the popover. Retries a pending `present()` once it + /// actually attaches to a window: a same-transaction "open immediately" + /// request (e.g. a zero-item workspace's first Add Checklist Item, where + /// the anchor is mounted in the very same SwiftUI update that also asks + /// to present) can find `window == nil` on the first `present()` call, + /// since AppKit view attachment can lag the SwiftUI commit that inserted + /// it. Without this retry the request was silently dropped. + final class AnchorView: NSView { + weak var coordinator: Coordinator? + + override func viewDidMoveToWindow() { + super.viewDidMoveToWindow() + coordinator?.anchorViewDidMoveToWindow() + } + } + func makeNSView(context: Context) -> NSView { - let view = NSView() + let view = AnchorView() view.translatesAutoresizingMaskIntoConstraints = false + view.coordinator = context.coordinator context.coordinator.anchorView = view return view } @@ -160,6 +193,8 @@ struct SidebarWorkspaceTodoPopoverHost: coordinator.minWidth = minWidth coordinator.maxHeight = maxHeight coordinator.preferredEdge = preferredEdge + coordinator.isPresentedBinding = $isPresented + coordinator.acknowledge(isPresented: isPresented, requestToken: presentationRequestToken) coordinator.update(model: model) { model, close in AnyView(content(model, close)) } @@ -176,7 +211,15 @@ struct SidebarWorkspaceTodoPopoverHost: @MainActor final class Coordinator: NSObject, NSPopoverDelegate { - @Binding var isPresented: Bool + /// Refreshed by every `updateNSView` tick so reads and write-backs + /// target the CURRENT container state, not the value snapshot from + /// whichever render created this coordinator (see the note on + /// `makeCoordinator`). + var isPresentedBinding: Binding + private var isPresented: Bool { + get { isPresentedBinding.wrappedValue } + set { isPresentedBinding.wrappedValue = newValue } + } weak var anchorView: NSView? var minWidth: CGFloat = 200 var maxHeight: CGFloat = 480 @@ -194,9 +237,38 @@ struct SidebarWorkspaceTodoPopoverHost: /// Bumped on every hidden-to-shown transition; used as the SwiftUI /// view identity so each open gets fresh view-local state. private var presentationCount = 0 + /// Set when AppKit closed the popover out from under SwiftUI (app + /// deactivation, transient click-away) while the container still said + /// `isPresented == true`. The container's `isPresented = false` write + /// lands asynchronously, so an unrelated re-render can deliver a + /// stale `isPresented == true` to `updateNSView` first — without this + /// latch that stale tick re-presents the popover the user just + /// dismissed, producing a close/reopen churn loop (observed live: + /// five `didShow`s in 18s with zero user actions). Cleared when the + /// container acknowledges `false`, or when an explicit presentation + /// request token changes. + private var awaitingDismissAck = false + private var lastRequestToken = 0 init(isPresented: Binding) { - _isPresented = isPresented + isPresentedBinding = isPresented + } + + /// Called on every `updateNSView` tick, before `present()`/`dismiss()`. + func acknowledge(isPresented: Bool, requestToken: Int) { + if !isPresented { + awaitingDismissAck = false + } + if requestToken != lastRequestToken { + lastRequestToken = requestToken + // Only a real request unlatches. Token zero is the CONSUMED + // state (the container resets the activation token after the + // add field arms) — treating that reset as a fresh request + // re-presented a popover the user had just dismissed. + if requestToken != 0 { + awaitingDismissAck = false + } + } } func update( @@ -228,21 +300,50 @@ struct SidebarWorkspaceTodoPopoverHost: updateContentSize() } + /// Retries a pending open once the anchor finishes attaching to its + /// window (see `AnchorView`). Without this, a `present()` call that + /// hit a detached anchor had no way to recover except an unrelated + /// later SwiftUI re-render happening to land after attachment. + func anchorViewDidMoveToWindow() { + guard isPresented, popover?.isShown != true else { return } + present() + } + func present() { - guard let anchorView, anchorView.window != nil else { - isPresented = false + // After an AppKit-side close, wait for the container to confirm + // the matching `isPresented = false` before honoring any further + // present ticks — see `awaitingDismissAck`. + guard !awaitingDismissAck else { return } + guard let anchorView, let window = anchorView.window else { + // No window yet — don't clobber isPresented. AnchorView's + // viewDidMoveToWindow() retries once it actually attaches. return } - anchorView.superview?.layoutSubtreeIfNeeded() let popover = popover ?? makePopover() - // Only bump identity on a hidden-to-shown transition; bumping on - // every updateNSView would reset view-local state on every tick. - if !popover.isShown { - presentationCount += 1 - refreshContent() - } - updateContentSize() + // Everything below happens ONLY on the hidden-to-shown + // transition: `present()` is re-entered on every parent update + // tick while shown, and a window-wide layout flush (or an + // identity bump) on those ticks would widen row-scoped checklist + // churn into an app-wide synchronous layout pass per update. + // While shown, content and size updates flow through + // `update(model:builder:)` -> `refreshContent()` instead. guard !popover.isShown else { return } + // Lay out from the window's root, not just the anchor's + // immediate superview: `layoutSubtreeIfNeeded()` only resolves + // the subtree it's called on, not ancestors above it. A + // same-transaction "open immediately" anchor (a zero-item + // workspace's first Add Checklist Item — see the AnchorView doc + // comment) is freshly inserted into the sidebar's lazy list, so + // the row containers above `anchorView.superview` may not have + // an up-to-date frame yet. `popover.show(relativeTo:of:)` + // resolves the anchor's window-coordinate position by walking + // that whole ancestor chain, so a stale frame anywhere above the + // immediate superview anchors the popover to the wrong spot. + window.contentView?.layoutSubtreeIfNeeded() + anchorView.superview?.layoutSubtreeIfNeeded() + presentationCount += 1 + refreshContent() + updateContentSize() popover.show(relativeTo: anchorView.bounds, of: anchorView, preferredEdge: preferredEdge) } @@ -258,8 +359,16 @@ struct SidebarWorkspaceTodoPopoverHost: func popoverDidClose(_ notification: Notification) { popover = nil if isPresented { + // AppKit closed us (transient click-away / app deactivation) + // while SwiftUI still thinks we're shown: latch until the + // container acknowledges the write below, so stale re-render + // ticks can't instantly re-present (churn loop). + awaitingDismissAck = true isPresented = false } +#if DEBUG + cmuxDebugLog("focus.todoPopover.didClose awaitingAck=\(awaitingDismissAck)") +#endif } func popoverDidShow(_ notification: Notification) { diff --git a/Sources/Workspace+TodoPane.swift b/Sources/Workspace+TodoPane.swift index c2ed918b78ee..067ffe6fe611 100644 --- a/Sources/Workspace+TodoPane.swift +++ b/Sources/Workspace+TodoPane.swift @@ -74,9 +74,17 @@ extension Workspace { guard let todoPanel = panel as? WorkspaceTodoPanel else { continue } if focus { focusPanel(existingId) + // Re-arm even when the pane was already focused: `isFocused` + // never transitions then, so the pane's own focus-driven arm + // doesn't fire and "Open as Pane" would visibly do nothing. + todoPanel.armAddField() } return todoPanel } - return newWorkspaceTodoSurface(inPane: paneId, focus: focus) + let created = newWorkspaceTodoSurface(inPane: paneId, focus: focus) + if focus { + created?.armAddField() + } + return created } } diff --git a/Sources/WorkspaceTodoFeature.swift b/Sources/WorkspaceTodoFeature.swift index b356749bfc63..6cffd71c80e2 100644 --- a/Sources/WorkspaceTodoFeature.swift +++ b/Sources/WorkspaceTodoFeature.swift @@ -155,8 +155,10 @@ enum WorkspaceTodoActions { extension Notification.Name { /// Posted by ``WorkspaceTodoActions/requestChecklistAddField(workspaceId:)``; - /// observed by the workspace sidebar, which expands the row's checklist - /// and arms its add-item field. + /// observed by the workspace sidebar, which arms the row's add-item + /// field — via the anchored checklist popover in `.popover` style (even + /// for a workspace's very first item), or by expanding the row's inline + /// checklist in `.inline` style. static let workspaceChecklistAddItemRequested = Notification.Name( "cmux.workspaceChecklistAddItemRequested" ) diff --git a/cmux.xcodeproj/project.pbxproj b/cmux.xcodeproj/project.pbxproj index 58f613efbfb0..8c08ffc97a96 100644 --- a/cmux.xcodeproj/project.pbxproj +++ b/cmux.xcodeproj/project.pbxproj @@ -297,6 +297,8 @@ C0DE71B10000000000000001 /* AppDelegate+AgentChatNotifications.swift in Sources CA52B0070000000000000000 /* CanvasPaneContent.swift in Sources */ = {isa = PBXBuildFile; fileRef = CA52C0070000000000000000 /* CanvasPaneContent.swift */; }; C4A570030000000000000001 /* CanvasShortcutContextTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C4A570030000000000000002 /* CanvasShortcutContextTests.swift */; }; 081696D3A66545C50AD0598E /* ChecklistInputField.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3E3E449BD36EA0670CC7D75C /* ChecklistInputField.swift */; }; + D78129DCAE6C4D919AE20FF9 /* ChecklistPopoverPointerTracking.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9890AFCF5E5F4F3DA9F00CCC /* ChecklistPopoverPointerTracking.swift */; }; + ED226C59A21547BB962DB2A5 /* ChecklistSummaryPopoverModifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = 319C469164C74F2F8E41D2C3 /* ChecklistSummaryPopoverModifier.swift */; }; F3000000A1B2C3D4E5F60718 /* CJKIMEInputTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F3000001A1B2C3D4E5F60718 /* CJKIMEInputTests.swift */; }; D3571002A1B2C3D4E5F60718 /* CJKIMEMarkedSelectionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D3571003A1B2C3D4E5F60718 /* CJKIMEMarkedSelectionTests.swift */; }; A5D41232A1B2C3D4E5F60718 /* ClaudeBackgroundWorkNotifyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5D41233A1B2C3D4E5F60718 /* ClaudeBackgroundWorkNotifyTests.swift */; }; @@ -2005,6 +2007,8 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa = CA52C0070000000000000000 /* CanvasPaneContent.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CanvasPaneContent.swift"; sourceTree = ""; }; C4A570030000000000000002 /* CanvasShortcutContextTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CanvasShortcutContextTests.swift; sourceTree = ""; }; 3E3E449BD36EA0670CC7D75C /* ChecklistInputField.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ChecklistInputField.swift; sourceTree = ""; }; + 9890AFCF5E5F4F3DA9F00CCC /* ChecklistPopoverPointerTracking.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ChecklistPopoverPointerTracking.swift"; sourceTree = ""; }; + 319C469164C74F2F8E41D2C3 /* ChecklistSummaryPopoverModifier.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ChecklistSummaryPopoverModifier.swift"; sourceTree = ""; }; F3000001A1B2C3D4E5F60718 /* CJKIMEInputTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CJKIMEInputTests.swift; sourceTree = ""; }; D3571003A1B2C3D4E5F60718 /* CJKIMEMarkedSelectionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CJKIMEMarkedSelectionTests.swift; sourceTree = ""; }; A5D41233A1B2C3D4E5F60718 /* ClaudeBackgroundWorkNotifyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClaudeBackgroundWorkNotifyTests.swift; sourceTree = ""; }; @@ -3861,6 +3865,8 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa = BAC8DE2AF74EC2C272681A75 /* SidebarWorkspaceChecklistPopover.swift */, 3896D9D4AD4D31A54882813E /* SidebarWorkspaceStatusPopover.swift */, 4AEAD128139F57CB0E5BD39E /* SidebarWorkspaceTodoPopoverHost.swift */, + 319C469164C74F2F8E41D2C3 /* ChecklistSummaryPopoverModifier.swift */, + 9890AFCF5E5F4F3DA9F00CCC /* ChecklistPopoverPointerTracking.swift */, 55E01DE5B078B38299582BB0 /* SidebarWorkspaceTaskStatusGlyph.swift */, EE3CB8DEEC6E12B7D04D9B92 /* WorkspaceTodoFeature.swift */, A91C0D0E0000000000000001 /* TerminalTabAgentIcon.swift */, @@ -5679,6 +5685,8 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa = CA52B0060000000000000000 /* CanvasLayoutSettings.swift in Sources */, CA52B0070000000000000000 /* CanvasPaneContent.swift in Sources */, 081696D3A66545C50AD0598E /* ChecklistInputField.swift in Sources */, + D78129DCAE6C4D919AE20FF9 /* ChecklistPopoverPointerTracking.swift in Sources */, + ED226C59A21547BB962DB2A5 /* ChecklistSummaryPopoverModifier.swift in Sources */, C7A533000000000000000002 /* ClaudeSessionCanonicalizationContext.swift in Sources */, A9F200000000000000000015 /* ClaudeStreamJSONAccumulator.swift in Sources */, C46790000000000000000003 /* CLIForwardingLaunchRouter.swift in Sources */,