Add new keyboard shortcuts help modal #8316

talldan · 2018-07-31T08:40:49Z

Description

Related issues - #71, #2980, #5420

Pressing ctl+option+h (shift+alt+h on windows/linux) displays a modal detailing keyboard shortcuts.

How has this been tested?

Manual testing (both Mac & Windows)
- Test opening/closing modal using keyboard (esc also closes, as does hitting enter while the close
  button is focused)
- Test opening/closing modal using mouse (open from more menu, close using button or clicking surrounding space)
- Test classic block (that TinyMCE shortcut help doesn't also open at the same time as gutenberg shortcut help)
Snapshot tests
Added an e2e test that opens and closes modal using mouse and keyboard

Screenshots

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.

talldan · 2018-07-31T08:45:13Z

Hey @jasmussen - before I take this one any further I thought I'd seek some design / copy feedback.

This has the same issue as the other PR with the formatting of shortcuts (e.g. ⌘+? instead of ⌘?), so I'll work on that now and cherry-pick the change onto both branches. (edit: updated screenshot with improvements)

Not sure if this is too wordy, or perhaps it should just be stripped back to something simpler. Any other design hints would be good. I know heading sizes are not quite right, should the modal have a bigger header?

jasmussen · 2018-07-31T10:13:29Z

Very very cool work, thank you for working on this.

I can't get the modal to show. meta+? isn't working for me. Also, can we make it Shift+? instead? This keyboard shortcut is standard in Google properties and others, might as well reuse existing patterns if we can.

Finally, can we add it as a menu item? Like this:

I think this will be very welcome.

Just looking at the screenshot, I think this is a great start. I like how Google Inbox shows their shortcuts:

Note the little gray background behind each shortcut button. We can probably use the kbd element for this: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/kbd — possibly with a little minimalist styling.

Notably I like the relaxed row-like layout of keyboard shortcuts in Chrome OS:

Flagging also @karmatosed for thoughts.

michelleweber · 2018-07-31T10:14:16Z

(On the copy front, I'm not sure that you need the first sentence/subhead in each section; they bulk the modal up, but don't add any info that's not already there in the title and instructions. It also seems like the slash inserter could be part of the block shortcuts section, and I wonder if selection shortcuts can be part of global; then you have a simpler structure: general, block-specific, text-specific.)

karmatosed · 2018-07-31T20:53:29Z

I would many +1's the way Google show this as being a good pattern for us to follow. Thanks for all your work on this @talldan.

talldan · 2018-08-01T08:58:15Z

@jasmussen I've made some changes today based on the feedback. Here's how it looks now:

I can't get the modal to show.

Looks like the editor needed focus for shortcut to work, I've fixed that now and it should work regardless.

Also, can we make it Shift+? instead?

The drawback with Shift+? is that it can't be used while typing, whereas Cmd+? can be. I'm happy to change it but thought I'd at least make you aware of why I went for Cmd+?.

Finally, can we add it as a menu item

I still need to work on this. Will pick that up next.

jasmussen · 2018-08-01T09:13:45Z

The drawback with Shift+? is that it can't be used while typing, whereas Cmd+? can be. I'm happy to change it but thought I'd at least make you aware of why I went for Cmd+?.

Fair point. @karmatosed any thoughts? Go for Google consistency or go for the meta key?

Might meta+? conflict with other things?

Looking forward to the menu item!

Modal looks great. Awesome work. A few small things:

"Select all" — might be cool to rephrase this so it notes that you can do multiple consecutive instances of this. For example

Select all blocks when no block is selected, or select all text in current text block ⌘A
Select all blocks from within a text block ⌘A ⌘A

Something like that, but perhaps with @michelleweber help.

Typo: "Delete the select block(s)" — should be "selected" no?

On the ticket it was noted that ⌘+ duplicates for zoom, and ⌘D duplicates for bookmark. What other combos can we use here? ⌘+Shift+ + ? ⌘+Shift+D?

earnjam · 2018-08-01T14:47:20Z

@talldan Design is looking good! I'm not a designer, but have two small points of feedback:

The section headers sort of blend in with the shortcut descriptions making it hard to find the section you're looking for. Perhaps a heavier weight, or larger size would help?
It's a bit difficult to trace across the line to the actual keyboard shortcut from the shorter descriptions. There is a lot of scanning eyes back and forth to read down the list.
Google handles this in the design posted above by putting the shortcut on the left side and aligning them right so they are flush with the descriptions.
This is what Cheatsheet for OSX does as well:

Slack adds little grey lines to help easily align shortcut to description

talldan · 2018-08-02T09:55:45Z

Here's another update:

I think I've addressed a lot of the feedback

Update copy and some of the shortcuts
Added some border between the rows
Add an option to the more menu to open the modal

@jasmussen - I haven't been able to find any issues with using Cmd+?, but I haven't checked Ctrl+? on Windows or Linux. I'll start looking at the best way to set up my windows box tomorrow.

talldan · 2018-08-02T09:58:06Z

Just spotted, I should also add the shortcut to the close modal button :)

jasmussen · 2018-08-02T10:46:29Z

Love it. This is really coming together. Thanks for adding the menu item, modal works great.

I still can't get ⌘+? to work on my danish keyboard. Question mark is the shift option of the plus button, so if I press ⌘? I zoom in. If I press ⌘+Shift+? I open Chrome help.

I think we either need to find a different shortcut — perhaps one localized to each keyboard, or one that leverages a key that's in the same place in all locales. Or, just be fine with NOT having a keyboard shortcut for help.

Does TinyMCE have a keyboard shortcut for help, that we can use?

Also I think it's worth adding that if you are in text, and press ⌘A you first select all the text in that block, but if you press ⌘A again (with that text selected), you expand to select all blocks. It's a cool feature and worth noting.

afercia · 2018-08-02T12:05:14Z

Does TinyMCE have a keyboard shortcut for help, that we can use?

It uses Ctrl+Alt+H (Edit: however, I wonder if it would conflict with the classic block)

afercia · 2018-08-02T12:26:20Z

I'm not sure I see some of the keyboard shortcuts specific for keyboard navigation, e.g.:

'alt+f10': this.focusToolbar

and the ones to jump through the main landmark regions:

'ctrl+`': this.focusNextRegion
'ctrl+shift+`': this.focusPreviousRegion

Worth noting #8005 is going to add a couple additional shortcut.

I'd suggest to group al the shortcuts related to navigation under a group "Navigation".

afercia · 2018-08-02T12:44:25Z

I still can't get ⌘+? to work on my danish keyboard. Question mark is the shift option of the plus button, so if I press ⌘? I zoom in.

Can confirm it doesn't work with an Italian keyboard layout too. Worth noting using ? for the shortcut exposes to the same problem faced for other shortcuts: not all the keyboard layout have non-alphanumeric keys in the same location: https://it.wikipedia.org/wiki/File:Italian_Keyboard_layout.svg

Tested on Window Chrome:

pressing Ctrl + single quote or Ctrl + Shift + single quote doesn't do anything
what works, surprisingly, is Shift + ù

Given all the exploration made in #3218 I'd suggest to use two modifiers and a character-key.

chrisvanpatten · 2018-08-02T12:58:11Z

The drawback with Shift+? is that it can't be used while typing, whereas Cmd+? can be. I'm happy to change it but thought I'd at least make you aware of why I went for Cmd+?.

FWIW, here are what a few popular web apps do (not that we should necessarily copy other web apps, but just for testing purposes)…

Gmail: Shift+/
Slack: Cmd+/
Google Docs: Cmd+/
GitHub: Shift+/

@afercia @jasmussen Out of curiosity, does cmd or shift + forward slash work for you? (Not "does it work for you" in an "are you okay with that shortcut" sense, but from a practical/usability sense, like is it literally possible on your keyboard layouts?)

afercia · 2018-08-02T16:26:55Z

No, also forward slash is placed differently in various keyboard layouts. See the link to the wikipedia page in #3218 (comment)

talldan · 2018-08-03T09:57:17Z

I spent too much time looking into this today, and have come to the conclusion that I don't think there's an easy way to bind either cmd/ctrl + ? or cmd/ctrl + / and have it work well across keyboard layouts.

Looking at how slack handles it, they use a different shortcut depending on the keyboard layout:
https://get.slack.help/hc/en-us/articles/115005429666-Manage-your-keyboard-layout

I tested and in Italian and Danish layouts they use cmd + .

Google docs doesn't bother handling this at all.

We could consider using cmd+k as the shortcut. Either on its own or as an additional shortcut.

? on its own may be possible, though it'd not be possible while typing in a block.

jasmussen · 2018-08-03T11:07:13Z

I personally think it's okay if the help shortcut doesn't work while typing. In fact I also think it's okay to not have a shortcut at all — you can access the More menu using the keyboard and then invoke the help modal from there. Perhaps in order to ship the improvement that is this modal in the first place, it would be good to remove the shortcut altogether and possibly revisit later?

Did you try Ctrl+Alt+H which is what TinyMCE uses? Be sure to test also in the classic block.

noisysocks · 2018-08-06T03:56:50Z

Is it easy to detect the keyboard layout and do what Slack does? Otherwise I reckon let's borrow the TinyMCE shortcut for now.

jasmussen · 2018-08-06T07:17:55Z

Is it easy to detect the keyboard layout and do what Slack does?

I can't say with certainty because I'm not as skilled codewise as many of you are. And I would also love us to do this — it feels like the way superior solution. But my inkling is that it is extremely difficult, very onerous and time consuning, and prone to error as there are many many keyboard layouts we have to account for. Given how close we are to 5.0 deadlines, I'm thinking we should either keep things simple in order to ship improvements and iterate later, or just ship the improvements and be fine with not having any, or a clever, shortcut.

…er shortcuts

…as the classic editor

Using newly introduced displayShortcutList function results in cleaner, better tested code

…e lines)

…u item

noisysocks

Love it! Great job.

aduth · 2018-08-11T18:37:07Z

test/e2e/support/utils.js

+	const modifierKeys = castArray( modifiers );
+
+	await Promise.all(
+		modifierKeys.map( async ( modifier ) => page.keyboard.down( modifier ) )


Minor: There's no need for this to be an async function, since the await keyword is not used within the function's body.

afercia · 2018-08-26T14:19:47Z

Pressing ctl+option+h (shift+alt+h on windows/linux) displays a modal detailing keyboard shortcuts.

when VoiceOver is running, ctrl+option+h opens the VoiceOver help
shift+alt+h on Windows opens the Firefox and IE11 Help menu

Will open a separate issue.

talldan requested a review from jasmussen July 31, 2018 08:41

talldan self-assigned this Jul 31, 2018

talldan changed the title ~~Add/keyboard shortcut modal~~ Add new keyboard shortcuts help modal Jul 31, 2018

jasmussen mentioned this pull request Jul 31, 2018

Keyboard shortcuts support #71

Closed

35 tasks

talldan force-pushed the add/keyboard-shortcut-modal branch from ef530a9 to 8bfcd16 Compare August 1, 2018 07:29

talldan force-pushed the add/keyboard-shortcut-modal branch from ba70cfd to fe8a15c Compare August 2, 2018 09:56

talldan added 16 commits August 10, 2018 10:19

Rename class name to reflect component is now in edit-post, and reord…

8cd1a3d

…er shortcuts

Display shortcut for more menu menu item

941771b

Disable WP_Help TinyMCE command for classic block

886f147

Change keyboard shortcut for triggering help to access + h, the same …

2f0bd3d

…as the classic editor

Add utility to keycodes for returning key combinations in an array

bb9be30

Avoid ugly regular expression to split key combination into array

282b3af

Using newly introduced displayShortcutList function results in cleaner, better tested code

Add e2e test for shortcut help modal

79d4a58

Vertically center shortcut text (if adjacent label wraps onto multipl…

cd508ba

…e lines)

Close more menu when keyboard shortcuts modal is opened using the men…

1e8c7b4

…u item

Use castArray over lengthy ternary

2e5fdce

Styling linter fixes

99ac0ca

JSDoc fixes

0fe9a1c

Group activeModal unit tests in a describe

223718b

Add test for initial activeModal state

f3c8056

Fix incorrect state used in test

6020c68

Update list of key shortcuts for the help modal

3374379

talldan force-pushed the add/keyboard-shortcut-modal branch from faf4976 to 3374379 Compare August 10, 2018 02:23

talldan added 2 commits August 10, 2018 10:28

Remove unecessary return

d7aedc2

Make KeyboardShortcutsHelpMenuItem only open the help modal

52aa738

talldan removed Needs Copy Review Needs review of user-facing copy (language, phrasing) [Status] In Progress Tracking issues with work in progress labels Aug 10, 2018

noisysocks approved these changes Aug 10, 2018

View reviewed changes

talldan merged commit 0a21147 into master Aug 10, 2018

talldan deleted the add/keyboard-shortcut-modal branch August 10, 2018 02:57

aduth reviewed Aug 11, 2018

View reviewed changes

afercia mentioned this pull request Aug 26, 2018

Change keyboard shortcut for remove block to Cmd+Shift+X / Ctrl+Shift+X #9190

Merged

4 tasks

This was referenced Aug 26, 2018

The keyboard shortcut to open the Keyboard Shortcuts modal conflicts with native shortcuts #9361

Closed

Some keyboard shortcuts are announced incorrectly by screen readers #9362

Closed

chrisvanpatten mentioned this pull request Oct 22, 2018

Use ? and Alt+? to open keyboard shortcuts panel #10914

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new keyboard shortcuts help modal #8316

Add new keyboard shortcuts help modal #8316

talldan commented Jul 31, 2018 •

edited

Loading

talldan commented Jul 31, 2018 •

edited

Loading

jasmussen commented Jul 31, 2018

michelleweber commented Jul 31, 2018

karmatosed commented Jul 31, 2018

talldan commented Aug 1, 2018 •

edited

Loading

jasmussen commented Aug 1, 2018

earnjam commented Aug 1, 2018

talldan commented Aug 2, 2018 •

edited

Loading

talldan commented Aug 2, 2018

jasmussen commented Aug 2, 2018

afercia commented Aug 2, 2018 •

edited

Loading

afercia commented Aug 2, 2018

afercia commented Aug 2, 2018

chrisvanpatten commented Aug 2, 2018 •

edited

Loading

afercia commented Aug 2, 2018

talldan commented Aug 3, 2018 •

edited

Loading

jasmussen commented Aug 3, 2018

noisysocks commented Aug 6, 2018

jasmussen commented Aug 6, 2018

noisysocks left a comment

aduth Aug 11, 2018

afercia commented Aug 26, 2018

Add new keyboard shortcuts help modal #8316

Add new keyboard shortcuts help modal #8316

Conversation

talldan commented Jul 31, 2018 • edited Loading

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

talldan commented Jul 31, 2018 • edited Loading

jasmussen commented Jul 31, 2018

michelleweber commented Jul 31, 2018

karmatosed commented Jul 31, 2018

talldan commented Aug 1, 2018 • edited Loading

jasmussen commented Aug 1, 2018

earnjam commented Aug 1, 2018

talldan commented Aug 2, 2018 • edited Loading

talldan commented Aug 2, 2018

jasmussen commented Aug 2, 2018

afercia commented Aug 2, 2018 • edited Loading

afercia commented Aug 2, 2018

afercia commented Aug 2, 2018

chrisvanpatten commented Aug 2, 2018 • edited Loading

afercia commented Aug 2, 2018

talldan commented Aug 3, 2018 • edited Loading

jasmussen commented Aug 3, 2018

noisysocks commented Aug 6, 2018

jasmussen commented Aug 6, 2018

noisysocks left a comment

Choose a reason for hiding this comment

aduth Aug 11, 2018

Choose a reason for hiding this comment

afercia commented Aug 26, 2018

talldan commented Jul 31, 2018 •

edited

Loading

talldan commented Jul 31, 2018 •

edited

Loading

talldan commented Aug 1, 2018 •

edited

Loading

talldan commented Aug 2, 2018 •

edited

Loading

afercia commented Aug 2, 2018 •

edited

Loading

chrisvanpatten commented Aug 2, 2018 •

edited

Loading

talldan commented Aug 3, 2018 •

edited

Loading