WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.23k stars 4.08k forks source link

Command Palette: Global command shortcut conflicts with existing shortcut #51737

Open t-hamano opened 1 year ago

t-hamano commented 1 year ago

Description

On Rich Text, there is a shortcut that converts the selected text into a link, but it conflicts with the global menu shortcut (command center) because they have the exact same key combination.

Executing this shortcut will bring up the link control and command menu. The link control is covered by a more front-facing command menu overlay. The cursor is focused on the link control and cannot be erased without clicking the command menu overlay with the mouse.

Update:I may have been validating in the wrong environment or on a different branch. To be precise, the command palette appears for a moment and then the link add popover appears. I have updated the screencast as well.

I think the key combinations for the command menu shortcuts should be changed.

Step-by-step reproduction instructions

Execute the following shortcut on a paragraph block, for example.

Screenshots, screen recording, code snippet

https://github.com/WordPress/gutenberg/assets/54422211/430a714f-31fd-4ef0-992e-23763b7dc022

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

ndiego commented 1 year ago

@t-hamano do you think we can list this as a bug? Feels buggy to me

t-hamano commented 1 year ago

do you think we can list this as a bug? Feels buggy to me

Yes, I too think it is a bug. It seems to me that the only way to keep the existing add link shortcut working is to change the command center key combination.

@youknowriad If you have a better solution, please let us know.

youknowriad commented 1 year ago

I think this issue might be a duplicate because I remember that this was mentioned before. Basically, the ultimate goal is to make the link modal part of the command center somehow.

t-hamano commented 1 year ago

However, in WordPress 6.3, I think we need some approach to solve this problem. Do you have any suggestions on how to do this?

youknowriad commented 1 year ago

I'm not sure that's something we can fix properly in time for 6.3. The potential solution I have is to actually make a command to create links that is only available when a RichText is selected, combined with a suggestion I heard from @richtabor where commands could render something instead of triggering something. It is a bit involved though.

stokesman commented 1 year ago

The command center has not be released in core correct? Would the proper fix not be to change the shortcut to one that doesn't conflict?

If anyone can point me to one, I'd love read up on some of discussion that was had on choosing the shortcut. I thought I'd seen it somewhere (and even thought I'd seen someone mention this conflict ahead of time). I'm not finding it now.

EDIT: from the original PR:

…when a RichText is selected, it's not triggered because of the existing link shortcut.

https://github.com/WordPress/gutenberg/pull/49330#issuecomment-1491988742

That seems preferable to how it is now but less so than a separate shortcut because of confusion like #51182.

stokesman commented 1 year ago

Let's discuss the pros/cons of using a separate shortcut in the context of this upcoming release.

Pros:

Cons:

Okay, a bit cheeky on the cons but I'm hoping to prompt someone to assist here. I suspect that ye Automatticians believe the current shortcut is the obvious choice due to your slack (and likely docusaurus) habituation.

For a broad audience I would have to guess they won't have such preconceptions so why pick one with a conflict? Some alternatives:

That last example also points to the way we could alias the shortcut once a command-based add-link like what Riad speculated about https://github.com/WordPress/gutenberg/issues/51737#issuecomment-1607365237 manifests and is equally effective as the current UX.

youknowriad commented 1 year ago

⌘+AltK How GitHub aliases their Command Palette so the Markdown add link shortcut isn't conflicted.

I've been unable to trigger the GitHub command palette when the comment text box is focused (either with the original or aliased shortcut).


For what it's worth, I don't have a strong opinion on the shortcut. The main "con" of an alternative shortcut is the fact that cmd+k is kind of widely used in the web today. That said, I can be onboard with two shortcuts to trigger the command center. Cmd+k being there for folks that are used to it elsewhere with the caveat that it doesn't work right now in RichText.

cc @richtabor @jasmussen @jameskoster

richtabor commented 1 year ago

For a broad audience I would have to guess they won't have such preconceptions so why pick one with a conflict?

It'd be neat if you could assign alternate shortcuts (in the future).

richtabor commented 1 year ago

This is what I'm seeing on trunk; I think it's acceptable.

https://github.com/WordPress/gutenberg/assets/1813435/bb5aedf9-a07d-40db-99f0-af73e448d6be

t-hamano commented 1 year ago

I may have been validating in the wrong environment or on a different branch. To be precise, the command palette appears for a moment and then the link add popover appears. I have updated the description and screencast. I am sorry for the confusion 🙇

joedolson commented 1 year ago

@richtabor We do have an open issue for assigning custom shortcuts: https://github.com/WordPress/gutenberg/issues/3218

It would be great to see that move forward. It's had a lot of work put into it already, just needs some dev commitment. Allowing users to customize shortcuts would make problems like this much more manageable.

stokesman commented 1 year ago

Thanks Riad 🙇

I've been unable to trigger the GitHub command palette when the comment text box is focused

Works for me on Chrome/macOS, maybe it's buggy elsewhere. It's in their docs.


This is what I'm seeing on trunk; I think it's acceptable.

Rich, thank you, though I am left wondering if you considered the whole enchilada here. Let's say your intention is to launch the command palette. Won't it be frustrating when it doesn't work from the rich text area?


I hope this is a better distillation: Benefit Tradeoff*
Current Shortcut A portion of the audience gets to use their accustomed shortcut although it can feel flaky. The shortcut can feel flaky for the whole audience.
Candidate Shortcut The shortcut always works for the whole audience. A portion of the audience won't get to use their accustomed shortcut.

*We may suppose that either tradeoff can be eliminated in a later release with https://github.com/WordPress/gutenberg/issues/51737#issuecomment-1607365237. This may prove true but otherwise which tradeoff would we rather be stuck with?

draganescu commented 1 year ago

I love the Command Palette, yet the implementation of the global keyboard shortcut seems to be somewhat disorienting. With a seemingly global shortcut that isn't entirely 'global,' I found myself on a bit of a wild goose chase when first attempting to test it out. The shortcut constantly opened link UI, which led me to the Gutenberg experiments page and subsequently to Github, believing I am doing something wrong, all the while keeping me oblivious to the need to deselect the text based block.

In my view, a feature as robust as the Command Palette should be launched smoothly and intuitively. This shortcut confusion feels as if there is more broken.

Riad has already pointed out that CMD+k doesn't operate as expected when the comment box is in focus. At least on Safari OSX, we're seeing the link format for markdown instead. However, the commenting experience on a GH issue or PR is modal, a separate experience mindset, while our default block is a paragraph, and most blocks include text. Furthermore, the current state of block deselection leaves something to be desired. So I think the example does not apply.

What do you think of a temporary change of plan? Let's hunt down an available keyboard shortcut, assign it to the Command Palette, and revisit Rich's idea of linking text via the command palette later. Once we're ready, we can update it to use CMD+k. Surely, communicating a shortcut update wouldn't be the most challenging task we've faced. We're not dealing with an age-old command like undo or redo, but rather a fairly recent innovation. Command palettes themselves are a relatively novel concept in the tech world, so I doubt we'd lose users over a temporary change in shortcut.

t-hamano commented 1 year ago

Perhaps this is not what was intended, but I have found that executing this shortcut twice on rich text can launch the command palette.

When the shortcut is executed the first time, the Command Palette flashes for a moment and the link UI opens. Since the focus has shifted to the link UI, the second shortcut would be able to activate the command palette.

However, I personally think it makes more sense to change the shortcut key, since this behavior seems unnatural.

https://github.com/WordPress/gutenberg/assets/54422211/011bd148-2d22-4888-b35d-5276de2a6fb2

stokesman commented 1 year ago

Let's hunt down an available keyboard shortcut, assign it to the Command Palette

I'm favoring forward slash and made the PR as such. I'm totally willing to change that but here’s what I think weighs in favor of it.

richtabor commented 1 year ago

Perhaps this is not what was intended, but I have found that executing this shortcut twice on rich text can launch the command palette.

I don't think the common use case would be to highlight text, then attempt to trigger the command palette (to do something else). They're not commonly related actions.

As an alternative, can we have CMD+K only attempt to make a link, if there is text indeed highlighted? Then there wouldn't be a conflict with CMD+K opening the command palette.

I didn't realize CMD+K worked without selecting text first—seems more a bug than a feature.

t-hamano commented 1 year ago

As an alternative, can we have CMD+K only attempt to make a link, if there is text indeed highlighted? Then there wouldn't be a conflict with CMD+K opening the command palette.

I think there may be use cases where you want to insert a link even when the text is not highlighted. For example, if you use the link UI when text is not highlighted, you can insert linked text.

https://github.com/WordPress/gutenberg/assets/54422211/b2151cf9-0bee-4e58-b22a-351338ee08ca

richtabor commented 1 year ago

For example, if you use the link UI when text is not highlighted, you can insert linked text.

I still don't think that's a common practice at all.

stokesman commented 1 year ago

I still don't think that's a common practice at all.

Sure, I'd bet that's correct yet that doesn't mean it’s a bug. The create link shortcut has worked like that since before the block editor was even around.

Additionally, trying to deconflict the shortcut as suggested would leave conflicts in corner cases like someone having text selected either without intent (low dexterity/accidents) or because the selection isn't cleared after applying other formats.

t-hamano commented 1 year ago

I prefer to simply assign different shortcut keys to the command palette rather than change shortcuts depending on the text selection.

In #52386, it is suggested to use forward slash. As noted in this comment, I'm concerned about cases where third-party plugins have the functionality to toggle comments via the same shortcut, but I believe this would have less impact than conflicting with the inline link UI.

stokesman commented 1 year ago

No one is making much of a case against changing the shortcut. Could ”silence deems consent” be applied here?

draganescu commented 1 year ago

I would like to add that pressing cmd k twice, once for the link, once for command center, when a button is selected does not work as the command center appears and quickly disappears for some reason.

t-hamano commented 1 year ago

I'd like to endorse the forward slash shortcut suggested in #52386, but I would be happy to hear your opinions.

richtabor commented 1 year ago

I'd like to endorse the forward slash shortcut suggested in https://github.com/WordPress/gutenberg/pull/52386, but I would be happy to hear your opinions.

I still don't think changing the command is good idea. Maybe introducing a mechanism for users to decide what shortcut is employed, but not just pushing a change that obscures the command palette further.

It has some precedence in that Figma and Docusaurus (and maybe others) use it for their command/search palettes

Everyone has a preference on what shortcut is used. I'm concerned this will bury the command palettes usefulness; it's not likely to be a key combination that most folks can implement without having to find the / key.

fwiw, Figma also employs CMD + P.

stokesman commented 1 year ago

I still don't think changing the command is good idea.

The feature is designed to be globally accessible by a keyboard shortcut. The current shortcut fails to fulfill the design. Another shortcut can fulfill the design and be just as easy to utilize. How is changing to it not a good idea?

Bear in mind nothing precludes using CMD+K at a later date.

draganescu commented 1 year ago

Hello, I wanted to add my understanding of the aspects here too for visibility:

richtabor commented 1 year ago

As an alternative, can we have CMD+K only attempt to make a link, if there is text indeed highlighted? Then there wouldn't be a conflict with CMD+K opening the command palette.

This seems the most sensible.

stokesman commented 1 year ago

That can be contested: Most sensible seems to be aliasing another shortcut that works regardless of context. GitHub, for example, opted to do that:

image

My only gripe with that is the one that works everywhere should be documented as the primary one. Also there's no reason it has to be a three-key variation of the other.

stokesman commented 1 year ago

Here’s a tidbit I hope we’ll all agree on: The visual regression that comes along with this should be fixed. Here’s the simplest way I found to do it #53001.

bph commented 1 year ago

@stokesman @t-hamano We'l move the issue off the 6.3 project board, and punt it to 6.4 so it stays active. The visual regression PR #53001 made it into 6.3 RC 3

There is also a related discussion happening on this PR: https://github.com/WordPress/gutenberg/pull/52386

afercia commented 11 months ago

Quick thought from my side:

(Cmd+K) In most text based apps (e.g. online MSWord) it adds links.

Correct, and that's a de facto standard singe ages. I'd like to remind also TinyMCE uses Cmd+K ant WordPress users are very familiar with it.

As an alternative, can we have CMD+K only attempt to make a link, if there is text indeed highlighted? Then there wouldn't be a conflict with CMD+K opening the command palette.

I think this would be less than ideal. As often happens, smart behaviors may seem a good idea at first but they likely fail with helping users in the long run. Also, WordPress (with TinyMCE and the Editor so far) has always allowed users to insert link with or without text selection, for good usability reasons. I'd strongly recommend against the idea of requiring an actual text selection, which would only partially solve the conflict anyways.

afercia commented 11 months ago

Worth also noting the keyboard shortcut conflict is also visible in the keyboard shortcuts modal dialog:

Screenshot 2023-09-04 at 10 11 53

Overall, I'm not sure why releasing software with known bugs is now considered acceptable in WordPress. And this is not the only known bug. I do realize such a consideration is out of the scope of this issue btu I'd think it's something that should be discussed at a higher level.

t-hamano commented 11 months ago

With the introduction of the Command Palette in WordPress 6.3 and the impending release of WordPress 6.4, I think we need to reconsider how to deal with this issue.

Personally, I think it would be better to change the key combinations as suggested in #52386 that do not completely conflict with other shortcuts or add aliases, rather than trying to somehow deal with the current key combinations as they are.

For example, in #54515, it is suggested that the command palette be made available anywhere in WordPress. This might conflict with the shortcut for users using the Classic Editor.

265574294-797095f1-2a1f-4f90-9d0f-54508814365b

justlevine commented 11 months ago

Late to the party, but to add another 👎 to switching what CMD+K does based on context ( that I don't believe has been mentioned yet ) is the cognitive load involved (#a11y).

Learning a global shortcut is a one time learning curve. Using that changes based on what you have actively selected adds cognitive load to every single interaction with that shortcut.

priethor commented 7 months ago

Late to the party, but to add another 👎 to switching what CMD+K does based on context ( that I don't believe has been mentioned yet ) is the cognitive load involved (#a11y).

This is a common pattern in similar software across the industry, and, in my humble opinion, working differently than the rest of the software adds more cognitive load. A good example of this is how Gutenberg lists are not indented with TAB; I still make the mistake of hitting TAB to this date because using uncommon shortcuts puts the burden of knowing the context on the user (in this case, the context of what text/block editor I'm using).

All in all, opening the command palette only when there is no text selected seems like the best approach to me.

t-hamano commented 7 months ago

Based on the discussion so far, there are probably three possible approaches at this point:

  1. Change to another keyboard shortcut such as /: https://github.com/WordPress/gutenberg/pull/52386
  2. Stop the link control from opening via command + k when we don’t have any text selected (See this comment)
  3. Integrate Link Controller and Command Palette (See this comment)
annezazu commented 6 months ago

After a review by core editor triage and core editor tech leads on the WordPress 6.5 release team, this issue is being removed from the release due to lack of consensus and progress.

I do want to know this in progress PR as related to this effort and something that might be possible to get in for 6.5 otherwise: https://github.com/WordPress/gutenberg/pull/58179 cc @getdave who perhaps can speak to this (and why it's closed?).

afercia commented 6 months ago

This is a common pattern in similar software across the industry, and, in my humble opinion, working differently than the rest of the software adds more cognitive load. A good example of this is how Gutenberg lists are not indented with TAB ...

I'm not sure comparing these two cases is a good argument. I'd encourage everyone to not think as an advanced user and rather try to think as an average, not tech-savvy user. To me, these two cases are very different.

t-hamano commented 6 months ago

Suppose we change the shortcut key from the current cmd+k to another shortcut key, for example cmd+/. As one approach to reduce the inability to recognize the key change, I propose displaying a snackbar to notify the user that the key has changed (See this comment).

getdave commented 6 months ago

After a review by core editor triage and core editor tech leads on the WordPress 6.5 release team, this issue is being removed from the release due to lack of consensus and progress.

I do want to know this in progress PR as related to this effort and something that might be possible to get in for 6.5 otherwise: #58179 cc @getdave who perhaps can speak to this (and why it's closed?).

The approach in #58179 was closed because there is an established convention in the editor that pressing Cmd + K within richtext will create a link even if no text is selected.

My PR was attempting to disable the ability to create links from unselected text as this felt to me like an anti-pattern. However, as you can see in the PR's comments, various folks contributed reasons why my assumption is incorrect.

This is unfortunate as my PR did partially solve the matching shortcut issue.

Note currently on trunk the command palette will only be invoked when you are outside a rich text context. So it is contextual. Will our users expect to be creating links when they are not within a rich text area.

draganescu commented 6 months ago

there is an established convention in the editor that pressing Cmd + K within richtext will create a link even if no text is selected.

Just want to clarify: inseting inline links while in the writing flow is a feature, present in other editors, inline with other formats, not a convention.

justlevine commented 6 months ago

there is an established convention in the editor that pressing Cmd + K within richtext will create a link even if no text is selected.

Just want to clarify: inseting inline links while in the writing flow is a feature, present in other editors, inline with other formats, not a convention.

I'm pretty sure the "established convention" here is in reference to WordPress itself which has used Cmd+K to insert links for years.

liviopv commented 2 months ago

While reviewing feedback for other 6.6 features, the conflict with Cmmd+K when a Paragraph Block is selected was mentioned twice, which surprised me. It's the first time I've received proactive feedback about the Command Palette, so it might be an interesting topic to discuss in future release cycles.

priethor commented 1 month ago

@liviopv, did the feedback say how they would expect it to work?

mrfoxtalbot commented 1 month ago

I am not Livio but I got some feedback about this today.

how they would expect it to work?

Not really, no. I am not sure how to fix this, we are taking over an existing (and very well-known) keyboard shortcut.

liviopv commented 1 month ago

I don't know how to fix it either without changing one of the commands.

Perhaps using Cmmd+K in a Paragraph Block would toggle the link inserter AND also toggle a snackbar notification explaining how to toggle the palette in this state?