Link Shortcut: Only trigger the link shortcut if there's a text selection #66056
Conversation
youknowriad
added
[Type] Bug
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +13 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
|
Just noting that this was proposed in #51737 (comment) but didn't make it into 6.5. Has the consensus changed? |
|
Why was it closed in the first place, I was personally not aware of that fix. It seems like a good approach to me. |
I can't add much more than what's conveyed in that comment. It boils down to:
Logically I agree with your proposal but it seems there's always been the expectation around creating links this way. |
|
It's obviously hard to measure but I think triggering the shortcut and writing a link before actually writing text doesn't feel like something folks would do much. |
I agree. |
|
I sympathize with this idea as practical but it doesn’t seem like the shortcut should be contextually dependent in the first place (i.e. the same shortcut used for two separate features).
Agreed, however as mentioned by Ella #58179 (comment) it’s also consistent with other formats that have shortcuts (like bold). It does seem like for some folks it’s a standby feature. I didn’t see the comment but I remember someone mentioning they commonly insert links where the URL is the displayed text and that feature is most seamless way to do it. |
|
I agree that it's not 100% a perfect solution (at least it's better than trunk for me) but I think this is the closest we have to an agreeable/simple path forward. I feel like we can try this since we're early on the WP lifecycle and adapt to feedback if needed. |
This seems reasonable, yet I doubt that all folks this is important to are users of the plugin. Back to the broader consideration, assuming this is a net win and we keep it, we’re still limited by the same shortcut for both features. Conceivably there are uses for selecting text and commands that operate from that. One idea is a "replace all occurrences" command. |
The plugin is used in .com so I think if this is ever a problem with regular users, we'll know about it very quickly.
Yeah, for me the long term thing is to have a way to merge the "link" popover as part of the command palette (make it a contextual command or something) |
Agreed. Let's try it early in this cycle and report feedback. |
youknowriad
force-pushed
the
fix/link-cmd-k
branch
from
8a8c47f to
3d93f3d
Compare
|
Flaky tests detected in 3d93f3d. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11348197458
|
That seems a good point.
I know getting into that is digressing but I can’t envision how it would not be a poorer UX for creating links. I liked the idea way back when I first heard it and would like for it to be workable so I’d be glad to be proven wrong. Overall, there’s some practicality to this but it seems it will make it easier to remain stuck without a complete fix. It also sacrifices an existing convention and self-consistency. I think we should continue to feel the pain of the issue in hopes it eventually spurs its complete removal. |
Can you clarify what you mean here just to make sure I understand properly. Removal of what? |
|
The removal of the actual issue that two features share a shortcut. |
|
So just to be clear, you prefer we do not merge this and instead keep the statusquo? I think for me it's clear at this point that we're not going to change the command bar shortcut (as it's also a common shortcut) and the same can be said for links. So this felt let the best path forward for me to avoid getting stuck. |
I still think changing the command bar shortcut is the proper fix. Merging this feels like a step toward the assumption that we can obviate the link creation interface with the command bar and I would be happy to see that work just as well. Yet I think it has to be evidenced before we move toward that. Otherwise, we land in a spot where both features still depend on the same shortcut yet advanced commands that operate on text selections are blocked. I'm not happy with the status quo and don’t intend to block this. I appreciate your consideration. |
Fair, I think it seems that an agreement/consensus on this is going to be hard to find. I'm going to keep the issue open so we can continue this discussion.
Yes, It definitely is a path towards that. Obviously that's a bit more involved to implement. I feel this PR is a bit better than what we have but I'm happy to revert it if there's evidence that it's not. |
Right. Though, it wouldn’t take a working implementation to prove. A design POC could do. I think we’ll find it would be workable but unable to match the directness of the current link creation popover. |
|
@mtias @jasmussen @richtabor Do we have ideas/mockups about how we could merge the "link popover" with the "command palette" UI? Also, what are your current thoughts on changing the shortcut of the command palette. |
|
Maybe it's fair to acknowledge here that we don't have enough cycles to explore a full "link merged with command palette" at the moment and that we should make a decision only based on whether this PR is better than trunk or not. |
It’s myopic yet doesn’t mean we can’t redirect later so not that’s not to argue it should not be done. I just think effort needs to go toward a full resolution and that is either:
If the first point can’t be realized, then the second has to be done unless we don’t care about:
|
|
For me it's not about not caring about any of that, it's about tactical moves. Right now, there's no consensus on the shortcut and not enough cycles to explore the full selection of command palette for text selections. So I want to be pragmatic here. Merge or close the PR, I don't want to keep this open forever. |
For me this is fudging in lieu of fully addressing the issues. I see the pragmatism but also see it as a shame that it’s pragmatic because of either social/network ennui or priorities and not inherent requirements or limitations.
Your call since you’ve got the approvals and my concerns over e2e tests are addressed. Thanks for that and in general for being so responsive and considerate. |
I'm quite opposed to changing the shortcut of the command palette. |
I was thinking that adding a link would open a modal to engage with, not necessarily use the command palette UI explicitly. But otherwise, could there be a command for "Add Link" that essentially just triggers the link control wherever the cursor is? |
|
Ok, let's move forward here. This seems like the "only" programmatic approach for the moment. I'm happy to take a look at follow-ups and reconsider this. |
Right, so link creation would require another step. It’s not the worst thing, but I don’t see how it’d be justified. Anyway, it’s not something that we’d have to look at until one or more of the following points are important enough to address:
I think they’re important enough already and would have been non-issues by using a separate shortcut yet this probably isn’t place to advance this. |
…tion (WordPress#66056) Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: richtabor <richtabor@git.wordpress.org> Co-authored-by: stokesman <presstoke@git.wordpress.org> Co-authored-by: getdave <get_dave@git.wordpress.org>
Related #51737
What?
The cmd + k shortcut conflicts between links and command palette. So when you're in a RichText, it's not possible to trigger the command palette. That said, the link is only useful if you actually have a text selection.
So while this doesn't solve the problem 100%, I think it's a decent a reasonable fix for 99% of the cases.
Testing Instructions
1- Create a post.
2- Type some text.
3- Type cmd + k, it should trigger the command palette.
4- Select some text and then hit cmd+k, it should open the link format popup.