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

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

Open
t-hamano opened this issue Jun 21, 2023 · 52 comments · May be fixed by #52386
Open

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

t-hamano opened this issue Jun 21, 2023 · 52 comments · May be fixed by #52386
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Feedback Needs general design feedback. [Package] Commands /packages/commands [Package] Format library /packages/format-library [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] Comand Palette

Comments

@t-hamano
Copy link
Contributor

t-hamano commented Jun 21, 2023

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.

  • Windows: Ctrl + k
  • Mac: + K

Screenshots, screen recording, code snippet

c2af9271e04c13c5fc2b0960e15cfcb2.mp4

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
Copy link
Member

ndiego commented Jun 26, 2023

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

@t-hamano
Copy link
Contributor Author

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.

@ndiego ndiego added the [Type] Bug An existing feature does not function as intended label Jun 26, 2023
@youknowriad
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

stokesman commented Jun 26, 2023

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.

#49330 (comment)

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

@stokesman
Copy link
Contributor

stokesman commented Jun 27, 2023

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:

  • +/ We have slash inserter, how about slash-it-all 🦾
  • +Shift+P I bet you know it but that’s probably no reason to favor it
  • +Alt+K How GitHub aliases their Command Palette so the Markdown add link shortcut isn't conflicted.

That last example also points to the way we could alias the shortcut once a command-based add-link like what Riad speculated about #51737 (comment) manifests and is equally effective as the current UX.

@youknowriad
Copy link
Contributor

⌘+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
Copy link
Member

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
Copy link
Member

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

CleanShot.2023-06-27.at.18.01.14.mp4

@t-hamano
Copy link
Contributor Author

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
Copy link
Contributor

@richtabor We do have an open issue for assigning custom shortcuts: #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
Copy link
Contributor

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 #51737 (comment). This may prove true but otherwise which tradeoff would we rather be stuck with?

@draganescu
Copy link
Contributor

draganescu commented Jul 6, 2023

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
Copy link
Contributor Author

t-hamano commented Jul 6, 2023

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.

077b45ed7695e7dc0741ae802392e9f2.mp4

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Jul 6, 2023
@stokesman
Copy link
Contributor

stokesman commented Jul 6, 2023

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.

  • It goes well with the slash inserter shortcut
  • It has some precedence in that Figma and Docusaurus (and maybe others) use it for their command/search palettes

@draganescu draganescu added the Needs Design Feedback Needs general design feedback. label Jul 7, 2023
@richtabor
Copy link
Member

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.

@afercia
Copy link
Contributor

afercia commented Sep 4, 2023

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
Copy link
Contributor Author

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
Copy link

justlevine commented Sep 22, 2023

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.

@bph bph moved this from Needs Decision / Discussion to Punted to 6.5 in WordPress 6.4 Editor Tasks Sep 23, 2023
@bph bph moved this to 🏗️ In Progress in WordPress 6.5 Editor Tasks Nov 23, 2023
@priethor
Copy link
Contributor

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
Copy link
Contributor Author

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

  1. Change to another keyboard shortcut such as /: Try forward slash keyboard shortcut for command palette #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
Copy link
Contributor

annezazu commented Feb 8, 2024

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?).

@afercia
Copy link
Contributor

afercia commented Feb 8, 2024

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.

  • Many users are used to use editors like Word and Google Docs. In the context of this kind of text editors, many users have learned that pressing Tab indents text.
  • Very few users, usually advanced users like developers or designers, use highly specialized software where the Cmd+K keyboard shortcut may open a command palette. This convention is used in a very niche kind of products the average users don't use and will never use. As such, I think the vast majority of WordPress users don't expect and will never expect CMd+k to open a Command palette.
  • Rather, Cmd+K has always been since ages a very known convention to insert links.

@t-hamano
Copy link
Contributor Author

t-hamano commented Feb 8, 2024

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
Copy link
Contributor

getdave commented Feb 8, 2024

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
Copy link
Contributor

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
Copy link

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
Copy link

liviopv commented Jun 12, 2024

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
Copy link
Contributor

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

@mrfoxtalbot
Copy link

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
Copy link

liviopv commented Jul 18, 2024

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?

@MadtownLems
Copy link

I've finally been playing with the Command Palette recently, and this issue is my primary complaint about implementation thus far. Having ctrl+k open the link dialog - instead of the command palette - while the cursor is inside a Paragraph Block makes the keyboard shortcut (and by extension, the entire Command Palette) MUCH less useful for content providers.

I also think the current situation is even more confusing for users than most people realize. Hitting ctrl+K opens the dialog box to create a link, but that box isn't labeled at all. I'd think most users - at least most of OUR users - won't even know what that box that just popped up is for!

It may not have mattered as much when the Command Palette was only shown in the Site Editor, and not the Post Editor. But now that it's present when writing content, and has its keyboard shortcut prominently displayed inside it, it's time to get this resolved.

I think @t-hamano did a great job of illustrating the 3 viable approaches here: #51737 (comment)

How can we get consensus on which approach to go with and move towards tackling this?

@afercia
Copy link
Contributor

afercia commented Sep 9, 2024

I'd agree this issue would need some more focus to be finally solved. To me:

  • Inserting a link via Cmd/Ctrl + K is the industry standard since ages for all the HTML editors I've seen across the years. It should not be changed.
  • Instead, the easiest solution is to just change the keyboard shortcut for the Command palette.

but that box isn't labeled at all. I'd think most users - at least most of OUR users - won't even know what that box that just popped up is for!

I agree that all dialogs, popovers and such should be clearly labeled with a visible title. That's an accessibility and usability issue across many UIs in the editor but seems there's no consensus on a way forward. It should go in a separate issue though.

@richtabor
Copy link
Member

Inserting a link via Cmd/Ctrl + K is the industry standard since ages for all the HTML editors I've seen across the years. It should not be changed.

For selected text, yes this is the standard. Less confident otherwise.

@afercia
Copy link
Contributor

afercia commented Nov 6, 2024

For selected text, yes this is the standard. Less confident otherwise.

All the following largely used applications open their 'insert link' UI when pressing Cmd+K even with no text selection and even on a new empty line:

  • Google Docs (and I guess all the other Google apps)
  • Microsoft Word (I could only test the web version in Microsoft Teams).
  • TinyMce, as one of hte most known WYSIWYG editors.

@andreawetzel
Copy link

andreawetzel commented Nov 27, 2024

It's confusing that ⌘K brings up the link dialog when paragraph text is selected but not when an image block is selected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Feedback Needs general design feedback. [Package] Commands /packages/commands [Package] Format library /packages/format-library [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] Comand Palette
Projects
No open projects
Status: Punted to 6.5
Development

Successfully merging a pull request may close this issue.