Repository navigation
Route the checklist's first item through the popover in popover style #7797
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f84155c
da4913d
98cc82b
e4297f6
8538a41
aee3354
7996c98
ad793aa
c647710
14d7e36
ac06925
cf219bf
4b746ef
5d9b6fd
4c3e43f
fb7df3a
73fa1c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 { | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| statusPopoverWorkspaceId = nil | ||
| checklistPopoverWorkspaceId = workspaceId | ||
|
Comment on lines
+10547
to
10549
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } else { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
@@ -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, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.