From cdd9a85e8187cf96347b3c97f5dcd42ec62411c2 Mon Sep 17 00:00:00 2001 From: Micah Alpern Date: Sat, 13 Jun 2026 12:24:54 -0500 Subject: [PATCH] Fix overlay output-type picker: hoisted-popover content updates + drill-down MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The overlay mapper's output-type picker was unusable: clicking an option that revealed more choices (System Action / Launch App / Go to Layer) did nothing. Root-caused with instrumentation to the window-anchored popover host, plus a non-key-window click issue, and reworked the picker UX. Core fix — WindowAnchoredPopover: - WindowAnchoredPopoverEntry had identity-only Equatable (compared just `id`). SwiftUI used it to skip both the preference update and the content re-render when a same-id entry's *content* changed, so the host kept showing stale content — the picker never reflected taps even though the model and rebuilt content were correct. Removed Equatable; the ESC monitor and appear/disappear animation key on `entries.map(\.id)` instead. - Build popover content fresh on each emission rather than caching a one-shot AnyView at open (the cache froze the content's dependency on live state). - Dismiss on clicks outside the app's own windows via a global mouse-down monitor — the in-window backdrop can't see those, so the popover lingered when clicking onto another app or the desktop. First-click registration: - FirstMouseHostingView overrides acceptsFirstMouse so a click lands on the control under the cursor even while the non-focus-stealing overlay window isn't key (previously the activation click was swallowed). Picker UX (iPhone-style drill-down): - Replaced inline section expansion with a fixed-frame paged drill-down: the root list slides to a sub-page (System Action grid / app list / layer list) and back. A stable frame avoids the height-reflow that fought the hoisted positioning pipeline. - Moved the picker's navigation/selection state (page, selectedLayerOutput, launch-app search) onto MapperViewModel: the popover renders in the detached host where view @State doesn't propagate, but the shared @Observable does. Launch App search: - Added a name filter to the app list. Uses AutoFocusTextField, an AppKit NSTextField that calls makeFirstResponder directly, because SwiftUI @FocusState does not bridge across the hoisted popover. Co-Authored-By: Claude Opus 4.8 --- .../UI/Experimental/MapperViewModel.swift | 33 + .../UI/Overlay/AutoFocusTextField.swift | 65 ++ .../LiveKeyboardOverlayController.swift | 6 +- .../UI/Overlay/LiveKeyboardOverlayTypes.swift | 19 + .../OverlayMapperSection+LaunchApps.swift | 68 ++- .../Overlay/OverlayMapperSection+Layers.swift | 4 +- ...rlayMapperSection+OutputTypeDropdown.swift | 2 +- .../OverlayMapperSection+ShiftOutput.swift | 2 +- ...rlayMapperSection+SystemActionGroups.swift | 577 ++++++++---------- .../UI/Overlay/OverlayMapperSection.swift | 10 +- .../Popover/WindowAnchoredPopover.swift | 98 +-- 11 files changed, 489 insertions(+), 395 deletions(-) create mode 100644 Sources/KeyPathAppKit/UI/Overlay/AutoFocusTextField.swift diff --git a/Sources/KeyPathAppKit/UI/Experimental/MapperViewModel.swift b/Sources/KeyPathAppKit/UI/Experimental/MapperViewModel.swift index 3b7f26174..6027a6907 100644 --- a/Sources/KeyPathAppKit/UI/Experimental/MapperViewModel.swift +++ b/Sources/KeyPathAppKit/UI/Experimental/MapperViewModel.swift @@ -47,6 +47,39 @@ class MapperViewModel { var selectedFolder: (path: String, name: String?)? /// Selected script path for script-run mapping (nil = normal key output) var selectedScript: (path: String, name: String?)? + + // MARK: - Output-Type Picker (Overlay) Navigation State + + /// Which page the overlay output-type picker is showing. + /// + /// The picker is an iPhone-style drill-down — the root list and each + /// sub-list (System Action / Launch App / Go to Layer) are separate pages + /// that slide over one another inside a *stable* popover frame. This + /// deliberately replaced inline expansion: growing the popover's height + /// forced the hoisted window-anchored layer to re-measure and reposition on + /// every toggle, which fought SwiftUI's preference/position machinery and + /// left the expandable rows unresponsive. Swapping pages in a fixed frame + /// avoids all of that. + /// + /// It lives on the view model (not `@State` on the view) because the picker + /// is rendered in that detached hoisted layer; `@State` mutated from there + /// does not propagate back, whereas this shared `@Observable` reference does. + enum OutputPickerPage: Equatable { + case root + case systemActions + case launchApps + case layers + } + + var outputPickerPage: OutputPickerPage = .root + /// Selected layer name for "Go to Layer" output (nil = not a layer output). + var selectedLayerOutput: String? + /// Filter text for the Launch App sub-page's known-apps list. Lives here + /// (not `@State` on the view) because the picker popover is hoisted; a + /// `TextField` bound to view `@State` from that detached layer wouldn't + /// propagate. Cleared each time the picker opens. + var launchAppSearchText: String = "" + /// Key code of the captured input (for overlay-style rendering) /// Default to 0 (A key) so the default state shows the A key selected var inputKeyCode: UInt16? = 0 diff --git a/Sources/KeyPathAppKit/UI/Overlay/AutoFocusTextField.swift b/Sources/KeyPathAppKit/UI/Overlay/AutoFocusTextField.swift new file mode 100644 index 000000000..ec6d6b870 --- /dev/null +++ b/Sources/KeyPathAppKit/UI/Overlay/AutoFocusTextField.swift @@ -0,0 +1,65 @@ +import AppKit +import SwiftUI + +/// A borderless text field that grabs first-responder focus as soon as it is +/// placed in a window. Used inside the hoisted output-type picker popover where +/// SwiftUI's `@FocusState` does not bridge across the window-anchored host, so +/// programmatic focus there is unreliable. This drops to AppKit and calls +/// `makeFirstResponder` directly, which works regardless of view-tree hoisting. +/// +/// Focus is grabbed once (guarded by the coordinator) so a re-render does not +/// keep stealing focus back from the user. +struct AutoFocusTextField: NSViewRepresentable { + @Binding var text: String + var autoFocus: Bool = true + + func makeNSView(context: Context) -> NSTextField { + let field = NSTextField() + field.delegate = context.coordinator + field.isBordered = false + field.isBezeled = false + field.drawsBackground = false + field.focusRingType = .none + field.font = .systemFont(ofSize: NSFont.systemFontSize) + field.lineBreakMode = .byTruncatingTail + field.usesSingleLineMode = true + field.cell?.isScrollable = true + field.cell?.wraps = false + field.stringValue = text + field.setContentHuggingPriority(.defaultLow, for: .horizontal) + field.setContentCompressionResistancePriority(.defaultLow, for: .horizontal) + return field + } + + func updateNSView(_ nsView: NSTextField, context: Context) { + if nsView.stringValue != text { + nsView.stringValue = text + } + guard autoFocus, !context.coordinator.didFocus else { return } + // Defer until the field is actually in a window and the responder chain + // has settled (the popover slides in), then make it first responder. + DispatchQueue.main.async { + guard let window = nsView.window, !context.coordinator.didFocus else { return } + context.coordinator.didFocus = true + window.makeFirstResponder(nsView) + } + } + + func makeCoordinator() -> Coordinator { + Coordinator(text: $text) + } + + final class Coordinator: NSObject, NSTextFieldDelegate { + private let text: Binding + var didFocus = false + + init(text: Binding) { + self.text = text + } + + func controlTextDidChange(_ notification: Notification) { + guard let field = notification.object as? NSTextField else { return } + text.wrappedValue = field.stringValue + } + } +} diff --git a/Sources/KeyPathAppKit/UI/Overlay/LiveKeyboardOverlayController.swift b/Sources/KeyPathAppKit/UI/Overlay/LiveKeyboardOverlayController.swift index f0856c8ec..13b1595b2 100644 --- a/Sources/KeyPathAppKit/UI/Overlay/LiveKeyboardOverlayController.swift +++ b/Sources/KeyPathAppKit/UI/Overlay/LiveKeyboardOverlayController.swift @@ -653,7 +653,11 @@ final class LiveKeyboardOverlayController: NSObject, NSWindowDelegate { let wrappedContent = buildRootView() - let hostingView = NSHostingView(rootView: wrappedContent) + // FirstMouseHostingView (not plain NSHostingView) so clicks register on + // the very first mouse-down even while the non-focus-stealing overlay is + // not key — otherwise hoisted controls (mapper output-type picker rows) + // swallow the activation click and appear unclickable. + let hostingView = FirstMouseHostingView(rootView: wrappedContent) hostingView.setFrameSize(initialSize) self.hostingView = hostingView diff --git a/Sources/KeyPathAppKit/UI/Overlay/LiveKeyboardOverlayTypes.swift b/Sources/KeyPathAppKit/UI/Overlay/LiveKeyboardOverlayTypes.swift index f00b75667..538900e07 100644 --- a/Sources/KeyPathAppKit/UI/Overlay/LiveKeyboardOverlayTypes.swift +++ b/Sources/KeyPathAppKit/UI/Overlay/LiveKeyboardOverlayTypes.swift @@ -139,6 +139,25 @@ final class OverlayWindow: NSWindow { } } +// MARK: - First-Mouse Hosting View + +/// Hosting view that processes the *first* click even when the overlay window is +/// not the key window. +/// +/// The overlay is intentionally non-focus-stealing (`OverlayWindow` only becomes +/// key when the user explicitly clicks into it). Without accepting first mouse, +/// a click on an interactive control rendered in a hoisted layer — notably the +/// mapper output-type picker rows in the window-anchored popover — is swallowed +/// as a window-activation event instead of hitting the control, so the rows +/// appear unclickable. The drag-header already overrides this for window +/// dragging (`NativeDragNSView`); this extends the same behavior to all overlay +/// content so the very first click lands on the control under the cursor. +final class FirstMouseHostingView: NSHostingView { + override func acceptsFirstMouse(for _: NSEvent?) -> Bool { + true + } +} + @MainActor final class OneShotLayerOverrideState { private(set) var currentLayer: String? diff --git a/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+LaunchApps.swift b/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+LaunchApps.swift index efc8d8f26..3ce7cdd8d 100644 --- a/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+LaunchApps.swift +++ b/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+LaunchApps.swift @@ -6,19 +6,16 @@ extension OverlayMapperSection { /// Content shown when Launch Apps section is expanded var launchAppsExpandedContent: some View { - VStack(spacing: 0) { - // Section header - HStack { - Text("Known Apps") - .font(.caption) - .foregroundStyle(.secondary) - Spacer() - } - .padding(.horizontal, 12) - .padding(.top, 10) - .padding(.bottom, 4) + let query = viewModel.launchAppSearchText.trimmingCharacters(in: .whitespaces) + let filteredApps = query.isEmpty + ? knownApps + : knownApps.filter { $0.name.localizedCaseInsensitiveContains(query) } - // List of known apps + return VStack(spacing: 0) { + // Search / filter field + launchAppSearchField + + // List of known apps (filtered) if knownApps.isEmpty { HStack { Text("No apps configured yet") @@ -28,8 +25,17 @@ extension OverlayMapperSection { } .padding(.horizontal, 12) .padding(.vertical, 8) + } else if filteredApps.isEmpty { + HStack { + Text("No apps match “\(query)”") + .font(.caption) + .foregroundStyle(.tertiary) + Spacer() + } + .padding(.horizontal, 12) + .padding(.vertical, 8) } else { - ForEach(knownApps, id: \.name) { app in + ForEach(filteredApps, id: \.name) { app in knownAppRow(app) } } @@ -57,6 +63,40 @@ extension OverlayMapperSection { } } + /// Search box that filters the known-apps list by name. + private var launchAppSearchField: some View { + HStack(spacing: 6) { + Image(systemName: "magnifyingglass") + .font(.caption) + .foregroundStyle(.secondary) + AutoFocusTextField(text: $viewModel.launchAppSearchText) + .frame(height: 18) + .accessibilityLabel("Search apps") + .accessibilityIdentifier("overlay-launch-app-search") + if !viewModel.launchAppSearchText.isEmpty { + Button { + viewModel.launchAppSearchText = "" + } label: { + Image(systemName: "xmark.circle.fill") + .font(.caption) + .foregroundStyle(.tertiary) + } + .buttonStyle(.plain) + .focusable(false) + .accessibilityIdentifier("overlay-launch-app-search-clear") + } + } + .padding(.horizontal, 10) + .padding(.vertical, 7) + .background( + RoundedRectangle(cornerRadius: 6, style: .continuous) + .fill(Color.primary.opacity(0.06)) + ) + .padding(.horizontal, 10) + .padding(.top, 8) + .padding(.bottom, 4) + } + /// Button for a known app in the list private func knownAppRow(_ app: AppLaunchInfo) -> some View { let isSelected = viewModel.selectedApp?.bundleIdentifier == app.bundleIdentifier @@ -70,7 +110,7 @@ extension OverlayMapperSection { viewModel.selectedFolder = nil viewModel.selectedScript = nil viewModel.clearShiftedOutput() - selectedLayerOutput = nil + viewModel.selectedLayerOutput = nil viewModel.outputLabel = app.name isSystemActionPickerOpen = false diff --git a/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+Layers.swift b/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+Layers.swift index 64a2c1e8b..e056594b9 100644 --- a/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+Layers.swift +++ b/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+Layers.swift @@ -49,7 +49,7 @@ extension OverlayMapperSection { /// Button for a layer in the list private func layerRow(_ layer: String) -> some View { - let isSelected = selectedLayerOutput == layer + let isSelected = viewModel.selectedLayerOutput == layer let isSystemLayer = viewModel.isSystemLayer(layer) let layerIdentifier = layer .lowercased() @@ -84,7 +84,7 @@ extension OverlayMapperSection { /// Select a layer as the output action func selectLayerOutput(_ layer: String) { - selectedLayerOutput = layer + viewModel.selectedLayerOutput = layer viewModel.selectedApp = nil viewModel.selectedSystemAction = nil viewModel.selectedURL = nil diff --git a/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+OutputTypeDropdown.swift b/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+OutputTypeDropdown.swift index 8e8e4aa02..ac4c432c3 100644 --- a/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+OutputTypeDropdown.swift +++ b/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+OutputTypeDropdown.swift @@ -30,7 +30,7 @@ extension OverlayMapperSection { ("Open Folder", "folder.fill", false) } else if viewModel.selectedScript != nil { ("Run Script", "terminal.fill", false) - } else if selectedLayerOutput != nil { + } else if viewModel.selectedLayerOutput != nil { ("Go to Layer", "square.stack.3d.up", false) } else if viewModel.hasShiftedOutputConfigured { ("Keystroke + ⇧", "keyboard", false) diff --git a/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+ShiftOutput.swift b/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+ShiftOutput.swift index 28dfe6982..1708fac9e 100644 --- a/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+ShiftOutput.swift +++ b/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+ShiftOutput.swift @@ -95,7 +95,7 @@ extension OverlayMapperSection { if viewModel.selectedURL != nil { return "Opens URL" } - if selectedLayerOutput != nil { + if viewModel.selectedLayerOutput != nil { return "Goes to a layer" } return "Types \(viewModel.outputLabel)" diff --git a/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+SystemActionGroups.swift b/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+SystemActionGroups.swift index 5c6fcd849..a033d1f46 100644 --- a/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+SystemActionGroups.swift +++ b/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection+SystemActionGroups.swift @@ -1,122 +1,32 @@ import AppKit import SwiftUI -private struct SystemActionPopoverContentHeightKey: PreferenceKey { - static let defaultValue: CGFloat = 0 - - static func reduce(value: inout CGFloat, nextValue: () -> CGFloat) { - value = max(value, nextValue()) - } -} - extension OverlayMapperSection { - /// Keep the picker compact by default, then grow it to a larger scrollable surface - /// whenever any section expands. Using an explicit height lets the popover resize - /// with the presented SwiftUI view instead of relying on a maxHeight that never - /// increases the popover's ideal size. - private var collapsedPopoverHeight: CGFloat { - 270 - } - - private var hasExpandedSection: Bool { - isSystemActionsExpanded || isLaunchAppsExpanded || isLayersExpanded - } - - private var maximumPopoverHeight: CGFloat { - let visibleScreenHeight = NSScreen.main?.visibleFrame.height ?? 900 - return min(collapsedPopoverHeight * 2, max(collapsedPopoverHeight, visibleScreenHeight - 160)) + // MARK: - Output-Type Picker (drill-down) + + /// Fixed popover size. A stable frame is the whole point of the drill-down: + /// the picker swaps pages instead of growing in place, so the hoisted + /// window-anchored layer never has to re-measure or reposition mid-interaction + /// (which is what left the old inline-expand rows unresponsive). + private var outputPickerWidth: CGFloat { + 280 } - private var popoverHeight: CGFloat { - guard hasExpandedSection else { return collapsedPopoverHeight } - let measuredHeight = max(collapsedPopoverHeight, outputActionPopoverContentHeight) - return min(measuredHeight, maximumPopoverHeight) + private var outputPickerHeight: CGFloat { + 340 } - /// Popover content for output type picker with collapsible sections + /// Output-type picker. An iPhone-style drill-down: the root page lists the + /// output types; the three with sub-choices (System Action, Launch App, Go + /// to Layer) slide over to their own page rather than expanding in place. var systemActionPopover: some View { - let isKeystrokeSelected = viewModel.selectedSystemAction == nil && viewModel.selectedApp == nil && selectedLayerOutput == nil && viewModel.selectedURL == nil && viewModel - .selectedFolder == nil && viewModel.selectedScript == nil - let isSystemActionSelected = viewModel.selectedSystemAction != nil - let isAppSelected = viewModel.selectedApp != nil - let isLayerSelected = selectedLayerOutput != nil - let isURLSelected = viewModel.selectedURL != nil - let isFolderSelected = viewModel.selectedFolder != nil - let isScriptSelected = viewModel.selectedScript != nil - - return ScrollView { - VStack(spacing: 0) { - // "Keystroke" option - Button { - collapseAllSections() - selectedLayerOutput = nil - viewModel.revertToKeystroke() - isSystemActionPickerOpen = false - } label: { - HStack(spacing: 10) { - Image(systemName: isKeystrokeSelected ? "checkmark.circle.fill" : "circle") - .font(.title3) - .foregroundStyle(isKeystrokeSelected ? Color.accentColor : .secondary) - .frame(width: 24) - Image(systemName: "keyboard") - .font(.body) - .frame(width: 20) - Text("Keystroke") - .font(.body) - Spacer() - } - .foregroundStyle(.primary) - .padding(.horizontal, 12) - .padding(.vertical, 10) - .contentShape(Rectangle()) - } - .buttonStyle(LayerPickerItemButtonStyle()) - .focusable(false) - .accessibilityIdentifier("overlay-mapper-output-keystroke") - - PopoverListDivider() - - // "System Action" option - clickable to expand/collapse - Button { - withAnimation(.easeInOut(duration: 0.25)) { - if !isSystemActionsExpanded { - isLaunchAppsExpanded = false - isLayersExpanded = false - } - isSystemActionsExpanded.toggle() - } - } label: { - HStack(spacing: 10) { - Image(systemName: isSystemActionSelected ? "checkmark.circle.fill" : "circle") - .font(.title3) - .foregroundStyle(isSystemActionSelected ? Color.accentColor : .secondary) - .frame(width: 24) - Image(systemName: "gearshape") - .font(.body) - .frame(width: 20) - Text("System Action") - .font(.body) - Spacer() - if let action = viewModel.selectedSystemAction { - Text(action.name) - .font(.caption) - .foregroundStyle(.secondary) - } - Image(systemName: isSystemActionsExpanded ? "chevron.up" : "chevron.down") - .font(.caption) - .foregroundStyle(.secondary) - } - .foregroundStyle(.primary) - .padding(.horizontal, 12) - .padding(.vertical, 10) - .contentShape(Rectangle()) - } - .buttonStyle(LayerPickerItemButtonStyle()) - .focusable(false) - .accessibilityIdentifier("overlay-mapper-output-system-action") - - // Collapsible system actions grid - if isSystemActionsExpanded { + ZStack(alignment: .top) { + switch viewModel.outputPickerPage { + case .root: + outputPickerRootPage + .transition(.move(edge: .leading)) + case .systemActions: + outputPickerSubPage(title: "System Action") { SystemActionGridView( groups: OutputActionGrouping.detailed, selectedActionID: viewModel.selectedSystemAction?.id, @@ -126,265 +36,278 @@ extension OverlayMapperSection { isSystemActionPickerOpen = false } ) - .transition(.opacity.combined(with: .move(edge: .top))) + } + .transition(.move(edge: .trailing)) + case .launchApps: + outputPickerSubPage(title: "Launch App") { + launchAppsExpandedContent + } + .transition(.move(edge: .trailing)) + case .layers: + outputPickerSubPage(title: "Go to Layer") { + layersExpandedContent + } + .transition(.move(edge: .trailing)) + } + } + .frame(width: outputPickerWidth, height: outputPickerHeight, alignment: .top) + .pickerPopoverChrome() + .onAppear { + // Always open at the root list; the selected type is shown with a + // checkmark there, and a predictable entry point avoids surprising + // the user with a deep page on reopen. + viewModel.outputPickerPage = .root + viewModel.launchAppSearchText = "" + } + .sheet(isPresented: $showingNewLayerDialog) { + newLayerDialogContent + } + } + + // MARK: - Root Page + + private var outputPickerRootPage: some View { + let isKeystrokeSelected = viewModel.selectedSystemAction == nil + && viewModel.selectedApp == nil + && viewModel.selectedLayerOutput == nil + && viewModel.selectedURL == nil + && viewModel.selectedFolder == nil + && viewModel.selectedScript == nil + + return ScrollView { + VStack(spacing: 0) { + outputTypeLeafRow( + title: "Keystroke", + icon: "keyboard", + isSelected: isKeystrokeSelected, + accessibilityID: "overlay-mapper-output-keystroke" + ) { + viewModel.selectedLayerOutput = nil + viewModel.revertToKeystroke() + isSystemActionPickerOpen = false } PopoverListDivider() - // "Launch App" option - clickable to expand/collapse - Button { - withAnimation(.easeInOut(duration: 0.25)) { - if !isLaunchAppsExpanded { - isSystemActionsExpanded = false - isLayersExpanded = false - } - isLaunchAppsExpanded.toggle() - if isLaunchAppsExpanded { - loadKnownApps() - } - } - } label: { - HStack(spacing: 10) { - Image(systemName: isAppSelected ? "checkmark.circle.fill" : "circle") - .font(.title3) - .foregroundStyle(isAppSelected ? Color.accentColor : .secondary) - .frame(width: 24) - if let app = viewModel.selectedApp { - Image(nsImage: app.icon) - .resizable() - .frame(width: 20, height: 20) - } else { - Image(systemName: "app.fill") - .font(.body) - .frame(width: 20) - } - Text(viewModel.selectedApp?.name ?? "Launch App") - .font(.body) - Spacer() - if let app = viewModel.selectedApp { - Text(app.name) - .font(.caption) - .foregroundStyle(.secondary) - } - Image(systemName: isLaunchAppsExpanded ? "chevron.up" : "chevron.down") - .font(.caption) - .foregroundStyle(.secondary) - } - .foregroundStyle(.primary) - .padding(.horizontal, 12) - .padding(.vertical, 10) - .contentShape(Rectangle()) + outputTypeNavRow( + title: "System Action", + icon: "gearshape", + isSelected: viewModel.selectedSystemAction != nil, + detail: viewModel.selectedSystemAction?.name, + accessibilityID: "overlay-mapper-output-system-action" + ) { + viewModel.outputPickerPage = .systemActions } - .buttonStyle(LayerPickerItemButtonStyle()) - .focusable(false) - .accessibilityIdentifier("overlay-mapper-output-launch-app") - // Collapsible known apps list - if isLaunchAppsExpanded { - launchAppsExpandedContent - .transition(.opacity.combined(with: .move(edge: .top))) + PopoverListDivider() + + outputTypeNavRow( + title: "Launch App", + icon: "app.fill", + customIcon: viewModel.selectedApp?.icon, + isSelected: viewModel.selectedApp != nil, + detail: viewModel.selectedApp?.name, + accessibilityID: "overlay-mapper-output-launch-app" + ) { + loadKnownApps() + viewModel.outputPickerPage = .launchApps } PopoverListDivider() - // "Open URL" option - Button { - collapseAllSections() + outputTypeLeafRow( + title: "Open URL", + icon: "link", + isSelected: viewModel.selectedURL != nil, + detail: viewModel.selectedURL.map { KeyMappingFormatter.extractDomain(from: $0) }, + accessibilityID: "overlay-mapper-output-url" + ) { isSystemActionPickerOpen = false viewModel.showURLInputDialog() - } label: { - HStack(spacing: 10) { - Image(systemName: isURLSelected ? "checkmark.circle.fill" : "circle") - .font(.title3) - .foregroundStyle(isURLSelected ? Color.accentColor : .secondary) - .frame(width: 24) - Image(systemName: "link") - .font(.body) - .frame(width: 20) - Text("Open URL") - .font(.body) - Spacer() - if let url = viewModel.selectedURL { - Text(KeyMappingFormatter.extractDomain(from: url)) - .font(.caption) - .foregroundStyle(.secondary) - } - } - .foregroundStyle(.primary) - .padding(.horizontal, 12) - .padding(.vertical, 10) - .contentShape(Rectangle()) } - .buttonStyle(LayerPickerItemButtonStyle()) - .focusable(false) - .accessibilityIdentifier("overlay-mapper-output-url") PopoverListDivider() - // "Open Folder" option - Button { - collapseAllSections() + outputTypeLeafRow( + title: "Open Folder", + icon: "folder.fill", + isSelected: viewModel.selectedFolder != nil, + detail: viewModel.selectedFolder.map { $0.name ?? URL(fileURLWithPath: $0.path).lastPathComponent }, + accessibilityID: "overlay-mapper-output-folder" + ) { isSystemActionPickerOpen = false browseForFolder() - } label: { - HStack(spacing: 10) { - Image(systemName: isFolderSelected ? "checkmark.circle.fill" : "circle") - .font(.title3) - .foregroundStyle(isFolderSelected ? Color.accentColor : .secondary) - .frame(width: 24) - Image(systemName: "folder.fill") - .font(.body) - .frame(width: 20) - Text("Open Folder") - .font(.body) - Spacer() - if let folder = viewModel.selectedFolder { - Text(folder.name ?? URL(fileURLWithPath: folder.path).lastPathComponent) - .font(.caption) - .foregroundStyle(.secondary) - } - } - .foregroundStyle(.primary) - .padding(.horizontal, 12) - .padding(.vertical, 10) - .contentShape(Rectangle()) } - .buttonStyle(LayerPickerItemButtonStyle()) - .focusable(false) - .accessibilityIdentifier("overlay-mapper-output-folder") PopoverListDivider() - // "Run Script" option - Button { - collapseAllSections() + outputTypeLeafRow( + title: "Run Script", + icon: "terminal.fill", + isSelected: viewModel.selectedScript != nil, + detail: viewModel.selectedScript.map { $0.name ?? URL(fileURLWithPath: $0.path).lastPathComponent }, + accessibilityID: "overlay-mapper-output-script" + ) { isSystemActionPickerOpen = false browseForScript() - } label: { - HStack(spacing: 10) { - Image(systemName: isScriptSelected ? "checkmark.circle.fill" : "circle") - .font(.title3) - .foregroundStyle(isScriptSelected ? Color.accentColor : .secondary) - .frame(width: 24) - Image(systemName: "terminal.fill") - .font(.body) - .frame(width: 20) - Text("Run Script") - .font(.body) - Spacer() - if let script = viewModel.selectedScript { - Text(script.name ?? URL(fileURLWithPath: script.path).lastPathComponent) - .font(.caption) - .foregroundStyle(.secondary) - } - } - .foregroundStyle(.primary) - .padding(.horizontal, 12) - .padding(.vertical, 10) - .contentShape(Rectangle()) } - .buttonStyle(LayerPickerItemButtonStyle()) - .focusable(false) - .accessibilityIdentifier("overlay-mapper-output-script") PopoverListDivider() - // "Go to Layer" option - clickable to expand/collapse - Button { - withAnimation(.easeInOut(duration: 0.25)) { - if !isLayersExpanded { - isSystemActionsExpanded = false - isLaunchAppsExpanded = false - } - isLayersExpanded.toggle() - if isLayersExpanded { - Task { await viewModel.refreshAvailableLayers() } - } - } - } label: { - HStack(spacing: 10) { - Image(systemName: isLayerSelected ? "checkmark.circle.fill" : "circle") - .font(.title3) - .foregroundStyle(isLayerSelected ? Color.accentColor : .secondary) - .frame(width: 24) - Image(systemName: "square.stack.3d.up") - .font(.body) - .frame(width: 20) - Text("Go to Layer") - .font(.body) - Spacer() - if let layer = selectedLayerOutput { - Text(layer.capitalized) - .font(.caption) - .foregroundStyle(.secondary) - } - Image(systemName: isLayersExpanded ? "chevron.up" : "chevron.down") - .font(.caption) - .foregroundStyle(.secondary) - } - .foregroundStyle(.primary) - .padding(.horizontal, 12) - .padding(.vertical, 10) - .contentShape(Rectangle()) - } - .buttonStyle(LayerPickerItemButtonStyle()) - .focusable(false) - .accessibilityIdentifier("overlay-mapper-output-go-to-layer") - - // Collapsible layers list - if isLayersExpanded { - layersExpandedContent - .transition(.opacity.combined(with: .move(edge: .top))) + outputTypeNavRow( + title: "Go to Layer", + icon: "square.stack.3d.up", + isSelected: viewModel.selectedLayerOutput != nil, + detail: viewModel.selectedLayerOutput?.capitalized, + accessibilityID: "overlay-mapper-output-go-to-layer" + ) { + Task { await viewModel.refreshAvailableLayers() } + viewModel.outputPickerPage = .layers } } .padding(.vertical, 6) - .background( - GeometryReader { geometry in - Color.clear.preference( - key: SystemActionPopoverContentHeightKey.self, - value: geometry.size.height - ) - } - ) } .scrollBounceBehavior(.basedOnSize) - .frame(width: 280, height: popoverHeight, alignment: .top) - .pickerPopoverChrome() - .animation(.easeInOut(duration: 0.25), value: popoverHeight) - .animation(.easeInOut(duration: 0.25), value: isSystemActionsExpanded) - .animation(.easeInOut(duration: 0.25), value: isLaunchAppsExpanded) - .animation(.easeInOut(duration: 0.25), value: isLayersExpanded) - .onPreferenceChange(SystemActionPopoverContentHeightKey.self) { measuredHeight in - let snappedHeight = ceil(measuredHeight) - guard abs(outputActionPopoverContentHeight - snappedHeight) > 1 else { return } - withAnimation(.easeInOut(duration: 0.25)) { - outputActionPopoverContentHeight = snappedHeight + } + + // MARK: - Sub-Page Scaffold + + /// A sub-page: a back header that returns to the root list, then the + /// category's content. The content reuses the same leaf-row lists the + /// inline version used (`launchAppsExpandedContent`, `layersExpandedContent`, + /// `SystemActionGridView`); each row selects and closes the popover. + private func outputPickerSubPage( + title: String, + @ViewBuilder content: () -> some View + ) -> some View { + VStack(spacing: 0) { + Button { + withAnimation(.easeInOut(duration: 0.22)) { + viewModel.outputPickerPage = .root + } + } label: { + HStack(spacing: 6) { + Image(systemName: "chevron.left") + .font(.body.weight(.semibold)) + Text(title) + .font(.body.weight(.semibold)) + Spacer() + } + .foregroundStyle(.primary) + .padding(.horizontal, 12) + .padding(.vertical, 10) + .contentShape(Rectangle()) } - } - .onAppear { - // Auto-expand the relevant section based on current selection - isSystemActionsExpanded = viewModel.selectedSystemAction != nil - isLaunchAppsExpanded = viewModel.selectedApp != nil - isLayersExpanded = selectedLayerOutput != nil - outputActionPopoverContentHeight = collapsedPopoverHeight - if isLaunchAppsExpanded { - loadKnownApps() + .buttonStyle(LayerPickerItemButtonStyle()) + .focusable(false) + .accessibilityIdentifier("overlay-mapper-output-back") + + PopoverListDivider() + + ScrollView { + content() + .padding(.vertical, 4) } + .scrollBounceBehavior(.basedOnSize) } - .sheet(isPresented: $showingNewLayerDialog) { - newLayerDialogContent + } + + // MARK: - Row Builders + + /// A terminal output type: selecting it applies the output and closes the + /// popover (Keystroke, Open URL, Open Folder, Run Script). + private func outputTypeLeafRow( + title: String, + icon: String, + isSelected: Bool, + detail: String? = nil, + accessibilityID: String, + action: @escaping () -> Void + ) -> some View { + Button(action: action) { + HStack(spacing: 10) { + Image(systemName: isSelected ? "checkmark.circle.fill" : "circle") + .font(.title3) + .foregroundStyle(isSelected ? Color.accentColor : .secondary) + .frame(width: 24) + Image(systemName: icon) + .font(.body) + .frame(width: 20) + Text(title) + .font(.body) + Spacer() + if let detail { + Text(detail) + .font(.caption) + .foregroundStyle(.secondary) + .lineLimit(1) + } + } + .foregroundStyle(.primary) + .padding(.horizontal, 12) + .padding(.vertical, 10) + .contentShape(Rectangle()) } + .buttonStyle(LayerPickerItemButtonStyle()) + .focusable(false) + .accessibilityIdentifier(accessibilityID) } - /// Collapse all expandable sections - private func collapseAllSections() { - withAnimation(.easeInOut(duration: 0.2)) { - isSystemActionsExpanded = false - isLaunchAppsExpanded = false - isLayersExpanded = false + /// An output type with sub-choices: tapping it drills into a sub-page. + /// The trailing chevron points right to signal navigation. + private func outputTypeNavRow( + title: String, + icon: String, + customIcon: NSImage? = nil, + isSelected: Bool, + detail: String? = nil, + accessibilityID: String, + navigate: @escaping () -> Void + ) -> some View { + Button { + withAnimation(.easeInOut(duration: 0.22)) { + navigate() + } + } label: { + HStack(spacing: 10) { + Image(systemName: isSelected ? "checkmark.circle.fill" : "circle") + .font(.title3) + .foregroundStyle(isSelected ? Color.accentColor : .secondary) + .frame(width: 24) + if let customIcon { + Image(nsImage: customIcon) + .resizable() + .frame(width: 20, height: 20) + } else { + Image(systemName: icon) + .font(.body) + .frame(width: 20) + } + Text(title) + .font(.body) + Spacer() + if let detail { + Text(detail) + .font(.caption) + .foregroundStyle(.secondary) + .lineLimit(1) + } + Image(systemName: "chevron.right") + .font(.caption) + .foregroundStyle(.secondary) + } + .foregroundStyle(.primary) + .padding(.horizontal, 12) + .padding(.vertical, 10) + .contentShape(Rectangle()) } + .buttonStyle(LayerPickerItemButtonStyle()) + .focusable(false) + .accessibilityIdentifier(accessibilityID) } + // MARK: - Folder / Script Pickers + /// Browse for a folder using NSOpenPanel private func browseForFolder() { let panel = NSOpenPanel() diff --git a/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection.swift b/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection.swift index 1a43ad3b8..34e4b7266 100644 --- a/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection.swift +++ b/Sources/KeyPathAppKit/UI/Overlay/OverlayMapperSection.swift @@ -24,17 +24,15 @@ struct OverlayMapperSection: View { @State var showTapCountPicker = false @State var showingAppPickerSheet = false @State var showingResetDialog = false - @State var isSystemActionsExpanded = false - @State var isLaunchAppsExpanded = false - @State var isLayersExpanded = false - @State var outputActionPopoverContentHeight: CGFloat = 0 + // Output-type picker navigation (page + selectedLayerOutput) lives on + // MapperViewModel — the picker is rendered in a hoisted window-anchored + // layer where view @State does not propagate. See MapperViewModel + // "Output-Type Picker (Overlay) Navigation State". @State var knownApps: [AppLaunchInfo] = [] @State var showingNewLayerDialog = false @State var newLayerName = "" @State var isHoldVariantPopoverOpen = false @State var showMultiTapSlideOver = false - /// Currently selected layer for "Go to Layer" output - @State var selectedLayerOutput: String? /// Selected tap count (1 = single, 2 = double, 3 = triple) @State var selectedTapCount: Int = 1 /// Cached set of installed pack IDs (for green dot on visual-only packs) diff --git a/Sources/KeyPathAppKit/UI/Overlay/Popover/WindowAnchoredPopover.swift b/Sources/KeyPathAppKit/UI/Overlay/Popover/WindowAnchoredPopover.swift index d2398dd64..d9aefc612 100644 --- a/Sources/KeyPathAppKit/UI/Overlay/Popover/WindowAnchoredPopover.swift +++ b/Sources/KeyPathAppKit/UI/Overlay/Popover/WindowAnchoredPopover.swift @@ -27,9 +27,9 @@ public extension View { /// choice for triggers in a right-side sidebar. The host auto-flips /// to the opposite edge if the popover would clip the host bounds. /// - gap: Space between the trigger edge and the popover edge. - /// - content: The popover content. Built once when `isPresented` - /// becomes `true` and cached until dismissal, so heavier content - /// doesn't pay a rebuild cost on every layout pass. + /// - content: The popover content. Rebuilt from live state as the host + /// re-renders, so it always reflects the trigger view's current state + /// (selection, drill-down page, etc.) rather than a snapshot from open. func windowAnchoredPopover( isPresented: Binding, edge: WindowAnchoredPopoverEdge = .leading, @@ -57,28 +57,25 @@ public extension View { // MARK: - Internal Entry -struct WindowAnchoredPopoverEntry: Identifiable, Equatable, @unchecked Sendable { +struct WindowAnchoredPopoverEntry: Identifiable, @unchecked Sendable { let id: UUID let anchor: Anchor let edge: WindowAnchoredPopoverEdge let gap: CGFloat /// Type-erased so any caller can attach arbitrary popover content /// without bubbling a generic parameter all the way to the host. - /// The trade-off is that SwiftUI can't diff inside the popover - /// subtree across `entries` updates — acceptable for picker-style - /// content; revisit if the host renders large dynamic trees. let content: AnyView let dismiss: () -> Void - /// Identity-only equality. `anchor` is `Anchor` (not Equatable), - /// `content` is `AnyView` (untyped), and `dismiss` is a closure — none of - /// these can participate. Comparing only `id` also matches our semantic - /// intent: the host only needs to know when the *set of open popovers* - /// changes (so it can rebind the ESC monitor). Layout-driven anchor - /// movement should not retrigger that work. - static func == (lhs: WindowAnchoredPopoverEntry, rhs: WindowAnchoredPopoverEntry) -> Bool { - lhs.id == rhs.id - } + // Deliberately NOT `Equatable`. An identity-only `==` (comparing just `id`) + // used to live here, but it caused SwiftUI to treat a same-id entry whose + // *content* changed as unchanged — skipping both the preference update and + // the content re-render. The result: the popover kept showing stale content + // (e.g. the picker never navigated to a tapped sub-page even though the + // model and rebuilt content were both correct). Without `Equatable`, the + // host re-renders the content whenever it is re-emitted, so it always + // reflects the latest state. Work that only cares about the *set* of open + // popovers (the ESC monitor) keys on `entries.map(\.id)` instead. } struct WindowAnchoredPopoverPreferenceKey: PreferenceKey { @@ -102,37 +99,33 @@ private struct WindowAnchoredPopoverModifier: ViewModifier /// reused for every emitted entry from this modifier. @State private var id = UUID() - /// Cached popover content. Built once when `isPresented` flips to `true` - /// and held until dismissal, so the popover view tree is not - /// re-instantiated on every layout pass (which the surrounding - /// `anchorPreference` transform would otherwise trigger). - @State private var cachedContent: AnyView? - func body(content: Content) -> some View { content .anchorPreference( key: WindowAnchoredPopoverPreferenceKey.self, value: .bounds ) { anchor in - guard isPresented, let cachedContent else { return [] } + guard isPresented else { return [] } + // Build the content fresh each time the preference is + // recomputed rather than caching it once on open. The popover + // body depends on the trigger view's live state (e.g. the + // mapper picker's expand/selection flags); a one-shot cache + // froze that state, so expanding a section or changing the + // selection never re-rendered and the rows looked dead to + // clicks. Rebuilding ties content freshness to the source + // view's re-renders. The build is a lightweight picker tree; + // structural identity is stable so in-popover state persists. return [ WindowAnchoredPopoverEntry( id: id, anchor: anchor, edge: edge, gap: gap, - content: cachedContent, + content: AnyView(contentBuilder()), dismiss: { isPresented = false } ) ] } - .onChange(of: isPresented) { _, isOpen in - if isOpen { - cachedContent = AnyView(contentBuilder()) - } else { - cachedContent = nil - } - } } } @@ -140,6 +133,7 @@ private struct WindowAnchoredPopoverModifier: ViewModifier private struct WindowAnchoredPopoverHostModifier: ViewModifier { @State private var escMonitor: Any? + @State private var outsideClickMonitor: Any? func body(content: Content) -> some View { content.overlayPreferenceValue(WindowAnchoredPopoverPreferenceKey.self) { entries in @@ -158,8 +152,8 @@ private struct WindowAnchoredPopoverHostModifier: ViewModifier { .frame(width: 0, height: 0) .hidden() .allowsHitTesting(false) - .onChange(of: entries) { _, _ in removeEscMonitor() } - .onDisappear { removeEscMonitor() } + .onChange(of: entries.map(\.id)) { _, _ in removeDismissMonitors() } + .onDisappear { removeDismissMonitors() } } else { GeometryReader { proxy in ZStack(alignment: .topLeading) { @@ -187,41 +181,59 @@ private struct WindowAnchoredPopoverHostModifier: ViewModifier { .transition(.opacity) } } - .animation(.easeOut(duration: 0.12), value: entries) + .animation(.easeOut(duration: 0.12), value: entries.map(\.id)) } - .onChange(of: entries) { _, newEntries in - if newEntries.isEmpty { - removeEscMonitor() + // Key only on the set of open popover ids: the ESC monitor only + // needs rebinding when a popover opens/closes, not when content or + // anchor change on every layout pass. + .onChange(of: entries.map(\.id)) { _, ids in + if ids.isEmpty { + removeDismissMonitors() } else { - installEscMonitor(dismissAll: { for entry in newEntries { + installDismissMonitors(dismissAll: { for entry in entries { entry.dismiss() } }) } } .onAppear { - installEscMonitor(dismissAll: { for entry in entries { + installDismissMonitors(dismissAll: { for entry in entries { entry.dismiss() } }) } - .onDisappear { removeEscMonitor() } + .onDisappear { removeDismissMonitors() } } } - private func installEscMonitor(dismissAll: @escaping () -> Void) { - removeEscMonitor() + private func installDismissMonitors(dismissAll: @escaping () -> Void) { + removeDismissMonitors() + // ESC dismisses (local key monitor — no accessibility permission needed). escMonitor = NSEvent.addLocalMonitorForEvents(matching: [.keyDown]) { event in // 53 = kVK_Escape (no Carbon import needed for this single constant) guard event.keyCode == 53 else { return event } dismissAll() return nil } + // A click outside the app's own windows (another app, the desktop) + // dismisses too. The in-window backdrop can't see those events, so the + // popover would otherwise linger after the user clicks away. Global + // monitors only observe events destined for *other* apps, so this never + // fires for clicks on the overlay itself (those stay local). + outsideClickMonitor = NSEvent.addGlobalMonitorForEvents( + matching: [.leftMouseDown, .rightMouseDown, .otherMouseDown] + ) { _ in + dismissAll() + } } - private func removeEscMonitor() { + private func removeDismissMonitors() { if let monitor = escMonitor { NSEvent.removeMonitor(monitor) escMonitor = nil } + if let monitor = outsideClickMonitor { + NSEvent.removeMonitor(monitor) + outsideClickMonitor = nil + } } }