git / git-scm.com

The git-scm.com website. Note that this repository is only for the website; issues with git itself should go to https://git-scm.com/community.
https://git-scm.com/
MIT License
2.16k stars 1.21k forks source link

search: add shortcuts to focus #1856

Closed benknoble closed 3 weeks ago

benknoble commented 1 month ago

This cribs from Racket's documentation site 1, where pressing "s" will focus the search bar. This is a modified version of 2, which I choose to receive and use under the MIT license 3, 4, which is compatible with MIT-LICENSE.txt.

Add a note to the search bar's placeholder: there's a probably a better way to steer users into this, but "/" is common thanks to Vim and many websites; "s" is natural (at least to me, having used Racket's docs so long).

The jQuery features may be using deprecated handlers 5, but the site appears to be on 12-year old jQuery 6. Modifications to the jQuery code from Racket work better on Firefox and allow other document elements to intercept the event.


I haven't yet managed to setup a local version to test this; feedback from testers welcome.

benknoble commented 4 weeks ago

The range-diff is probably not very useful, but I've included it here anyway (with --creation-factor=90):

range-diff ``` 1: 96c138c6 ! 1: 9aa7f614 search: add shortcuts to focus @@ Commit message to receive and use under the MIT license [3], [4], which is compatible with MIT-LICENSE.txt. - There's a probably a good way to steer users into this, but "/" is - common thanks to Vim and many websites; "s" is natural (at least to me, - having used Racket's docs so long). + Add a note to the search bar's placeholder: there's a probably a better + way to steer users into this, but "/" is common thanks to Vim and many + websites; "s" is natural (at least to me, having used Racket's docs so + long). - There's probably a "jQuery" way to do this, but I didn't look very hard. + The jQuery features may be using deprecated handlers [5], but the site + appears to be on 12-year old jQuery [6]. Modifications to the jQuery + code from Racket work better on Firefox and allow other document + elements to intercept the event. [1]: https://docs.racket-lang.org [2]: https://github.com/racket/scribble/blob/02ecb223c8e0e4dda3c00739338c96dcd1d5bac6/scribble-lib/scribble/scribble-common.js#L145C2-L152C13 [3]: https://github.com/racket/scribble/blob/02ecb223c8e0e4dda3c00739338c96dcd1d5bac6/LICENSE.txt [4]: https://github.com/racket/racket/blob/240aa086c548440e0b11641c40cf79e393a560a4/racket/src/LICENSE-MIT.txt + [5]: https://api.jquery.com/keyup-shorthand/ + [6]: https://blog.jquery.com/2012/03/21/jquery-1-7-2-released/ + + Co-authored-by: Johannes Schindelin ## app/assets/javascripts/application.js ## @@ app/assets/javascripts/application.js: var Search = { Search.observeFocus(); Search.observeTextEntry(); Search.observeResultsClicks(); -+ Search.listenForKeys(); ++ Search.installKeyboardShortcuts(); }, observeFocus: function() { @@ app/assets/javascripts/application.js: var Search = { }); }, -+ listenForKeys: function() { -+ window.addEventListener("keyup", function(e) { -+ if ((e.key === 's' || e.key === 'S' || e.key === '/') && e.target === document.body) { ++ installKeyboardShortcuts: function() { ++ $(document).keydown(function(e) { ++ if (e.target.tagName.toUpperCase() !== 'INPUT' && ['s', 'S', '/'].includes(e.key)) { ++ e.preventDefault(); + $('form#search input').focus(); + } -+ }, false); ++ }); + }, + runSearch: function() { var term = $('#search-text').val(); if(term.length < 2) { return false }; + + ## app/views/shared/_header.html.erb ## +@@ + document.getElementById('tagline').innerHTML = '--' + tagline; + + +
+ ```
dscho commented 3 weeks ago

https://blog.jquery.com/2012/03/21/jquery-1-7-2-released/

Small correction: the site uses jQuery v1.7.1 (which is almost 13 years old).

[Press s] Search entire site...

I have a slight preference for "Type [/] to search entire site..." similar to what GitHub shows in its search box:

image

Maybe an even better way would be to mention the shortcut less prominently, after the original placeholder text something like "Search entire site... (press `/` to focus)"; I tried to figure out a way to use Unicode to render this as a key, but failed (it looks ugly: /⃣), and also to render the [/] on the right-hand side (but placeholder="Search entire site...&#x202E;[/]&#x202D;" seems not to extend to the entire search box' width).

FWIW the slash is the most common keyboard shortcut for search on websites that I know of, much more common than s (even if I have no problem keeping the functionality, I just don't want the label to say s instead of /); Not only does GitHub support it, but also YouTube, and likely others. It used to be supported in GMail, but now that I tried it, it seems that this shortcut has joined the Google Graveyard.

Other than those two small nits, I am totally in favor of this PR.

benknoble commented 3 weeks ago

https://blog.jquery.com/2012/03/21/jquery-1-7-2-released/

Small correction: the site uses jQuery v1.7.1 (which is almost 13 years old).

Fixed, thanks.

[Press s] Search entire site...

I have a slight preference for "Type [/] to search entire site..." similar to what GitHub shows in its search box:

GitHub cheats and uses <kbd> markup inside a button>div>span, which I think would be hard to do with the placeholder text of an input element.

Maybe an even better way would be to mention the shortcut less prominently, after the original placeholder text something like "Search entire site... (press `/` to focus)"; I tried to figure out a way to use Unicode to render this as a key, but failed (it looks ugly: /⃣), and also to render the [/] on the right-hand side (but placeholder="Search entire site...&#x202E;[/]&#x202D;" seems not to extend to the entire search box' width).

This is all reasonable: what do you think about an unadorned Type / to search the entire site…? I worried that [/] might make folks think they needed to type the keys [/]. I could also move to this to a parenthetical as suggested.

FWIW the slash is the most common keyboard shortcut for search on websites that I know of, much more common than s (even if I have no problem keeping the functionality, I just don't want the label to say s instead of /); Not only does GitHub support it, but also YouTube, and likely others. It used to be supported in GMail, but now that I tried it, it seems that this shortcut has joined the Google Graveyard.

At least for me, / still works in GMail: you have to enable keyboard shortcuts, though, IIRC.

Other than those two small nits, I am totally in favor of this PR.

❤️

range-diff ``` 1: 9aa7f614 ! 1: 4c72ad9a search: add shortcuts to focus @@ Commit message Add a note to the search bar's placeholder: there's a probably a better way to steer users into this, but "/" is common thanks to Vim and many websites; "s" is natural (at least to me, having used Racket's docs so - long). + long). While we're here, use a true ellipsis character. The jQuery features may be using deprecated handlers [5], but the site - appears to be on 12-year old jQuery [6]. Modifications to the jQuery + appears to be on 13-year old jQuery [6]. Modifications to the jQuery code from Racket work better on Firefox and allow other document elements to intercept the event. @@ Commit message [3]: https://github.com/racket/scribble/blob/02ecb223c8e0e4dda3c00739338c96dcd1d5bac6/LICENSE.txt [4]: https://github.com/racket/racket/blob/240aa086c548440e0b11641c40cf79e393a560a4/racket/src/LICENSE-MIT.txt [5]: https://api.jquery.com/keyup-shorthand/ - [6]: https://blog.jquery.com/2012/03/21/jquery-1-7-2-released/ + [6]: https://blog.jquery.com/2011/11/21/jquery-1-7-1-released/ Co-authored-by: Johannes Schindelin @@ app/views/shared/_header.html.erb
```
dscho commented 3 weeks ago

Thank you so much @benknoble!