-
-
Notifications
You must be signed in to change notification settings - Fork 470
feat: Add a search function that supports input filtering and activation via shortcut keys. #5216
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds a keyword search feature to the window switcher, allowing users to filter windows by app name and window title. The implementation includes a visual search field UI component, keyboard input handling that works with the existing shortcut system, and support for search activation via the 'S' key when "focus on release" mode is enabled.
Changes:
- Added a
SearchFieldViewUI component with rounded styling and icon support - Implemented search query state management and filtering logic in
Windows.swift - Extended keyboard event handling to capture search input while respecting modifier key constraints
- Integrated search state into the app lifecycle with proper reset on UI hide/show
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ui/main-window/ThumbnailsView.swift | Adds SearchFieldView class and integrates search field into the thumbnails view layout |
| src/logic/Windows.swift | Implements search query state management, filtering logic, and window visibility updates |
| src/logic/events/KeyboardEvents.swift | Adds keyboard input handling for search with modifier key validation and character filtering |
| src/ui/App.swift | Integrates search reset into app lifecycle and adds UI refresh method for search changes |
| resources/l10n/zh-CN.lproj/Localizable.strings | Adds Chinese translations for search UI strings |
| resources/l10n/en.lproj/Localizable.strings | Adds English search UI strings |
| resources/l10n/Localizable.strings | Adds localized search UI strings to main file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var scrollView: ScrollView! | ||
| var contentView: EffectView! | ||
| var rows = [[ThumbnailView]]() | ||
| private var searchField = SearchFieldView() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The searchField is already initialized at declaration on line 7, and then re-initialized in setupSearchField() on line 28. This redundant initialization should be removed. Consider either initializing only at declaration or only in the setup method.
| private var searchField = SearchFieldView() | |
| private var searchField: SearchFieldView! |
| } | ||
|
|
||
| private func setupSearchField() { | ||
| searchField = SearchFieldView() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The searchField is initialized here, but it was already initialized at property declaration on line 7. This is redundant and inefficient. Remove this line since the property is already initialized.
| searchField = SearchFieldView() |
| static func resetSearchQuery() { | ||
| searchQuery = "" | ||
| isSearchModeActive = false | ||
| } | ||
|
|
||
| @discardableResult | ||
| static func activateSearchMode() -> Bool { | ||
| guard !isSearchModeActive else { return false } | ||
| isSearchModeActive = true | ||
| if App.app.appIsBeingUsed { | ||
| App.app.forceDoNothingOnRelease = true | ||
| App.app.refreshOpenUiAfterSearchChange() | ||
| } | ||
| return true | ||
| } | ||
|
|
||
| @discardableResult | ||
| static func appendSearchQuery(_ text: String) -> Bool { | ||
| guard !text.isEmpty else { return false } | ||
| return setSearchQuery(searchQuery + text) | ||
| } | ||
|
|
||
| @discardableResult | ||
| static func removeLastSearchCharacter() -> Bool { | ||
| guard !searchQuery.isEmpty else { return false } | ||
| searchQuery.removeLast() | ||
| refreshUiAfterSearchChange() | ||
| return true | ||
| } | ||
|
|
||
| @discardableResult | ||
| static func setSearchQuery(_ query: String) -> Bool { | ||
| guard searchQuery != query else { return false } | ||
| searchQuery = query | ||
| if !searchQueryDisplayText.isEmpty { | ||
| isSearchModeActive = true | ||
| if App.app.appIsBeingUsed { | ||
| App.app.forceDoNothingOnRelease = true | ||
| } | ||
| } | ||
| refreshUiAfterSearchChange() | ||
| return true | ||
| } | ||
|
|
||
| private static func refreshUiAfterSearchChange() { | ||
| guard App.app.appIsBeingUsed else { return } | ||
| refreshVisibilityForSearch() | ||
| App.app.refreshOpenUiAfterSearchChange() | ||
| } | ||
|
|
||
| static func refreshVisibilityForSearch() { | ||
| for window in list { | ||
| refreshIfWindowShouldBeShownToTheUser(window) | ||
| } | ||
| refreshWhichWindowsToShowTheUser() | ||
| applySearchFilter() | ||
| sort() | ||
| } | ||
|
|
||
| private static func applySearchFilter() { | ||
| let tokens = searchTokens() | ||
| guard !tokens.isEmpty else { return } | ||
| for window in list where window.shouldShowTheUser { | ||
| window.shouldShowTheUser = matchesSearch(window, tokens) | ||
| } | ||
| } | ||
|
|
||
| private static func searchTokens() -> [String] { | ||
| let trimmed = searchQueryDisplayText | ||
| guard !trimmed.isEmpty else { return [] } | ||
| return trimmed.split(whereSeparator: { $0.isWhitespace }).map { normalizeSearchString(String($0)) } | ||
| } | ||
|
|
||
| private static func matchesSearch(_ window: Window, _ tokens: [String]) -> Bool { | ||
| let haystack = normalizedSearchText(window) | ||
| return tokens.allSatisfy { haystack.contains($0) } | ||
| } | ||
|
|
||
| private static func normalizedSearchText(_ window: Window) -> String { | ||
| let appName = window.application.localizedName ?? "" | ||
| let windowTitle = window.title ?? "" | ||
| return normalizeSearchString("\(appName) \(windowTitle)") | ||
| } | ||
|
|
||
| private static func normalizeSearchString(_ value: String) -> String { | ||
| value.folding(options: [.diacriticInsensitive, .caseInsensitive], locale: .current) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new search functionality, including search query handling, filtering, and keyboard input processing, lacks test coverage. Given that the project has existing unit tests (particularly KeyboardEventsTests), consider adding tests for the search-related methods such as searchTokens(), matchesSearch(), normalizeSearchString(), appendSearchQuery(), removeLastSearchCharacter(), and the search filter application logic.
| private static func handleSearchInputIfNeeded(_ event: NSEvent, shortcutTriggered: Bool) -> Bool { | ||
| guard App.app.appIsBeingUsed, event.type == .keyDown, !shortcutTriggered else { return false } | ||
| let cleanedModifiers = event.modifierFlags.cleaned() | ||
| guard isAllowedSearchModifiers(cleanedModifiers) else { return false } | ||
| if Windows.requiresSearchActivation && !Windows.isSearchModeActive { | ||
| if isSearchActivationKey(event) { | ||
| return Windows.activateSearchMode() | ||
| } | ||
| return false | ||
| } | ||
| if event.keyCode == kVK_Delete || event.keyCode == kVK_ForwardDelete { | ||
| return Windows.removeLastSearchCharacter() | ||
| } | ||
| guard let text = event.charactersIgnoringModifiers, !text.isEmpty else { return false } | ||
| if !isSearchableText(text) { return false } | ||
| if text.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty && !Windows.isSearchQueryActive { | ||
| return false | ||
| } | ||
| return Windows.appendSearchQuery(text) | ||
| } | ||
|
|
||
| private static func isSearchableText(_ text: String) -> Bool { | ||
| let blocked = CharacterSet.searchInputBlockedCharacters | ||
| return text.unicodeScalars.allSatisfy { !blocked.contains($0) } | ||
| } | ||
|
|
||
| private static func isSearchActivationKey(_ event: NSEvent) -> Bool { | ||
| guard let chars = event.charactersIgnoringModifiers, chars.count == 1 else { return false } | ||
| return chars.lowercased() == "s" | ||
| } | ||
|
|
||
| private static func isAllowedSearchModifiers(_ modifiers: NSEvent.ModifierFlags) -> Bool { | ||
| if modifiers.isEmpty { | ||
| return true | ||
| } | ||
| guard D100 let holdModifiers = currentHoldShortcutModifiers() else { return false } | ||
| let allowedTyping: NSEvent.ModifierFlags = [.shift, .capsLock] | ||
| let allowed = holdModifiers.union(allowedTyping) | ||
| return modifiers.isSubset(of: allowed) | ||
| } | ||
|
|
||
| private static func currentHoldShortcutModifiers() -> NSEvent.ModifierFlags? { | ||
| let shortcutIndex = App.app.shortcutIndex | ||
| let holdId = Preferences.indexToName("holdShortcut", shortcutIndex) | ||
| if let holdShortcut = ControlsTab.shortcuts[holdId]?.shortcut { | ||
| return holdShortcut.modifierFlags.cleaned() | ||
| } | ||
| if let fallback = Shortcut(keyEquivalent: Preferences.holdShortcut[shortcutIndex]) { | ||
| return fallback.modifierFlags.cleaned() | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new search input handling logic, including isAllowedSearchModifiers(), isSearchActivationKey(), isSearchableText(), and handleSearchInputIfNeeded(), lacks test coverage. Given that the project has comprehensive KeyboardEventsTests, consider adding tests to verify that search activation works correctly with different modifier combinations, that blocked characters are properly filtered, and that the 'S' key activation works as expected in focus-on-release mode.
|
Hi, Thank you for sharing this PR! I was surprised that you share a PR for search, when there is already one in-progress: #4962 Have you checked the other PR out? How does it compare? It's not to see such interest for adding the search feature. On the other hand, I think we should combine efforts 👍 Thank you |
|
Sorry, I didn't notice there was this PR before. The main difference between me and it is that I can perform a search by entering "s" without having to set it to "Do not perform task operation", right? I only gave the other PR a cursory look and haven't experienced it in detail. When your tabs exceed ten, it becomes difficult to find the one you want at a glance. At this point, searching by keywords can help you locate the desired window more quickly. |
…uence of event monitoring.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…t in the search box
|
Whether pressing S or not is a minor topic. I would prefer press any key and just start searching. But all configurable I implemented advanced fuzzy search and ranking in that PR. If you'd like to, I'm happy for you to take it over or improve on it |
|
Hi, |
Summary
Swhen "focus on release" is enabled.Before
After
Sactivates search mode without triggering focus-on-release, so typing is possible.Notes
Sto activate search if "focus on release" is enabled.Testing