feat: add keyboard shortcut to focus search #1028
Conversation
|
Deploy preview for docusaurus-preview ready! Built with commit 1fba7f0 |
|
Also - are there any other inputs fields? This naive implementation is not checking to see if some other input is already focused. |
|
Thanks for the PR @third774! '/' is usually for searching so I have no issues with using this key for focusing on the search input. I'm not sure whether other sites with more dynamic content might break if we add this (for e.g. Reason, Babel and Prettier have a playground). I'll test it out with those sites first and report back. |
| dangerouslySetInnerHTML={{ | ||
| __html: ` | ||
| document.addEventListener('keyup', function(e) { | ||
| // keyCode for '/' (slash) |
There was a problem hiding this comment.
Add
if (e.target !== document.body) {
return;
}
here so that we only focus on the search bar when no other items on the page are in focus.
Try your current script vs the script with the e.target checking on https://reasonml.github.io/en/try and you'll see why it's important to add that line.
There was a problem hiding this comment.
Perfect - I thought that might be an issue. Thanks!
v1/lib/core/Site.js
Outdated
| document.addEventListener('keyup', function(e) { | ||
| // keyCode for '/' (slash) | ||
| if (e.keyCode === 191) { | ||
| var search = document.getElementById('search_input_react'); |
There was a problem hiding this comment.
Use const search since it's not being reassigned.
There was a problem hiding this comment.
👍
Out of curiosity - do these inlined scripts somehow get run through babel? I was surprised to see that const is even supported in IE11 after you mentioned it, so it's probably fine if it doesn't?
There was a problem hiding this comment.
Nope these scripts do not get processed through Babel, so we have to be careful about not using new language features. const has actually existed since ES5 so we're fine 😄
|
@JoelMarcey @yangshun - requested changes have been pushed up! Thanks for the feedback! |
This PR adds a keyboard shortcut
/for focusing the search input.Motivation
Was browsing the
draft-jsdocs and found myself wishing this VIM keyboard shortcut existed since it is becoming more common across various websites. Not sure if where I've put this is the most appropriate place though.Have you read the Contributing Guidelines on pull requests?
Yep!
Test Plan
yarn startfromv1/Related PRs
I don't believe this PR is related to any others.