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

rustdoc: Ctrl-Enter on search result should open link in new tab #84384

Closed
jsha opened this issue Apr 21, 2021 · 4 comments · Fixed by #84462
Closed

rustdoc: Ctrl-Enter on search result should open link in new tab #84384

jsha opened this issue Apr 21, 2021 · 4 comments · Fixed by #84462
Assignees
Labels
A-rustdoc-js Area: Rustdoc's JS front-end C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@jsha
Copy link
Contributor

jsha commented Apr 21, 2021

Steps to reproduce:

  1. Visit a rustdoc page, e.g. https://doc.rust-lang.org/nightly/std/
  2. Hit 'S' to focus search.
  3. Type a search, e.g. "String"
  4. Press the down arrow once to highlight a search result.
  5. Hold down Ctrl (incidentally, notice that the highlighting goes away).
  6. Hit enter.

Expected result:

First search result is opened in a new tab.

Actual result:

Nothing happens.

In general, Ctrl-Enter works when an <a> is focused. The search results are <a>s. I think mainly we need two things:

  1. Make modifier keys (shift, ctrl, etc) not remove the highlighting of search results.
  2. Make sure highlighting search results with the arrow keys also focuses them.

Reproduces in latest Chrome and Firefox on Linux.

@jsha jsha added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-bug Category: This is a bug. A-rustdoc-js Area: Rustdoc's JS front-end labels Apr 21, 2021
@GuillaumeGomez GuillaumeGomez self-assigned this Apr 22, 2021
@GuillaumeGomez
Copy link
Member

Make sure highlighting search results with the arrow keys also focuses them.

This is not possible because it'd make us lose focus on the search input, preventing anything else to work. A workaround would be to add the focus on the search only if the ctrl key is pressed, but then what happens if you want to switch result tab instead? I don't think focus will work nicely here...

@GuillaumeGomez
Copy link
Member

As for opening in new tab with ctrl+tab, it'd require a big rework of how we handle the events on the search because when we do it only from JS, on FF it asks confirmation to go to the new tab as "big security concern". If we want to focus on the elements with they keyboard arrows, we won't be able to bind the event for them because people want to scroll using them.

@jsha
Copy link
Contributor Author

jsha commented Apr 22, 2021

If we want to focus on the elements with they keyboard arrows, we won't be able to bind the event for them because people want to scroll using them.

What I'm proposing is:

  • When search is focused, down arrow focuses the first search result
  • When a search result is focused, up/down focus the next/previous search result, or the search box if you press up from the first result.
  • When neither is focused, let the default action occur.

That way up/down would continue to work correctly when scrolling docs (I agree it would be very bad to break that behavior).

@GuillaumeGomez
Copy link
Member

But then we couldn't switch between tabs anymore unless handling it from two places. Well, considering all the great work you did until now, I'm pretty sure you have already a plan in mind so I'll just wait for you to amaze me as usual. :p

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Apr 23, 2021
…=jsha

Prevent control, shift and alt keys to make search input lose focus

Part of rust-lang#84384.

r? `@jsha`
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Apr 23, 2021
…=jsha

Prevent control, shift and alt keys to make search input lose focus

Part of rust-lang#84384.

r? ``@jsha``
JohnTitor added a commit to JohnTitor/rust that referenced this issue Apr 23, 2021
…=jsha

Prevent control, shift and alt keys to make search input lose focus

Part of rust-lang#84384.

r? ```@jsha```
JohnTitor added a commit to JohnTitor/rust that referenced this issue Apr 23, 2021
…=jsha

Prevent control, shift and alt keys to make search input lose focus

Part of rust-lang#84384.

r? ````@jsha````
@bors bors closed this as completed in c9b6bb9 May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-js Area: Rustdoc's JS front-end C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants