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

Make linking only work when you have selected text #5960

Closed
karmatosed opened this issue Apr 3, 2018 · 6 comments · Fixed by #6886
Closed

Make linking only work when you have selected text #5960

karmatosed opened this issue Apr 3, 2018 · 6 comments · Fixed by #6886
Labels
Needs Dev Ready for, and needs developer efforts [Type] Enhancement A suggestion for improvement.
Milestone

Comments

@karmatosed
Copy link
Member

Right now you can add a link on no text. This is an existing limitation in WordPress, the blocks makes this even more of an issue as more and more won't be interacting or correcting this via code.

2018-04-03 at 21 19

Whilst I understand this may be an issue with Tiny MCE, what about taking this a little further and only showing the link icon if you have selected text?

2018-04-03 at 21 12

@karmatosed karmatosed added the Needs Design Feedback Needs general design feedback. label Apr 3, 2018
@twsp
Copy link
Contributor

twsp commented Apr 4, 2018

Perhaps instead of only showing the link icon when text is selected, make it clickable only when text is selected. That way people know they have the ability to add a link, but can't accidentally add a link to a section of no text. Getting rid of it when nothing is selected could be confusing.

@karmatosed
Copy link
Member Author

@twsp I am ok with greying it out if there is enough contrast. I don't absolutely think it would be confusing to show contextual actions, we do that in a lot of places. I'm up for either option, the action of not having it working is the experience I'm focused on.

@noisysocks
Copy link
Member

Could we insert the link with the URL set as the link's text? This would align with what a lot of text editors do, e.g. TextEdit:

links

@karmatosed
Copy link
Member Author

Hmmm great idea! Let's have that as the option, it makes a lot of sense over the nothing entry. Thanks for the suggestion!

@danielbachhuber danielbachhuber added the [Type] Enhancement A suggestion for improvement. label Apr 10, 2018
@jasmussen
Copy link
Contributor

Perhaps in addition to this, there's an issue where the selection disappears when you enter the link modal:

new links

This is likely due to focus having to be there, and when you set focus elsewhere, the previous selection is unset. Maybe.

In any case, it looks like the classic editor solves this by immediately inserting a link placeholder, as soon as you create a link:

classic links

Bundle a fix for that one into this? :) 🍒

@noisysocks
Copy link
Member

@jasmussen: Yeah, that's something that was originally a part of #4572. I ran into a lot of pain (see #4909 (comment)) while implementing it though and so I gave up... for now. I'll create a seperate issue so that we remember to address this before merge 🙂

@karmatosed karmatosed added Needs Dev Ready for, and needs developer efforts and removed Needs Design Feedback Needs general design feedback. labels Apr 27, 2018
@karmatosed karmatosed added this to the Merge Proposal: Editor milestone Apr 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Dev Ready for, and needs developer efforts [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants