Skip to content
This repository has been archived by the owner on Nov 17, 2022. It is now read-only.

Add autofocus to sidebar input, and support navigating filtered results. #305

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

thomcc
Copy link
Contributor

@thomcc thomcc commented Feb 11, 2018

Most of the sidebars in the browser have focus in the search bar when they come up, so it seems like this one should too. It also helps alleviate #304, which doesn't seem doable currently (el.focus() when called from a command handler never seems to work. I can't tell if there's a bug on file for that yet, or if it's even a bug at all vs a design choice).

The second commit is much meatier but basically just adds the notion of a currently "selected" tab to the sidebar. This allows easier navigation of search results with the arrow keys. The behavior I've used was chosen by what "feels" good, and I've added comments to explain the non-obvious parts.

Some notes, most of which are also in comments:

  • On mac ctrl+n and ctrl+p are usable as arrow key substitutes. This works in most places on macs (for whatever reason it inherited them from emacs), and I use them constantly, so I've supported them. We ignore them on other platforms, of course.
  • Hitting enter in the searchbox opens the selected tab and clears the search, hitting shift+enter opens the selected tab and keeps the search.
  • Most manipulations of the tab list clear the selection. This is just because there's no reason to keep it around for long, not a technical limitation.
  • We won't use pinned tabs as the initially selected element, but you can still select them.
  • If you use arrow keys (or ctrl n/p) when the search panel has focus but is empty, we let you navigate through the tabs same as if you had a search panel.

Thom Chiovoloni added 2 commits February 10, 2018 19:34
This is the behavior of the builtin sidebars. It also partially addresses issue 304
@eoger
Copy link
Owner

eoger commented May 5, 2018

Apologies for the very long review delay.
Now that we have explicit tab positioning (41128b1), I came round and think that this is a good addition (and the patch looks great).
Since I'm super late to that review, I don't except you to make the effort to re-base/update it and I'll probably do it myself when I get some time.

@mitliao
Copy link

mitliao commented Oct 15, 2018

Such a good addition. Do you have plan to merge this?

@bjesus
Copy link

bjesus commented Mar 23, 2019

Is there a way to help this get merged?

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

Successfully merging this pull request may close these issues.

4 participants