Skip to content

Add autofocus for the searchbar #41

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

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

Add autofocus for the searchbar #41

wants to merge 3 commits into from

Conversation

buschtoens
Copy link
Contributor

As in #1 by @ianstormtaylor.

I'm kinda unsatisfied with this.searchbar = this.el.querySelector("input");.
Is there a way for reactive to access nested input elements and .focus() them?

I'm kinda unsatisfied with `this.searchbar =
this.el.querySelector("input");`.

Is there a way for `reactive` to access nested input elements and
`.focus()` them?

events.bind(el, 'keydown', function(e){
if(!~['input', 'select', 'textarea'].indexOf(e.target.tagName.toLowerCase())
&& !(e.target.contentEditable && e.target.contentEditable == 'true'))
Copy link
Contributor

Choose a reason for hiding this comment

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

what's this stuff doing? really hard to parse

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First line checks if the target element is an <input>, <select> or <textarea>, in which case we should not focus the search bar since one would be unable to type in those other fields.

Second line tests if the target element is editable. Pretty unlikely imho, but who knows. :D

(Inspired by mousetrap)

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah was just saying we can make easier for the next guy :p

var tag      = e.target.tagName
  , tags     = /(input|select|textarea)/i
  , editable = e.target.contentEditable == 'true';

if (editable || tags.test(tag)) return;

or if the editable is something we can throw out for now, we could do it in one line if we wanted

if (/(input|select|textarea)/i.test(e.target.tagName)) return;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Convinced. Editable falls out for now.

@tj
Copy link
Member

tj commented Mar 5, 2013

going to leave this out for now since the caret makes it look kinda funny, it would be cool even just to have an s shortcut key for "power users" haha, though I think most people will use the cli anyway

@ForbesLindesay
Copy link
Contributor

I really like the idea of autofocus. I think you're vastly underestimating the number of people who will be using the site. As the ultimate power user (the creator of the product) I'm not surprised you use the command line tool almost exclusively. Most people will probably use the command line tool purely for installation/build and use the website for search. A significant number of people won't begin using component until someone releases a GUI client that doesn't force the user to install node.js (manually). The component.jit.su site which was not hugely well publicised and was always a temporary measure gets about 150 page views a day (and that hasn't declined much after the release of component.io). As a proportion of the total number of users for component I'd say that's quite a few.

One issue I have with the search as it currently stands is that it's in a very unusual location, and as such is not easy to find when you first navigate to the site. This is especially true if, like me, you open it on a very tall screen. Perhaps it could be moved to the top right?

@ianstormtaylor
Copy link
Contributor

+1 for autofocus, actually i just think the idea of already being search mode when you get there is kinda badass in general :p

i'd also use the site over the command line, mostly because it's easier to get more information after deciding on a component there. or maybe i just dont like reading text in my terminal.. should swap to white text on dark instead and give the cli another chance

@tj
Copy link
Member

tj commented Mar 5, 2013

the new design has a fixed header so we could move the search to a more traditional position if we need to. expressjs.com gets about 10k/day. I'm not against the auto focus I just think it's kinda weird, especially since soon the search wont be the primary thing on the page (I will keep it on the front page though), we need way more real content

@ianstormtaylor
Copy link
Contributor

if search becomes not the main content then it makes sense not to have it. could also throw a tabindex="1" on there and that should make it the first focusable thing via tab, if i remember correctly (last i tried i had to tab past the logo first)

@stephenmathieson
Copy link
Contributor

please no autofocus. this was recently removed from npmjs.org for good reason.

@xixixao
Copy link

xixixao commented Dec 29, 2013

@stephenmathieson All those cases can be solved, without removing autofocus.

s was implemented but Esc to leave the search box is missing.

@stephenmathieson
Copy link
Contributor

@xixixao yep, i added the access key s in #58. why esc to leave to search box?

@xixixao
Copy link

xixixao commented Dec 29, 2013

If I can throw myself into a search box with keyboard, I also expect the option of leaving it with keyboard. The same way search box works in FF, Chrome, Windows, OS X (virtually anywhere I can think of).

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.

6 participants