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: Don't override system keyboard shortcuts for search results tab-switcher #79872

Closed
camelid opened this issue Dec 9, 2020 · 11 comments · Fixed by #84462
Closed

rustdoc: Don't override system keyboard shortcuts for search results tab-switcher #79872

camelid opened this issue Dec 9, 2020 · 11 comments · Fixed by #84462
Labels
A-rustdoc-search Area: Rustdoc's search feature A-rustdoc-ui Area: rustdoc UI (generated HTML) C-bug Category: This is a bug. O-macos Operating system: macOS T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@camelid
Copy link
Member

camelid commented Dec 9, 2020

Once #79862 is merged, rustdoc will be overriding the macOS system keyboard
shortcuts Ctrl-↑ and Ctrl-↓ (Control–Up Arrow and Control–Down Arrow).

Personally, I don't use these shortcuts, but I imagine others do. We should find
different key combinations that won't override system shortcuts.

See the discussion around #79862 (comment).

cc @GuillaumeGomez @Manishearth

@camelid camelid added A-rustdoc-search Area: Rustdoc's search feature A-rustdoc-ui Area: rustdoc UI (generated HTML) O-macos Operating system: macOS T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Dec 9, 2020
@Manishearth
Copy link
Member

Not a fan of overriding system shortcuts, but I do think this is less pressing than the tab issue. Wondering what suggestions people have.

@camelid camelid added C-discussion Category: Discussion or questions that doesn't represent real issues. and removed C-discussion Category: Discussion or questions that doesn't represent real issues. labels Dec 10, 2020
@GuillaumeGomez
Copy link
Member

Wouldn't replace ctrl with cmd on mac be enough? It's generally what's done for most shortcuts. I just need a confirmation before making the fix.

@Manishearth
Copy link
Member

I believe cmd-up/down is also a system shortcut? If it's not that would work.

@GuillaumeGomez
Copy link
Member

To be checked then! I'll try to do that this evening or tomorrow.

@camelid
Copy link
Member Author

camelid commented Dec 10, 2020

Command–Up Arrow and Command–Down Arrow are used for scrolling to the top and bottom of the page in many browsers, including Safari and Firefox (and I assume Google Chrome). I don't think we should override that if we're looking to be as accessible as possible.

Unfortunately, many of the keyboard shortcut options are already taken :/

@GuillaumeGomez
Copy link
Member

Arf. :-/

@jsha
Copy link
Contributor

jsha commented Apr 22, 2021

What about using keys without modifiers? E.g. t to cycle through the tabs, or <- and ->. This would mean you'd need to hit down or tab to take focus out of the search box, but I think that's an okay tradeoff.

@GuillaumeGomez
Copy link
Member

I'd personnally prefer that those events are only handled when we have the focus on the search input.

@Manishearth
Copy link
Member

Manishearth commented Apr 22, 2021

I think using letter keys for navigation when you're in a search view but outside the search box makes sense.

@GuillaumeGomez
Copy link
Member

That could work. I don't know how many of our users have their own letter keys bound to something, not sure if that would be a problem?

@Manishearth
Copy link
Member

It's very rare.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-search Area: Rustdoc's search feature A-rustdoc-ui Area: rustdoc UI (generated HTML) C-bug Category: This is a bug. O-macos Operating system: macOS 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.

4 participants