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.

Sign up for GitHub

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

Jump to bottom

Always show the word when displaying a keyboard shortcut modifier #1255

Closed
bhousel opened this issue Dec 9, 2023 · 0 comments · Fixed by #1261
Closed

Always show the word when displaying a keyboard shortcut modifier #1255

bhousel opened this issue Dec 9, 2023 · 0 comments · Fixed by #1261
Assignees
@tannerwuster
Labels
feature-usability Issue or feature related to usability - something hard to do good first issue Good for newcomers
Milestone
v2.2.1

Comments

@bhousel
Copy link
Contributor

bhousel commented Dec 9, 2023 •
edited
Loading

This is a small usability nitpick but I noticed there are places where we display a keyboard shortcut like this:

Screenshot 2023-12-09 at 11 09 38 AM

And other places where we show just the symbol:

Screenshot 2023-12-09 at 11 09 50 AM

I think we should include the word in all cases, because nobody knows what symbols mean.

e.g.: ❌ Bad: ⇧ ⌘ ⌥
e.g.: ✅ Good: ⇧ Shift ⌘ Cmd ⌥ Option
@bhousel bhousel added feature-usability Issue or feature related to usability - something hard to do good first issue Good for newcomers labels Dec 9, 2023
@tannerwuster tannerwuster self-assigned this Dec 13, 2023
@tannerwuster tannerwuster mentioned this issue Dec 13, 2023
Merged
@bhousel bhousel added this to the 2.2.1 milestone Dec 22, 2023
bhousel added a commit that referenced this issue Jan 2, 2024
@bhousel
uiCmd shouldn't confuse up the shortcut with the display of the shortcut
18537cb 
(re: #1272, re: #1261, re: #1255)

Windows/linux should not replace '⌘Z' with 'Ctrl+Z', it should just replace it
with '^Z' and then the `uiCmd.display` should handle how to present that to the user.

This change simplifies the `uiCmd` code, and avoids the 'Ctrl+` stuff.

It also changes `uiTooltip` to work with an array of keys, using
d3-selection.each and `uiCmd.display` to render each one.
Previous code was attempting to split the keycombo into chars, causing #1272

I still need to make sure that `tooltip.keys()` is being called consistently,
with an Array of keys - we do this sometimes but not everywhere.

Also, in 01a16a9, the '+' to zoom in broke, because it is a shifted key.
This commit fixes that issue also.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tannerwuster tannerwuster

Labels
feature-usability Issue or feature related to usability - something hard to do good first issue Good for newcomers
Projects
None yet
Milestone
v2.2.1
Development

Successfully merging a pull request may close this issue.

Include keyboard key word alongside shortcut facebook/Rapid
2 participants
@bhousel @tannerwuster