Skip to content

Improve ergonomics of search #2698

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

Merged
merged 4 commits into from
May 14, 2025
Merged

Conversation

traviscross
Copy link
Contributor

@traviscross traviscross commented May 11, 2025

This PR improves the ergonomics of search by doing the following, and is best reviewed commit by commit:

  • Allows for search by / as well as s.
  • Fixes an uncaught exception when pressing down when there are no search results.
  • Navigates automatically to the first search result when pressing enter.
  • Uses <kbd> to render shortcut keys in the guide.

The commit messages contain more details and motivation.

Closes #2379

@rustbot rustbot added the S-waiting-on-review Status: waiting on a review label May 11, 2025
@traviscross traviscross force-pushed the TC/search-ergonomics branch 4 times, most recently from a395669 to 2703081 Compare May 13, 2025 01:05
We allow for using `s` to open the search box, but it's more common to
use `/` (forward slash) for this.  E.g., MDN's documentation uses `/`
for search.  Rustdoc and GitHub accept either.

Let's allow either key to be used, and let's switch to "advertising"
`/` rather than `s` in the hover text for the search button.

In making that switch, let's also simplify that hover text a bit.
Previously it had said "Search. (Shortkey: s)".  This was the only top
button on which we had included a period in the hover text.  Let's
remove that, and let's remove the "shortkey" bit of jargon.  It's
enough to just put `/` in a parenthetical, i.e. "Search (`/`)".
People will gleam from that what we mean.

We've also updated the guide accordingly.
If, when searching, one pressed the down arrow key when there were no
results, this caused an uncaught exception and defocused the search
box.

Let's prevent this and keep the search box focused when pressing down
in this state by checking first whether there is a result for us to
focus instead.
It's common for search boxes like ours to automatically navigate to
the first search result when the `enter` / `select` key is pressed, as
that can allow for rapid navigation.  E.g., the MDN documentation does
this.

Let's similarly navigate to the first result when, in the search box,
the user presses the `enter` key and there is a first result to which
to navigate.
@traviscross traviscross force-pushed the TC/search-ergonomics branch from 2703081 to 0561db6 Compare May 13, 2025 21:58
When describing, in the guide, the keyboard shortcuts that we accept,
let's use the `<kbd>` element.  This causes the key to render in a box
that people will recognize as conventional.

The way that this is displayed helps to make it clear that, though we
present the key in uppercase, we actually mean for the lowercase
letter to be entered.  Therefore, we present the key in uppercase
since 1) that's how it appears on most keyboards and 2) for some
characters such as `l`, presenting the character in lowercase might be
ambiguous.

We'll spell out "Escape" rather than saying "Esc" (even though many
keyboards spell it that way) since the `KeyboardEvent.keycode`[^1] is
called "Escape", and that's how it would appear in an
`aria-keyshortcuts` attribute[^2].

[^1]: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode

[^2]: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Attributes/aria-keyshortcuts
@traviscross traviscross force-pushed the TC/search-ergonomics branch from 0561db6 to 52e406b Compare May 13, 2025 22:08
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks!

@ehuss ehuss added this pull request to the merge queue May 14, 2025
Merged via the queue into rust-lang:master with commit 63ae0d5 May 14, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: waiting on a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support '/' shortcut to search
3 participants