Skip to content

Add pop-up window showing the keyboard shortcuts #2608

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 5 commits into from
May 15, 2025

Conversation

szabgab
Copy link
Contributor

@szabgab szabgab commented Mar 21, 2025

Make it display when the user presses ?.

Implements #2607

@rustbot rustbot added the S-waiting-on-review Status: waiting on a review label Mar 21, 2025
@ehuss
Copy link
Contributor

ehuss commented Mar 31, 2025

Thanks! Would it be possible to update the styling for some of the dark themes? Right now it is unreadable there:

image

Also, can you resolve the conflicts?

@shenef
Copy link

shenef commented Apr 1, 2025

How would a regular user know about the ? shortcut without seeing the list of shortcuts? (Edit: checking a bunch of websites, github was the only one that reacts to pressing ?)

Looking at the code, it seems like the shortcut is only active when shift is being held. On custom keyboard layouts the user might not have to press shift to write ? which in turn would make the shortcut inaccessible. (specifying "custom" here cause I, at a glance, wasn't able to find a language/region specific layout where that is the case)

@szabgab
Copy link
Contributor Author

szabgab commented Apr 1, 2025

I thought if this is accepted then I'd suggest to also add an icon to the menu to show a question mark.

You are right about the shift. I'll have to check how to catch ? in general also on keyboards where it might not require pressing down the Shift key, though maybe we can postpone that to when someone has such a case. (I wonder what happens when CapsLock is pressed and then some presses that key.)

szabgab and others added 5 commits May 14, 2025 17:06
Make it display when the user presses `?`.

Implements rust-lang#2607
Also handle the ? key with or without shift being pressed.
According to
https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode
the keyCode is deprecated (and removed). It is causing me some
difficulty with the GUI tests because it can't differentiate between a
slash and question mark correctly. Using keys seems to work.
This makes a few changes to the help popup:

- Move css to chrome.css, since this is a UI element.
- Move HTML code to index.hbs instead of generated in JavaScript.
  In general I prefer to keep HTML out of JavaScript when possible,
  and I didn't see a particular reason to avoid it.
- Added a click handler to dismiss the popup.
- Make sure handlers get removed when dismissed.
- Use `mdbook-` prefixes for IDs to avoid collisions with headers.
- Don't show search if it isn't enabled.
- Add the new `/` shortcut.
- Use flex layout for better positioning.
- Dim out the surrounding text using an overlay.
- Various other styling tweaks.
- Add a GUI test.
@ehuss ehuss force-pushed the navigation-help branch from 7a858b7 to 33da0c2 Compare May 15, 2025 01:17
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! I pushed some updates to adjust some of the styling and layout.

@ehuss ehuss enabled auto-merge May 15, 2025 01:23
@ehuss ehuss added this pull request to the merge queue May 15, 2025
Merged via the queue into rust-lang:master with commit 4813468 May 15, 2025
14 checks passed
@ehuss ehuss mentioned this pull request May 15, 2025
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.

4 participants