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

Typing over the link (replacing link text) #4762

Closed
Reinmar opened this issue Nov 30, 2016 · 9 comments · Fixed by #7590
Closed

Typing over the link (replacing link text) #4762

Reinmar opened this issue Nov 30, 2016 · 9 comments · Fixed by #7590
Assignees
Labels
package:link type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@Reinmar
Copy link
Member

Reinmar commented Nov 30, 2016

This was mentioned in #4720 but requires a separate ticket. Because of UX we need to allow replacing link text.

See w3c/editing#156 (comment).

See also – typing after a link: https://github.com/ckeditor/ckeditor5-link/issues/72.

@Reinmar
Copy link
Member Author

Reinmar commented Feb 16, 2018

My comments from #852:

The behaviour that link gets selected after it's created, theoretically, allows us to override link's text after we do ckeditor/ckeditor5-link#73. So, I think that in such a case it should stay.

Alternatively, we could do the same thing which GDocs does, so add a "link text" input to the link balloon. And you can notice that GDocs makes a collapsed selection at the end of the newly inserted link.

What I meant by "after we do ckeditor/ckeditor5-link#73" is that we should somehow force the editor in this case that first letter typed when link is fully selected gets this link's linkHref attribute value and, therefore, typing over a link will allow changing its text while keeping the link itself.

However, I don't know which solution is better for the user – a clear "link text" input or this behaviour. Or maybe both. WDYT, guys?

cc @oleq @dkonopka @jodator

@jodator
Copy link
Contributor

jodator commented Feb 16, 2018

So the current behavior is that if the selection is inside link (with some exception to a selection of a part of a link that starts in link start) will behave as described - it will allow to change link's text. Currently the behavior of selection of whole link behaves differently thus when you type the link disappears.

Typing over link start on link end different behavior:
screencast 2018-02-16 14_47_02

I'd ask a question what is my goal when I'd select a block of text with link (only this) and start typing? I think that in this case:

  • I would expect that del, backspace would remove link and text.
  • I would also expect that typing anything (maybe besides space) would meant that I want to change a link's text but preserve a link.

Also partial selection of a link should also work similarly to above - typing just changes the link text. To remove link (preserving a text) there's a dedicated UI (click + remove link). Also as with any links in editors it would be nice to enter a text after a link (there's already a ticket for this but just to mention it).

@Reinmar
Copy link
Member Author

Reinmar commented Feb 19, 2018

As I wrote in ckeditor/ckeditor5-highlight#6 (comment), I don't know why we went this way that typing over fully selected styled text removes the formatting.

In CKEditor 4 this works differently for bold and italic, in which case you still type with these styles.

With link... it's broken:

<p>a b <a href="">[c]</a> d</p>

+ "xxx"

<p>a b <a href="">x</a>xx[] d</p>

I think that I'd rather expect the same behaviour as with bold and italic in CKEditor 4. This seems to be more useful for all kinds of styles.

Most likely, the current behaviour comes from the fact that on a key press (non-safe key) we completely remove the currently selected content and insertion happens separately. We'd need to somehow store this information in the typing feature and apply it when the first character after deletion is inserted.

@mlewand mlewand transferred this issue from ckeditor/ckeditor5-link Oct 9, 2019
@mlewand mlewand added this to the backlog milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:improvement This issue reports a possible enhancement of an existing feature. package:link labels Oct 9, 2019
@jodator jodator modified the milestones: backlog, iteration 34 Jun 22, 2020
@pomek
Copy link
Member

pomek commented Jul 7, 2020

I've checked a few solutions that we have available on the market.

Typing over the entire link

The link is selected from left to right.

  • CKEditor 5: replaces the text without the link, ignores 2SCM
  • CKEditor 4: replaces the text without the link (except the first letter that is being linked)
  • Tiny: uses 2SCM and produces different results based on starting selection:
    • when starts before the link - the same as CKEditor 5
    • when starts at the beginning of the link - the same as CKEditor 4
  • Quil: the same as CKEditor 4
  • Google Docs: the same as CKEditor 5

Typing in the non collapsed selection in the link

All editors listed above do the same action: replaces the text and keeps the link.

Typing when the selection starts at the beginning of the link (ends in)

  • CKEditor 5: replaces the text without the link
  • CKEditor 4: replaces the text and keeps the link
  • Tiny: the same as CKEditor 4
  • Quil: the same as CKEditor 4
  • Google Docs: the same as CKEditor 4

Typing when the selection ends at the end of the link (starts somewhere in)

  • CKEditor 5: replaces the text and keeps the link
  • CKEditor 4: replaces the text without the link (except the first letter that is being linked)
  • Tiny: the same as CKEditor 5
  • Quil: the same as CKEditor 4
  • Google Docs: replaces the text without the link

My 5 cents

  • Typing over the entire link
    The link should be removed. The same happens with basic styles and we should be consistent across all features (we do it now)
  • Typing in the non collapsed selection in the link
    The inserted text should be linked. We edit a part of the link (we do it now)
  • Typing when the selection starts at the beginning of the link (ends in)
    Most probably a user wants to update the beginning of the link, we should insert the text as the link (we don't do it now)
  • Typing when the selection ends at the end of the link (starts somewhere in)
    The same as above (we do it now)

@Reinmar
Copy link
Member Author

Reinmar commented Jul 7, 2020

Typing when the selection starts at the beginning of the link (ends in)

  • CKEditor 5: replaces the text and keeps the link
  • Google Docs: the same as CKEditor 4

From what I can see, GDocs behave like CKE5 – you write without the link.

My 5 cents

So, AFAICS, there's just one behavior that perhaps we should change – the one with typing at the beginning on a non-collapsed selection. We could indeed align our behavior to that of CKE4, Tiny, Quill here. 

However, my question would be – why? How's this different from typing over the entire link? Why can I edit the beginning of a link or the end of the link but not the entire link text at once?

@jodator
Copy link
Contributor

jodator commented Jul 7, 2020

Typing over the entire link
The link should be removed. The same happens with basic styles and we should be consistent across all features (we do it now)

For the above behavior I'm not sure TBH, so another voice needed.

  • 👍 to keep consistency (remove link)
  • 👎 for PITA whe I'd like to replace link text but keep a link and probably we never get 100% agreement here what is a desired action. Selecting a text with link and typing over it is a bit different than selecting a link and using del/backspace. In other words having replace mechanism for typing wouldn't be a big PITA compared to not having it.

 The below are 👍 for me:

Typing in the non collapsed selection in the link
The inserted text should be linked. We edit a part of the link (we do it now)

Typing when the selection starts at the beginning of the link (ends in)
Most probably a user wants to update the beginning of the link, we should insert the text as the link (we don't do it now)

Typing when the selection ends at the end of the link (starts somewhere in)
The same as above (we do it now)

@Reinmar
Copy link
Member Author

Reinmar commented Jul 7, 2020

  • 👎 for PITA whe I'd like to replace link text but keep a link and probably we never get 100% agreement here what is a desired action. Selecting a text with link and typing over it is a bit different than selecting a link and using del/backspace. In other words having replace mechanism for typing wouldn't be a big PITA compared to not having it.

Same thoughts here. I'm also considering being inconsistent with other editors. And I also see a difference between typing over a link and pressing backspace/delete. In the latter case, I think that it's more natural to remove the link. In the former, I think we could risk keeping the link.

Let's maybe discuss this tomorrow on the sync too.

@neongreen
Copy link

For me, not being able to replace the link text by selecting it and typing over is indeed a PITA. I resort to tricks like "select everything but the first and the last character, type what I want, remove the first and the last character".

I'd much prefer "keep the link but replace the link text when typing over; delete the link entirely on backspace".

@jodator
Copy link
Contributor

jodator commented Jul 8, 2020

Notes from internal meeting:

  • we agree we should have replace link text when whole link is selected and keep the link 👍
  • link only for safety
  • how to implement?
    • pasting - no
    • insertContent - check constrains and then override 👍 - start here and check if doable
    • command (input) - override behavior (first letter only probably) 
    • selection attributes - non empty selection with link - set selection attributes for link - check why selection attributes is not inherited on type
  • only one link (not two links selected)

jodator added a commit that referenced this issue Jul 15, 2020
Feature (link): Typing over the selected link will not remove the link itself. Instead, the typed text will replace the link text. Closes #4762.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:link type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants