Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upChange keyboard shortcut for remove block to Cmd+Shift+X / Ctrl+Shift+X #9190
Conversation
talldan
added
the
[Type] Bug
label
Aug 21, 2018
talldan
added this to the 3.7 milestone
Aug 21, 2018
talldan
self-assigned this
Aug 21, 2018
talldan
requested a review
from WordPress/gutenberg-core
Aug 21, 2018
tofumatt
approved these changes
Aug 21, 2018
|
My immediate thought is this is a less nice shortcut for the majority of desktop users (Windows + Mac). I'd almost like it to be Linux-only but I get that's not the tradition in Gutenberg/WordPress. It's also a lot of extra code. So it works for me! |
| @@ -15,6 +15,10 @@ class KeyboardShortcuts extends Component { | |||
| super( ...arguments ); | |||
|
|
|||
| this.bindKeyTarget = this.bindKeyTarget.bind( this ); | |||
|
|
|||
| // Firefox uses keycode 173 for '-', while other browsers use 189. | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
aldavigdis
Aug 22, 2018
•
Contributor
I think I may have spotted an i18n issue here, using Icelandic keyboard layout on MacOS. Both [ö] and [-] results in keycode 173 in Firefox and 189 in Chrome.
Perhaps sticking with alphabetical keys is a good idea?
talldan
referenced this pull request
Aug 21, 2018
Merged
Position caret at end of previous block for any type of block removal #8961
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
One followup now that we are touching keyboard shortcuts. I noticed a pattern on the Mac for keyboard shortcuts, that we might want to adopt. Here's Google Chrome on the Mac: Note how instead of Alt, they write ⌥, instead of Shift it's ⇧. There are also no plusses between the key indicators. Can we do the same? If you actually glance down at a Mac keyboard, those same icons are used on the keys, in the case of Shift it doesn't even actually have the word "Shift" on the keyboard. I think it's a far more Mac like way of doing things and I think we should adopt it. Note that this would be a Mac only thing, on Windows we'd still have to spell out a lot of the keys — Chrome does this as well: |
This comment has been minimized.
This comment has been minimized.
|
@jasmussen I tried that approach awhile back, but we actually favour using the keyboard names over the Mac symbols unless it's Command. See the discussion here: #4411 (review) |
This comment has been minimized.
This comment has been minimized.
|
Could that discussion be revisited? Thanks for the link but I couldn't exactly extract from the history of that PR exactly why it was preferable to do the other. Revisiting this now that keyboard shortcuts are in such a great state in Note how they even go further and use ^ to imply Ctrl, which frankly I'm totally fine with. From a certain point of view this feels less like a "what we think is best" and more of a "let's match the operating system" discussion. |
This comment has been minimized.
This comment has been minimized.
|
The issue is that plenty of keyboards don't actually include the symbol–@iseulde mentioned her keyboard doesn't have those symbols and it was somewhat confusing to see them. My US Logitech keyboard doesn't ship with the symbols either: And looking at @sarahmonster's British Apple Keyboard (Magic Keyboard 2, so the newest one they make), it doesn't have the symbols: Using those symbols saves space (though only for Mac users, so we can't rely on that space-saving cross-platform) but it's certainly less clear. Sure the symbols are more idiomatic, but I think they're less usable and it's an idiom I'm fine eschewing. |
This comment has been minimized.
This comment has been minimized.
|
While the macOS key symbols are not used on all keyboards, they are used throughout the OS (like in Finder, as @jasmussen pointed out), so I think using the symbols on macOS is fine, since users would already have been exposed to the symbols in the context menus of the OS, as well as common apps like Chrome, and they do make the editor fit in a bit more with the operating system and save some space. On a more related-to-the-PR note, thank you, @talldan! I am definitely looking forward to being able to use the Remove Block shortcut on my computer. |
This comment has been minimized.
This comment has been minimized.
|
I tested this branch, and it is working for me in both Firefox and Chromium on Antergos Linux. |
This comment has been minimized.
This comment has been minimized.
|
I think this is good to merge as-is; we can have a separate discussion about keyboard symbols if @jasmussen wants to open one… though as mentioned I believe he is, like me, well-meaning but in this particular instance: ultimately wrong |
This comment has been minimized.
This comment has been minimized.
|
Hah! You're the one to claim to know better than Apple what Mac users want. So I disagree with you, but in a friendly loving way. I also don't want to block improvements like this, but I still think the discussion is worth having. The devil is in the details, and if Mac users see one keyboard combo in their OS and browser, and another in their blogging software, I'm not sure we're benefiting anyone by being "better". |
This comment has been minimized.
This comment has been minimized.
|
Consistency with the OS is what tips the balance for me. |
This comment has been minimized.
This comment has been minimized.
|
Well, I WANT to agree with you, and your arguments have convinced me. 👍🏻 But yeah, a separate issue for that would be great. You can assign it to me as penance if you like |
This comment has been minimized.
This comment has been minimized.
|
i18n issue: Using For example, - is located in the bottom row in the Danish and German keymaps, while it is in the top row in English and Icelandic keymaps. Finally, this may be minor and a bug in MacOS, but pressing Ö and - using the Icelandic keymap in MacOS results in the same key code (189 in WebKit, 173 in FF). This could cause some issues with users switching keyboard layouts frequently or others supporting people with different layout then they are used to themselves. How about using a an alphabetical key like X, which is generally located on the same physical key regardless of keymap? |
This comment has been minimized.
This comment has been minimized.
|
@aldavigdis - thanks for letting us know about that. That's very unusual! Is there a use case for Ctrl+Shift+Ö? if not, I think it's ok to merge this and have both Ctrl+Shift+Ö and Ctrl+Shift+- do the same thing. We're using mousetrap for mouse events, which uses So this shouldn't cause an issue when using different mappings. For example, on the Italian keyboard, '-' is next to the right shift, but the browser still sends through
|
jasmussen
referenced this pull request
Aug 22, 2018
Closed
Adhere to platform standards in displaying keyboard shortcuts #9226
This comment has been minimized.
This comment has been minimized.
|
What I'm emphasising is I think we should try to use alphanumeric buttons as much as possible instead of symbols because they are generally consistent across keyboard layouts. |
This comment has been minimized.
This comment has been minimized.
|
There are AZERTY keyboards for French though, and DVORAK users. I know they’re slightly more fringe but even keyboard layouts aren’t really universally consistent, and generally a user’s languages are consistent across devices so we don’t need to optimise for a user who uses both an Icelandic and French keyboard–if they do that presumably they’re used to making the switch. I don’t think having the keys in the same place across locales is needed.
|
This comment has been minimized.
This comment has been minimized.
|
@talldan As someone who's a heavy user of keyboard shortcuts, I tend to switch to the en_US layout when working with certain software as some keys like { don't even exist on many non-English layouts. Word processing or something like WordPress that runs in the browser does not fall into the category of software where I may be expected to switch layouts. There are generally consistency issues bound to using non-alphanumeric keyboard shortcuts as well as they are often either bound to the physical key, so - and + are located right next to each other as in the US keymap — or if bound to the character, all over the place — or perhaps nowhere at all. |
This comment has been minimized.
This comment has been minimized.
|
@tofumatt even across QWERTY, QWERTZ and AZERTY, a normal user can generally find an alphanumeric key naturally — and X is usually located on the same spot consistently across layouts. |
aldavigdis
referenced this pull request
Aug 22, 2018
Closed
Keyboard shortcuts do not take keyboard layouts into consideration #9227
This comment has been minimized.
This comment has been minimized.
|
I'm definitely up for defining some rules about keyboard shortcuts as discussed in the meeting last night. Off the top of my head, and in a general order of significance:
I think the trouble is that there aren't actually many shortcuts freely available after going through all of those rules. I'm definitely open to changing it if there's a good suggestion. |
afercia
changed the title
Change keyboard shortcut for remove block to crl+shift+-
Change keyboard shortcut for remove block to ctrl+shift+-
Aug 26, 2018
This comment has been minimized.
This comment has been minimized.
I'd strongly recommend to take @aldavigdis recommendation into account. For the records, it was already noted in #8316 (comment) Also, there's a problem in the way some non-alphanumeric keys are announced by screen readers, as they usually don't read out "punctuation" like |
afercia
referenced this pull request
Aug 26, 2018
Closed
Some keyboard shortcuts are announced incorrectly by screen readers #9362
This comment has been minimized.
This comment has been minimized.
|
If Ctrl+Shift+- can not be used due to accessibility issues, I would be completely happy with something like Ctrl+Shift+X. I mean, anything is better than having a keyboard shortcut that kills your GUI session. I also think the ability to remap keyboard shortcuts (#3218) would be a great addition to cover more obscure conflicts and let users choose shortcuts that work better for them, but would not work for other users. |
This comment has been minimized.
This comment has been minimized.
|
Ok - changed now. I'll do some testing tomorrow and maybe we can merge then. |
This comment has been minimized.
This comment has been minimized.
|
Nice! This PR should probably be renamed. |
noisysocks
approved these changes
Aug 28, 2018
|
Haven't tested whether the shortcuts conflict with anything, but the code still looks fresh and groovy |
tofumatt
reviewed
Aug 28, 2018
|
Just be sure to update the newly-minted keyboard shortcuts section in the reference doc |
talldan
added some commits
Aug 21, 2018
talldan
force-pushed the
update/remove-block-shortcut
branch
from
1e6f3f7
to
486ac46
Aug 28, 2018
talldan
changed the title
Change keyboard shortcut for remove block to ctrl+shift+-
Change keyboard shortcut for remove block to Cmd+Shift+X / Ctrl+Shift+X
Aug 28, 2018
This comment has been minimized.
This comment has been minimized.
|
Cool - I just did some testing. All worked well. Renamed the ticket and pushed a commit that updates the faqs. Thanks for the shortcut suggestion @ZebulanStanphill |
talldan
merged commit 32b1f92
into
master
Aug 28, 2018
talldan
deleted the
update/remove-block-shortcut
branch
Aug 28, 2018
added a commit
to ZebulanStanphill/gutenberg
that referenced
this pull request
Aug 28, 2018
This comment has been minimized.
This comment has been minimized.
|
@talldan Actually, it was @aldavigdis' idea to use X:
Also... I forgot to actually test the keyboard shortcut. Fortunately, I just did, and it does not conflict with my browser or my OS. Instead, it conflicts with WordPress. This is what that keyboard shortcut does: Oops. |
This comment has been minimized.
This comment has been minimized.
|
Actually, nevermind. It is not a WordPress behavior. I think it might be a Firefox one. Let me check what happens in Chromium. |
This comment has been minimized.
This comment has been minimized.
|
Yeah, it is a Firefox behavior. Ctrl+Shift+X toggles RTL mode in Firefox. |
This comment has been minimized.
This comment has been minimized.
|
Oh no! Wonder why I couldn't reproduce it before. Perhaps because I was only testing with a block selected. edit- I must've also been testing on a site that doesn't support rtl, so was seeing no effect. It doesn't help that this is an undocumented shortcut either. |
This comment has been minimized.
This comment has been minimized.
|
@talldan How about trying Ctrl+Shift+Alt+X? That does not appear to do anything for me in Firefox on Linux. |
This comment has been minimized.
This comment has been minimized.
|
Probably a good time to share this spreadsheet I've been working on so that we don't have to have these conversations: I plan to try and move this out of google docs form and on to github at some point soon. |
added a commit
that referenced
this pull request
Aug 28, 2018
This comment has been minimized.
This comment has been minimized.
|
Ok - I'll revert the merge since the behaviour is not desirable (e.g. if a user forgets to select a block, they're left with rtl text direction)
My feeling is that three modifiers makes it so complex it's not worth having. There are other alternatives in the spreadsheet, but they're not very nice semantically. |
talldan
referenced this pull request
Aug 28, 2018
Merged
Revert "Change keyboard shortcut for remove block to Cmd+Shift+X / Ct… #9390
added a commit
that referenced
this pull request
Aug 28, 2018
talldan
referenced this pull request
Aug 28, 2018
Closed
Remove Block keyboard shortcut causes display server on Linux to shut down or reboot #9036
This comment has been minimized.
This comment has been minimized.
|
Personally, I would not mind Ctrl+Shift+Alt+X since all the keys are pretty close to each other (on US keyboards, anyway), and it at least does not cause a display server restart. Looking at your spreadsheet, it looks like Ctrl+Alt+X is also available. Perhaps that would be a better choice? |






talldan commentedAug 21, 2018
•
edited
Description
Closes #9036
How has this been tested?
Screenshots
Block Settings Menu

Keyboard Shortcut Help

Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: