From cd2e3da330e65f1a9daf22e98a5d6cfe724e1509 Mon Sep 17 00:00:00 2001 From: Valdemar Lorenzen Date: Fri, 16 Oct 2026 15:36:04 +0200 Subject: [PATCH] fix: complete Command presentation teardown --- README.md | 2 + .../ApplicationActionsController.swift | 18 +++- Sources/Conjure/EmojiPickerController.swift | 6 +- .../Conjure/LauncherWindowController.swift | 27 +++--- TODO.md | 2 +- Tests/ConjureKitTests/EmojiPickerTests.swift | 85 ++++++++++++++++++- 6 files changed, 115 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 9cfbbfb..1de9570 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,8 @@ Use the arrow keys or Control-H/J/K/L to navigate left/down/up/right. Return, cl Open **Actions** with Command-K for **Copy and Paste** (default), **Copy Emoji** (copy only, no Accessibility permission needed), **Favorite**, or **Remove Favorite**. Favorites are saved locally and grouped ahead of other emoji, with a small extra spacing break when browsing without a search. There are no star badges. +With Actions open, Escape or the Back chevron closes Actions first; another Back returns to Applications and restores your search. Command-W dismisses the entire launcher, including the current Command and its Actions. Switching destinations also removes the previous Command's Actions completely. Copy and Copy and Paste dismiss the launcher without an intermediate Back step. + The picker works offline with 1,870 emoji from [gemoji v4.1.0](https://github.com/github/gemoji/tree/v4.1.0). Its [MIT license](Sources/ConjureKit/Resources/LICENSE-gemoji.txt) is included in the application. ## Translation diff --git a/Sources/Conjure/ApplicationActionsController.swift b/Sources/Conjure/ApplicationActionsController.swift index 8b5d414..2ef997c 100644 --- a/Sources/Conjure/ApplicationActionsController.swift +++ b/Sources/Conjure/ApplicationActionsController.swift @@ -192,8 +192,8 @@ final class ApplicationActionsController: NSViewController, NSSearchFieldDelegat } } - func close() { - guard isShown else { return } + func close(animated: Bool = true) { + guard viewIfLoaded?.superview != nil, !isClosing || !animated else { return } transitionGeneration += 1 let generation = transitionGeneration isClosing = true @@ -202,13 +202,23 @@ final class ApplicationActionsController: NSViewController, NSSearchFieldDelegat if let mouseMonitor { NSEvent.removeMonitor(mouseMonitor) } mouseMonitor = nil coveredPill?.setAccessibilityHidden(false) + if !animated { + view.removeFromSuperview() + coveredPill?.layer?.removeAllAnimations() + coveredPill?.alphaValue = coveredPillAlpha + coveredPill = nil + isClosing = false + closed?() + return + } + // Finish teardown even if the presenting Command releases this controller. NSAnimationContext.runAnimationGroup { context in context.duration = NSWorkspace.shared.accessibilityDisplayShouldReduceMotion ? 0 : 0.12 view.animator().alphaValue = 0 coveredPill?.animator().alphaValue = coveredPillAlpha - } completionHandler: { [weak self] in + } completionHandler: { [self] in MainActor.assumeIsolated { - guard let self, self.transitionGeneration == generation, self.isClosing else { return } + guard self.transitionGeneration == generation, self.isClosing else { return } self.view.removeFromSuperview() self.coveredPill = nil self.isClosing = false diff --git a/Sources/Conjure/EmojiPickerController.swift b/Sources/Conjure/EmojiPickerController.swift index e367721..88b95fd 100644 --- a/Sources/Conjure/EmojiPickerController.swift +++ b/Sources/Conjure/EmojiPickerController.swift @@ -110,10 +110,10 @@ final class EmojiPickerController: NSViewController, NSCollectionViewDataSource, @objc private func categoryChanged() { inputChanged() } @objc func goBack() { - if actions.isShown { actions.close() } else { closed?() } + if actions.isShown { actions.close() } else { close() } } func close() { - actions.close() + actions.close(animated: false) closed?() } override func cancelOperation(_ sender: Any?) { goBack() } @@ -254,7 +254,7 @@ final class EmojiPickerController: NSViewController, NSCollectionViewDataSource, var recent = defaults.stringArray(forKey: "recentEmoji") ?? [] recent.removeAll { $0 == value } defaults.set(Array(([value] + recent).prefix(30)), forKey: "recentEmoji") - closed?() + actions.close(animated: false) if pasting { pasteRequested?() } else { copied?() } } diff --git a/Sources/Conjure/LauncherWindowController.swift b/Sources/Conjure/LauncherWindowController.swift index 46554b7..7ac6c98 100644 --- a/Sources/Conjure/LauncherWindowController.swift +++ b/Sources/Conjure/LauncherWindowController.swift @@ -14,6 +14,8 @@ final class LauncherWindowController: NSWindowController, NSWindowDelegate, NSSe private let sectionLabel = NSTextField(labelWithString: "Applications") private let openButton = NSButton(title: "Open Application", target: nil, action: nil) private let history: LaunchHistory + private let defaults: UserDefaults + private let pasteboard: NSPasteboard private var applications: [Application] = Application.commands private var results: [Application] = [] private var calculation: CalculatorResult? @@ -49,9 +51,12 @@ final class LauncherWindowController: NSWindowController, NSWindowDelegate, NSSe } }() - init(history: LaunchHistory = LaunchHistory(), uninstaller: ApplicationUninstaller = ApplicationUninstaller()) { + init(history: LaunchHistory = LaunchHistory(), uninstaller: ApplicationUninstaller = ApplicationUninstaller(), + defaults: UserDefaults = .standard, pasteboard: NSPasteboard = .general) { self.history = history self.uninstaller = uninstaller + self.defaults = defaults + self.pasteboard = pasteboard let window = LauncherPanel( contentRect: NSRect(x: 0, y: 0, width: 750, height: 474), styleMask: [.borderless], @@ -118,7 +123,7 @@ final class LauncherWindowController: NSWindowController, NSWindowDelegate, NSSe return } closeCommand() - actions.close() + actions.close(animated: false) (window as? LauncherPanel)?.beginNumberHintSession() let frontmost = NSWorkspace.shared.frontmostApplication if frontmost?.processIdentifier != ProcessInfo.processInfo.processIdentifier { @@ -145,8 +150,6 @@ final class LauncherWindowController: NSWindowController, NSWindowDelegate, NSSe guard !isLaunching, window?.attachedSheet == nil else { return } if window?.isVisible == true, window?.isKeyWindow == true, launcherFrameBeforeReview == nil, activeCommand == command { - closeCommand() - actions.close() dismiss() } else { present(command: command) @@ -156,12 +159,9 @@ final class LauncherWindowController: NSWindowController, NSWindowDelegate, NSSe func dismiss(restoringFocus: Bool = true) { guard !restoringFocus || (!isLaunching && window?.attachedSheet == nil) else { return } if let review = uninstallReview { review.goBack(); return } - if isCommandOpen { - goBackCommand() - if restoringFocus { return } - } + closeCommand() guard launcherFrameBeforeReview == nil else { return } - actions.close() + actions.close(animated: false) alignmentGuides?.end() let previous = previousApplication previousApplication = nil @@ -712,7 +712,6 @@ final class LauncherWindowController: NSWindowController, NSWindowDelegate, NSSe guard !isLaunching, !isCommandOpen, launcherFrameBeforeReview == nil, window?.attachedSheet == nil else { return } if let calculation { guard tableView.selectedRow == 0, let value = calculation.copyValue else { return } - let pasteboard = NSPasteboard.general pasteboard.clearContents() if pasteboard.setString(value, forType: .string) { dismiss() @@ -763,7 +762,7 @@ final class LauncherWindowController: NSWindowController, NSWindowDelegate, NSSe private func pasteEmoji() { let target = previousApplication - let changeCount = NSPasteboard.general.changeCount + let changeCount = pasteboard.changeCount dismiss() guard let target, !target.isTerminated, target.processIdentifier != ProcessInfo.processInfo.processIdentifier, @@ -774,7 +773,7 @@ final class LauncherWindowController: NSWindowController, NSWindowDelegate, NSSe for _ in 0..<20 { try? await Task.sleep(for: .milliseconds(25)) guard let self, self.window?.isVisible == false, !target.isTerminated, - NSPasteboard.general.changeCount == changeCount else { return } + self.pasteboard.changeCount == changeCount else { return } guard NSWorkspace.shared.frontmostApplication?.processIdentifier == target.processIdentifier else { continue } guard let down = CGEvent(keyboardEventSource: nil, virtualKey: 9, keyDown: true), let up = CGEvent(keyboardEventSource: nil, virtualKey: 9, keyDown: false) else { return } @@ -812,7 +811,7 @@ final class LauncherWindowController: NSWindowController, NSWindowDelegate, NSSe placeholder = "Enter English text to translate…" accessibilityLabel = "English text to translate into Japanese" case .emoji: - let tool = EmojiPickerController() + let tool = EmojiPickerController(defaults: defaults, pasteboard: pasteboard) emojiPicker = tool tool.input = { [weak self] in self?.searchField.stringValue ?? "" } tool.focusSearch = { [weak self] in self?.window?.makeFirstResponder(self?.searchField) } @@ -847,7 +846,7 @@ final class LauncherWindowController: NSWindowController, NSWindowDelegate, NSSe } translation?.closed = close emojiPicker?.closed = close - actions.close() + actions.close(animated: false) (window as? LauncherPanel)?.resetNumberHints() openButton.keyEquivalent = "" searchSymbol.isHidden = true diff --git a/TODO.md b/TODO.md index 72efe4e..647adde 100644 --- a/TODO.md +++ b/TODO.md @@ -1,3 +1,3 @@ -- [ ] tighten Command presentation lifetime contracts: guarantee complete Command-owned Actions teardown and distinguish one-level Back from full dismissal or destination replacement. Switching Emoji β†’ Translate with Actions open currently leaves an invisible Actions view attached because removal depends on a weak-self animation completion. Fix ownership in the existing modules first; extract a presentation owner only if it removes distributed lifecycle knowledge. Cover nested Escape, query restoration, replacement with Actions open, and copy/paste dismissal. +- [x] tighten Command presentation lifetime contracts: guarantee complete Command-owned Actions teardown and distinguish one-level Back from full dismissal or destination replacement. Fixed in the existing controllers without extracting another presentation module. Covered nested Escape, query restoration, replacement with Actions open, and copy/paste dismissal. - [ ] add a notes app inspired by raycast notes - [ ] improve on the conjure translation command \ No newline at end of file diff --git a/Tests/ConjureKitTests/EmojiPickerTests.swift b/Tests/ConjureKitTests/EmojiPickerTests.swift index ded2793..f740300 100644 --- a/Tests/ConjureKitTests/EmojiPickerTests.swift +++ b/Tests/ConjureKitTests/EmojiPickerTests.swift @@ -19,7 +19,6 @@ import Testing picker.input = { query } picker.closed = { closed = true } picker.pasteRequested = { - #expect(closed) #expect(pasteboard.string(forType: .string) == EmojiCatalog.search(query)[1].value) pasteRequests += 1 } @@ -28,7 +27,7 @@ import Testing #expect(picker.handle(#selector(NSResponder.moveRight(_:)))) #expect(picker.handle(#selector(NSResponder.insertNewline(_:)))) #expect(pasteboard.string(forType: .string) == matches[1].value) - #expect(closed) + #expect(!closed) #expect(pasteRequests == 1) #expect(defaults.stringArray(forKey: "recentEmoji") == [matches[1].value]) @@ -85,13 +84,84 @@ import Testing #expect(pasteRequests == 1) } +@Test @MainActor func closingEmojiRemovesActionsEvenDuringBackAnimation() throws { + _ = NSApplication.shared + let suite = "ConjureEmojiLifetime.\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suite)! + let pasteboard = NSPasteboard.withUniqueName() + defer { defaults.removePersistentDomain(forName: suite); pasteboard.releaseGlobally() } + let picker = EmojiPickerController(defaults: defaults, pasteboard: pasteboard) + picker.input = { "πŸ‘©β€πŸ’»" } + let window = NSWindow(contentRect: NSRect(x: 0, y: 0, width: 750, height: 400), + styleMask: [.borderless], backing: .buffered, defer: false) + let host = try #require(window.contentView) + host.addSubview(picker.view) + picker.showActions() + let overlay = try #require(host.subviews.first { $0 !== picker.view }) + picker.goBack() + var closed = false + picker.closed = { closed = true } + picker.close() + #expect(closed) + #expect(overlay.superview == nil) +} + +@Test(arguments: ["replace", "dismiss", "copy", "paste"]) +@MainActor func commandFullExitRemovesEmojiAndItsActions(_ exit: String) throws { + _ = NSApplication.shared + let suite = "ConjureCommandExit.\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suite)! + let pasteboard = NSPasteboard.withUniqueName() + defer { defaults.removePersistentDomain(forName: suite); pasteboard.releaseGlobally() } + let launcher = LauncherWindowController( + history: LaunchHistory(defaults: defaults, now: { Date(timeIntervalSinceReferenceDate: 12_000) }), + defaults: defaults, pasteboard: pasteboard) + let window = try #require(launcher.window) + defer { window.orderOut(nil) } + let content = try #require(window.contentView) + func descendants(_ view: NSView) -> [NSView] { [view] + view.subviews.flatMap(descendants) } + let search = try #require(descendants(content).compactMap { $0 as? NSSearchField }.first) + search.stringValue = "emoji" + launcher.controlTextDidChange(Notification(name: NSControl.textDidChangeNotification, object: search)) + _ = launcher.control(search, textView: NSTextView(), doCommandBy: #selector(NSResponder.insertNewline(_:))) + search.stringValue = "πŸ‘©β€πŸ’»" + launcher.controlTextDidChange(Notification(name: NSControl.textDidChangeNotification, object: search)) + window.orderFront(nil) + let button = try #require(descendants(content).compactMap { $0 as? NSButton }.first { + $0.title == "Actions" && $0.target is EmojiPickerController + }) + let existing = Set(content.subviews.map(ObjectIdentifier.init)) + (button.target as? EmojiPickerController)?.showActions() + let overlay = try #require(content.subviews.last { !existing.contains(ObjectIdentifier($0)) }) + switch exit { + case "replace": launcher.present(command: .translate) + case "dismiss": window.performClose(nil) + case "copy": (button.target as? EmojiPickerController)?.copySelection() + default: _ = launcher.control(search, textView: NSTextView(), doCommandBy: #selector(NSResponder.insertNewline(_:))) + } + #expect(overlay.superview == nil) + #expect(!descendants(content).contains { $0 is NSCollectionView }) + if exit == "replace" { + #expect(search.accessibilityLabel() == "English text to translate into Japanese") + #expect(search.stringValue.isEmpty) + } else { + #expect(!window.isVisible) + #expect(search.stringValue == "emoji") + #expect(search.accessibilityLabel() == "Search applications, calculate, or convert units and currencies") + } + if exit == "copy" || exit == "paste" { + #expect(pasteboard.string(forType: .string) == "πŸ‘©β€πŸ’»") + #expect(defaults.stringArray(forKey: "recentEmoji") == ["πŸ‘©β€πŸ’»"]) + } +} + @Test @MainActor func emojiCommandRestoresLauncherSearchOnEscape() throws { _ = NSApplication.shared let suite = "ConjureEmojiCommand.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suite)! defer { defaults.removePersistentDomain(forName: suite) } let history = LaunchHistory(defaults: defaults, now: { Date(timeIntervalSinceReferenceDate: 12_000) }) - let launcher = LauncherWindowController(history: history) + let launcher = LauncherWindowController(history: history, defaults: defaults) let content = try #require(launcher.window?.contentView) func descendants(_ view: NSView) -> [NSView] { [view] + view.subviews.flatMap(descendants) } let search = try #require(descendants(content).compactMap { $0 as? NSSearchField }.first) @@ -103,6 +173,15 @@ import Testing launcher.controlTextDidChange(Notification(name: NSControl.textDidChangeNotification, object: search)) #expect(grid.numberOfItems(inSection: 0) == 1) #expect((grid.dataSource?.collectionView(grid, itemForRepresentedObjectAt: IndexPath(item: 0, section: 0)).view as? NSButton)?.title == "πŸ‘©β€πŸ’»") + let actionsButton = try #require(descendants(content).compactMap { $0 as? NSButton }.first { + $0.title == "Actions" && $0.target is EmojiPickerController + }) + (actionsButton.target as? EmojiPickerController)?.showActions() + _ = launcher.control(search, textView: NSTextView(), doCommandBy: #selector(NSResponder.cancelOperation(_:))) + #expect(descendants(content).contains { $0 === grid }) + #expect(search.stringValue == ":woman_technologist:") + let copyButton = try #require(descendants(content).compactMap { $0 as? NSButton }.first { $0.title == "Copy and Paste" }) + #expect(copyButton.keyEquivalent == "\r") _ = launcher.control(search, textView: NSTextView(), doCommandBy: #selector(NSResponder.cancelOperation(_:))) #expect(!descendants(content).contains { $0 === grid }) #expect(search.stringValue == "emoji")