Skip to content
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

feat: Add search bar #2278

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

feat: Add search bar #2278

wants to merge 4 commits into from

Conversation

ayroblu
Copy link
Contributor

@ayroblu ayroblu commented Jan 28, 2023

Initially I wanted to integrate alt-tab with Alfred as per #768, this covered a bunch of different things I tried, including extracting the images, and trying to figure out cross process communication in some way, such as with CFMessagePort etc.
But I realised that the primary benefit was for searching and a lot of the ordering and filtering tools options are already available in alt tab under the commands, and you can add lots of commands.

I realised that ultimately the best way was to integrate directly inside alt-tab, where everything is already in memory and low latency. I also saw that there's some buy in to do this with #590 so making this as a draft PR. This is "enough" for me, but there are some issues such as the changing window size, commented out instructions, manually changing the params to fit my needs, but maybe can get some direction or get some help with this.

It looks like this:
image

I set up my shortcut accordingly:
image
Note release does nothing, the removal of shortcut commands and making enter select the highlighted window

@ayroblu ayroblu marked this pull request as draft January 28, 2023 19:39
Copy link
Contributor

@decodism decodism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a partial review.

Also, you could check that thumbnailsPanel.thumbnailsView.searchField.stringValue is empty before hideUi() in refreshOpenUi().

@@ -220,6 +220,7 @@ class Windows {
}

static func refreshIfWindowShouldBeShownToTheUser(_ window: Window, _ screen: NSScreen) {
let searchText = App.app.thumbnailsPanel.thumbnailsView.searchField.stringValue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be lowercased().

func updateItemsAndLayout(_ screen: NSScreen) {
let widthMax = ThumbnailsPanel.widthMax(screen).rounded()
if let (maxX, maxY) = layoutThumbnailViews(screen, widthMax) {
if let (maxXThumbnails, maxYThumbnails) = layoutThumbnailViews(screen, widthMax) {
if !searchField.isTextChangeCallback {
Copy link
Contributor

@decodism decodism Jan 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to check that searchField.stringValue is empty.

lastOpenMaxY = maxYThumbnails
}
let maxX = searchField.isTextChangeCallback ? lastOpenMaxX : maxXThumbnails
let maxY = searchField.isTextChangeCallback ? lastOpenMaxY : maxYThumbnails
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are not needed. You can just reuse lastOpenMaxX and lastOpenMaxY.

searchField.frame.size = NSSize(width: min(maxX, widthMax), height: 40)
searchField.frame.origin = CGPoint(x: Preferences.windowPadding, y: min(maxY + 40, heightMax) - Preferences.windowPadding)

scrollView.frame.size = NSSize(width: min(maxX, widthMax), height: min(maxY, heightMax - 40))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use a constant for the height of the search field.

isBezeled = false
font = NSFont.systemFont(ofSize: 30)
focusRingType = .none

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could set textColor to Preferences.fontColor.


borderColor.set()
NSBezierPath.fill(borderRect)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could set currentEditor().insertionPointColor to Preferences.fontColor like this:

override func becomeFirstResponder() -> Bool {
    let success = super.becomeFirstResponder()
    if success {
        (currentEditor() as? NSTextView)?.insertionPointColor = Preferences.fontColor
    }
    return success
}

App.app.refreshOpenUi()
isTextChangeCallback = false
}
var isTextChangeCallback = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed.

@hexadecagram
Copy link

The need to select "Do nothing" under as the "then release" action could be obviated by making "Search" a configurable keypress to be used while open, with the default of course as /, which is a common default.

@ayroblu
Copy link
Contributor Author

ayroblu commented Feb 20, 2023

The need to select "Do nothing" under as the "then release" action could be obviated by making "Search" a configurable keypress to be used while open, with the default of course as /, which is a common default.

Yeah I wasn't sure, but I guess that making a special keypress that is for "search" would make sense. Cause there's no need for the search bar when pressing and holding. However some of the other shortcuts also don't make a lot of sense. It would be nice to have shortcuts per trigger type rather than having them be global

@wiwo192
Copy link

wiwo192 commented May 6, 2023

Thanks, @ayroblu and @decodism and for working on adding the search feature. The feature would be super useful.

Want to check to see if we have any good news on this. Haven't seen any update since Feb.

@ayroblu
Copy link
Contributor Author

ayroblu commented May 6, 2023

We need to settle on how the UI/UX should look. In the meantime I use this daily as is and it's "fine"

@wiwo192
Copy link

wiwo192 commented May 7, 2023

We need to settle on how the UI/UX should look. In the meantime I use this daily as is and it's "fine"

Is there a possibility to share the .app? Would love to use and try it. :).

Thanks again, on making it happen. :)

@ayroblu
Copy link
Contributor Author

ayroblu commented May 7, 2023

alttab-search.zip
Yeah sure, here you go, note that it's not up to date with the latest changes, and so provided as is

@vurtomatic
Copy link

Excited for this.

@mortang2410
Copy link

Thanks. I have been using the version with search for a day now.
There is a slight usability problem in that when you are typing for fuzzy search, it can also trigger the other hotkeys for hiding / minimizing etc. My current workaround is disabling those other hotkeys.

@ayroblu
Copy link
Contributor Author

ayroblu commented May 10, 2023

@mortang2410 please see the original post I have a screenshot of how I set it up. This is a critical part of setup.
As a reminder, we haven't solved the UI/UX issues yet so the point is that it's not supposed to be working well yet, so I'd like to focus discussion on how to transition the current layout to a more UX friendly layout.

@mortang2410
Copy link

Has there been any news on this? This would be a great addition overall.

@yakirlog
Copy link

Hi! any news about search bar addition? waiting for it long time 🤗
Thank you all for you hard work 🙏🏻

@ayroblu
Copy link
Contributor Author

ayroblu commented Apr 12, 2024

No news, but I use this everyday. It's up to the maintainer, theoretically we could land the half baked version as is behind a switch, but it requires a lot of setup (turning off hotkeys etc) that might not be intuitive to the wider group. I already published usable versions above if you want to try it

@lwouis lwouis mentioned this pull request Apr 25, 2024
@damnever
Copy link

I think we could leverage levenshtein distance to implement fuzzy search.

@akira-toriyama
Copy link

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants