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

Include keyboard key word alongside shortcut #1261

Merged
merged 4 commits into from
Dec 22, 2023
Merged

Conversation

tannerwuster
Copy link
Contributor

@tannerwuster tannerwuster commented Dec 13, 2023

Description

Include the key word in all cases, because nobody knows what symbols mean. Closes #1255
Screenshot 2023-12-18 at 16 52 30
Screenshot 2023-12-18 at 16 52 23

@tannerwuster tannerwuster force-pushed the keyboard_shortcut_details branch from f5c973d to f439304 Compare December 18, 2023 23:51
@tannerwuster tannerwuster marked this pull request as ready for review December 18, 2023 23:53
Copy link
Contributor

@bhousel bhousel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! I think the shortcuts are much more understandable now 👍

@bhousel bhousel merged commit f0c4c4f into main Dec 22, 2023
4 checks passed
@bhousel bhousel deleted the keyboard_shortcut_details branch December 22, 2023 21:54
@bhousel bhousel added this to the 2.2.1 milestone Dec 22, 2023
bhousel added a commit that referenced this pull request Jan 2, 2024
(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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Always show the word when displaying a keyboard shortcut modifier
3 participants