Support type '/' to search #123355

mu001999 · 2024-04-02T06:39:26Z

Related topic on IRLO: https://internals.rust-lang.org/t/rustdoc-use-key-to-search-instead-of-s/20559

rustbot · 2024-04-02T06:39:33Z

rustbot has assigned @fmease.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-04-02T06:39:35Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

GuillaumeGomez · 2024-04-02T08:38:10Z

I remember we had a discussion about changing the key to focus on the search input but can't find it again... I don't really have an opinion on this matter so let's see if everyone else agrees with it.

@rfcbot fcp merge

rfcbot · 2024-04-02T08:38:13Z

Team member @GuillaumeGomez has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

Manishearth · 2024-04-02T14:37:45Z

I'd have preferred / from the get-go, even, so I'm in favor of this.

notriddle · 2024-04-02T15:14:40Z

src/librustdoc/html/templates/page.html

@@ -135,7 135,7 @@ <h2>Files</h2> {# #}
 aria-label="Run search in the documentation" {# #}
 autocomplete="off" {# #}
 spellcheck="false" {# #}
- placeholder="Click or press ‘S’ to search, ‘?’ for more options…" {# #}
+ placeholder="Type ‘S’ or ‘/’ to search, ‘?’ for more options…" {# #}


Can we change this to just "Search"?

Suggested change

placeholder="Type ‘S’ or ‘/’ to search, ‘?’ for more options…" {# #}

placeholder="Search" {# #}

I know it's a bit off-topic, and I'll approve this PR even if the change isn't made, but the keyboard command should probably be made less prominent.

The whole premise of the PR is that we don't actually need to tell people about this keyboard command. They will assume it exists in Rustdoc because it exists in GitHub, Discourse, MDN, Zulip, GitLab, other websites that I don't know about, and VI.

If you don't have a hardware keyboard, the UI is presenting things to you that you can't use.

The ? command only works if you're on a big enough screen. If you're on a small viewport, the ? keyboard command does nothing even if you have a hardware keyboard. The button is also gone.

"Search" ought to be the first word, for ease of scanning.

I was thinking about adding the available keys on the right of the search input a bit like what the github search input has. Then depending on the screen width, we can easily hide the ones not available.

The search bar has had explicit words about the keyboard commands since 1.0. I'm pretty sure the reason they did it that way was because the ? button didn't exist yet.

Now that there's a help button, do we really need the stuff in the search box? We don't have on-screen indicators for most keyboard shortcuts; what makes search different other than the fact that we've always done it that way?

Ranked preferences

No indicator for keyboard shortcuts unless you open the Help window is ideal IMO. Keyboard shortcuts aren't important enough to justify redundancy. We should spend our above-the-fold-clutter budget on the actual crate's documentation.

If you really want to have an on-screen indicator for this keyboard shortcut, the way it's done now is ideal. It's very straightforward, so at least we're spending screen real estate on something the user will actually understand and benefit from.

Are you suggesting something like this?

I don't like that approach at all. It seems like a bad compromise that's worse than either of the "extremes," because it's adding stuff on the screen but it's so subtle that new readers probably won't even understand what it means. Few sites display it that way according to this guy

Convenience and also people (sadly) seem to rarely ever click on the ? button.

I think maybe just having "/ to search" as a short snippet would be fine. We shouldn't inundiate them with information here, but having "/ to search" on a search box is quite standard and not distracting (github does it).

Having more complex text is distracting

I agree with just having '/' to search or Type '/' to search, like what github does

rfcbot · 2024-04-03T07:32:31Z

🔔 This is now entering its final comment period, as per the review above. 🔔

wezm · 2024-04-09T23:46:00Z

Perhaps one thing worth noting is that Firefox has a default binding for / that this will potentially override. I don't personally use it myself but I do know it does bother some folks when websites take over browser shortcuts.

https://support.mozilla.org/en-US/kb/search-contents-current-page-text-or-links?redirectslug=Searching within a page&redirectlocale=en-US#w_quick-find

jplatte · 2024-04-10T00:36:33Z

Actually, the FF thing overrides GitHub's slash-to-search for me, not the other way around (and I would much rather have slash start GitHub's search).

wezm · 2024-04-10T00:48:16Z

Actually, the FF thing overrides GitHub's slash-to-search for me, not the other way around (and I would much rather have slash start GitHub's search).

That's weird. For me in Firefox (on Linux) / jumps to GitHub's search...

jplatte · 2024-04-10T00:49:55Z

🤷🏼 I'm also on [arch] linux, sway, FF dev edition 125.0b9 (though this has been the case for a few releases I'm pretty sure)

wezm · 2024-04-10T01:00:52Z

🤷🏼 I'm also on [arch] linux, sway, FF dev edition 125.0b9 (though this has been the case for a few releases I'm pretty sure)

Arch for me as well, although X11 (Awesome) not Wayland. FF 124.0.1

jplatte · 2024-04-10T12:33:56Z

I think we're going really off-topic here and honestly I don't care that much. You know where to message me in case you want to chat more about this; this GitHub PR doesn't seem like the right place.

rfcbot · 2024-04-13T07:36:22Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

GuillaumeGomez · 2024-04-13T09:55:12Z

With all the new comments since we debated about adding this feature. Should we move forward regardless?

Manishearth · 2024-04-13T16:15:33Z

They don't change my position here imo.

GuillaumeGomez · 2024-04-16T13:43:34Z

Well at least it comforts me in the idea that we should support both s and / to focus on the search input.

Well, as usual, we can revert if it's too much of an issue in the issue. For now I think it's fine to go through with this. Thanks everyone!

@bors r rollup

bors · 2024-04-16T13:43:37Z

📌 Commit 1ca1457 has been approved by GuillaumeGomez

It is now in the queue for this repository.

…meGomez Support type '/' to search Related topic on IRLO: https://internals.rust-lang.org/t/rustdoc-use-key-to-search-instead-of-s/20559

…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#122632 (fetch submodule before checking llvm stamp) - rust-lang#123355 (Support type '/' to search) - rust-lang#123501 (Stabilize checking of cfgs at compile-time: `--check-cfg` option) - rust-lang#123535 (Match ergonomics 2024: `mut` doesn't reset binding mode) - rust-lang#123711 (drop `changelog-seen`) - rust-lang#123969 (The new solver ignores `DefineOpaqueTypes`, so switch it to `Yes`) - rust-lang#124007 (Miri subtree update) - rust-lang#124017 (Change a diagnostics-path-only `DefineOpaqueTypes` to `Yes`.) - rust-lang#124018 (interpret: pass MemoryKind to before_memory_deallocation) - rust-lang#124024 (interpret: remove outdated comment) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#123355 - mu001999:rustdoc/search, r=GuillaumeGomez Support type '/' to search Related topic on IRLO: https://internals.rust-lang.org/t/rustdoc-use-key-to-search-instead-of-s/20559

Support type '/' to search

1ca1457

rustbot assigned fmease Apr 2, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Apr 2, 2024

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Apr 2, 2024

notriddle reviewed Apr 2, 2024

View reviewed changes

rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Apr 3, 2024

fmease added S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 3, 2024

syphar mentioned this pull request Apr 3, 2024

Support slash / keyboard shortcut to focus search rust-lang/docs.rs#1998

Open

This comment was marked as off-topic.

Sign in to view

fmease assigned GuillaumeGomez and unassigned fmease Apr 16, 2024

fmease added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. labels Apr 16, 2024

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 16, 2024

This was referenced Apr 16, 2024

Rollup of 11 pull requests #124025

Closed

Rollup of 10 pull requests #124026

Merged

bors merged commit 49f6c95 into rust-lang:master Apr 16, 2024
11 checks passed

rustbot added this to the 1.79.0 milestone Apr 16, 2024

apiraino removed the to-announce Announce this issue on triage meeting label Apr 29, 2024

This was referenced Apr 30, 2024

Support typing '/' to search rust-lang/docs.rs#2502

Merged

Support typing '/' to search rust-lang/crates.io#8566

Merged

aDotInTheVoid mentioned this pull request May 14, 2024

Support '/' shortcut to search rust-lang/mdBook#2379

Open

mu001999 deleted the rustdoc/search branch June 26, 2024 14:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support type '/' to search #123355

Support type '/' to search #123355

mu001999 commented Apr 2, 2024

rustbot commented Apr 2, 2024

rustbot commented Apr 2, 2024

GuillaumeGomez commented Apr 2, 2024

rfcbot commented Apr 2, 2024 •

edited by aDotInTheVoid

Loading

Manishearth commented Apr 2, 2024

notriddle Apr 2, 2024 •

edited

Loading

GuillaumeGomez Apr 2, 2024 •

edited

Loading

notriddle Apr 2, 2024 •

edited

Loading

GuillaumeGomez Apr 2, 2024

Manishearth Apr 2, 2024

mu001999 Apr 3, 2024

rfcbot commented Apr 3, 2024

wezm commented Apr 9, 2024

jplatte commented Apr 10, 2024

wezm commented Apr 10, 2024

jplatte commented Apr 10, 2024

wezm commented Apr 10, 2024

This comment was marked as off-topic.

jplatte commented Apr 10, 2024 •

edited

Loading

rfcbot commented Apr 13, 2024

GuillaumeGomez commented Apr 13, 2024

Manishearth commented Apr 13, 2024

GuillaumeGomez commented Apr 16, 2024

bors commented Apr 16, 2024

	placeholder="Type ‘S’ or ‘/’ to search, ‘?’ for more options…" {# #}
	placeholder="Search" {# #}

Support type '/' to search #123355

Support type '/' to search #123355

Conversation

mu001999 commented Apr 2, 2024

rustbot commented Apr 2, 2024

rustbot commented Apr 2, 2024

GuillaumeGomez commented Apr 2, 2024

rfcbot commented Apr 2, 2024 • edited by aDotInTheVoid Loading

Manishearth commented Apr 2, 2024

notriddle Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

GuillaumeGomez Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

notriddle Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

GuillaumeGomez Apr 2, 2024

Choose a reason for hiding this comment

Manishearth Apr 2, 2024

Choose a reason for hiding this comment

mu001999 Apr 3, 2024

Choose a reason for hiding this comment

rfcbot commented Apr 3, 2024

wezm commented Apr 9, 2024

jplatte commented Apr 10, 2024

wezm commented Apr 10, 2024

jplatte commented Apr 10, 2024

wezm commented Apr 10, 2024

This comment was marked as off-topic.

jplatte commented Apr 10, 2024 • edited Loading

rfcbot commented Apr 13, 2024

GuillaumeGomez commented Apr 13, 2024

Manishearth commented Apr 13, 2024

GuillaumeGomez commented Apr 16, 2024

bors commented Apr 16, 2024

rfcbot commented Apr 2, 2024 •

edited by aDotInTheVoid

Loading

notriddle Apr 2, 2024 •

edited

Loading

GuillaumeGomez Apr 2, 2024 •

edited

Loading

notriddle Apr 2, 2024 •

edited

Loading

jplatte commented Apr 10, 2024 •

edited

Loading