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

Add keyboard shortcut to focus search field #31702

Merged
merged 6 commits into from
Nov 5, 2020
Merged

Add keyboard shortcut to focus search field #31702

merged 6 commits into from
Nov 5, 2020

Conversation

mdo
Copy link
Member

@mdo mdo commented Sep 18, 2020

Picks up #29834 and adds a little tag to the search input to show you can use the / key to focus on it. I'm not sure what bugs @MartijnCuppens was seeing based on his comment there, but this seems to work great for me locally.

Screen Shot 2020-09-18 at 4 18 48 PM

Preview: https://deploy-preview-31702--twbs-bootstrap.netlify.app/docs/5.0/getting-started/introduction/

@patrickhlauke
Copy link
Member

I would advise against this, as it would fail https://www.w3.org/TR/WCAG21/#character-key-shortcuts

@mdo
Copy link
Member Author

mdo commented Sep 19, 2020

Oh boy, didn't know there were guidelines for that. Wonder if the built-in Algolia feature mentioned helps with that at all. Also wonder if GitHub does anything like this for their shortcuts.

@patrickhlauke
Copy link
Member

patrickhlauke commented Sep 19, 2020

Using an additional modifier key like ALT + / or CTRL + / would pass the WCAG SC and may be worth considering.

[edit: for github itself, I would suggest a general override/ability to turn keyboard shortcuts off would make sense - showing it as part of the user account settings/preferences, and perhaps firing up a modal the first time a keyboard shortcut is fired to advise the user - maybe show the hint modal that comes up when pressing ? and add a prominent "Turn keyboard shortcuts off" option there as well]

@MartijnCuppens
Copy link
Member

I'm not sure what bugs @MartijnCuppens was seeing based on his comment there

If an input or textarea is focused for example on the form controls page, the focus jumps to the search bar if / is typed.

We could make use of the native accesskey attribute, but I think it won't be clear enough which keyboard combo needs to be pressed.

@patrickhlauke
Copy link
Member

accesskey has historically seen very little use because of issues of discoverability, and because it varied between browsers which modifier key you'd use to trigger them (so sites shied away from providing explicit hints like ALT + / or similar, because that may not match the user's browser/platform).

and yes, if you just register an event listener on the page as a whole, it will kick in when it shouldn't (when user's focus is actually on an input, textarea, select, etc), so would generally need extra checks in the code about "if any of these types of element are currently the document.activeElement, just bail out and do nothing)"

@mdo mdo changed the title Use / key to focus search field Add keyboard shortcut to focus search field Oct 28, 2020
@mdo
Copy link
Member Author

mdo commented Oct 28, 2020

Pushed an update here to modify the shortcut from / to Ctrl /. I couldn't figure out how to do multiple keys in Algolia's autocomplete options, so I've stuck with the approach I started with. Thoughts?

@patrickhlauke
Copy link
Member

I couldn't figure out how to do multiple keys in Algolia's autocomplete options

if you mean the actual "once the search is focused, pressing normal keys triggers autocomplete suggestions", then that's kosher per the WCAG criterion, as you're not triggering actual functionality. in short, this now looks good :shipit:

@XhmikosR
Copy link
Member

XhmikosR commented Oct 28, 2020 via email

@XhmikosR
Copy link
Member

XhmikosR commented Nov 1, 2020

Not sure what's left here, it seems to work fine like @patrickhlauke said?

@mdo mdo merged commit 09a0938 into main Nov 5, 2020
@mdo mdo deleted the search-keyboard branch November 5, 2020 16:56
@einfallstoll
Copy link

Nice this even works with Ctrl + Shift + 7 (if / is created by typing Shift + 7).

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

Successfully merging this pull request may close these issues.

6 participants