Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

docs (index) there should be no autofocus when docs are read - #2737

Closed
eddiemonge wants to merge 1 commit into
angular:masterfrom
eddiemonge:nofocus
Closed

eddiemonge wants to merge 1 commit into
angular:masterfrom
eddiemonge:nofocus

Conversation

@eddiemonge

Copy link
Copy Markdown
Contributor

The search box should not receive autofocus when browsing the docs as the primary purpose of the docs it to convey information, not search.

The search box should not receive autofocus when browsing the docs as the primary purpose of the docs it to convey information, not search.
@petebacondarwin

Copy link
Copy Markdown
Contributor

@eddiemonge - not sure I agree here. Certainly in the API pages, I always want to search for an item. Even in the guide, I usually don't know exactly where I want to browse to and type something in to get me there. Having focus on the search box speeds this up dramatically. What is the downside of this input having focus?

@eddiemonge

Copy link
Copy Markdown
Contributor Author

you lose keyboard navigation on the page. Space and arrow keys dont work to navigate without pressing esc or clicking somewhere to remove the input boxes focus. On the index page its not so much a problem but lets say I land on http://docs.angularjs.org/api/ng.directive:ngChecked. Clearly I want to read whats on that page and the focus shouldnt be on searching for something else first. The focus should be on that pages content.

@eddiemonge eddiemonge closed this May 22, 2013
@eddiemonge eddiemonge reopened this May 22, 2013
@eddiemonge

Copy link
Copy Markdown
Contributor Author

didnt mean to close. github has weird tab navigation

@eddiemonge

Copy link
Copy Markdown
Contributor Author

Another place is http://docs.angularjs.org/tutorial/
Search shouldn't have focus there

@eddiemonge

Copy link
Copy Markdown
Contributor Author

any more decisions on this?

@pkozlowski-opensource

Copy link
Copy Markdown
Member

I tend to agree with @eddiemonge
I'm not sure I like the fact that the focus stays in the search box as it blocks keyboard navigation (up/down arrows) within a page. It is not a huge deal, though, as one can always:

  • press tab to get the focus out of the search field
  • press alt+s to get the focus back to the search box

I guess it is my timid +1 for merging this (unless I'm missing some other navigation patterns that initial focus makes easy / possible)

@IgorMinar

Copy link
Copy Markdown
Contributor

I'm ok with removing the focus and using tabindex="1" on this input field instead as we already to. however this PR is wrong. we should not break the focused directive. we should remove it or change it and rename it.

@IgorMinar

Copy link
Copy Markdown
Contributor

@matsko can you please look into dropping autofocus from the search bar in the docs app? thnx!

@IgorMinar

Copy link
Copy Markdown
Contributor

I would love for us to kill the focused directive. I'm looking at the implementation and it's super-lame. we however need to communicate the current focus state to ngClass somehow.

@pkozlowski-opensource

Copy link
Copy Markdown
Member

@IgorMinar there is a PR for read-only blur / focus: #2979 (it still needs work as it doesn't have tests / doc updates).

@matsko

matsko commented Jul 15, 2013

Copy link
Copy Markdown
Contributor

I'll make two commits. One to remove the autofocus and another to play with the ng-focus/ng-blur + ng-class directives on the input.

@matsko

matsko commented Jul 15, 2013

Copy link
Copy Markdown
Contributor

I also think the accesskey feature isn't helping here. Certain browsers/OSs have different shortcut combinations and I highly doubt most users will use them. We can keep it there to retain any compatibility, but perhaps we could add a global s key to search and / key to perform the fulltext search? https://en.wikipedia.org/wiki/Access_key

@matsko

matsko commented Jul 15, 2013

Copy link
Copy Markdown
Contributor

Here's commit 1: #3227

@eddiemonge eddiemonge closed this Jul 15, 2013
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.

5 participants