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

Consider implementing fake visual selection in editable #4721

Closed
oleq opened this issue Aug 24, 2016 · 17 comments · Fixed by #7436
Closed

Consider implementing fake visual selection in editable #4721

oleq opened this issue Aug 24, 2016 · 17 comments · Fixed by #7436
Assignees
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:link status:discussion type:question This issue asks a question (how to...).

Comments

@oleq
Copy link
Member

oleq commented Aug 24, 2016

Related issue https://github.com/ckeditor/ckeditor5-link/issues/2.


At this moment, if some content is selected in editable

image

and the user clicks "Link", which opens the panel and focuses an input inside, the original selection is visually lost in editable

image

which can be considered an UX problem.

  • Is is possible to implement something similar to what Find dialog does in v4? (fake selection using <span>)
  • Would that be safe an easy to maintain? It would reside in editing view layer only, not in the document tree model, I guess.
  • Can we react fast enough to changes in DOM like selection change using mouse or keyboard, command execution, etc. to get rid of fake selection and replace it with a real one so the features never get confused because of it?
  • Most importantly, is it worth our effort? None of the editors in the market seems to bother. Well, maybe except Google Docs, but they always render selection using <span> in their custom editing environment, which is something completely different.

Just FYI, it works in classic editor in v4, because editable is enclosed in an iframe, which is technically a separate window and a separate focus context.

@scofalik
Copy link
Contributor

scofalik commented Aug 24, 2016

I'd ask from different side: do we lose something by introducing multiple <iframe>s to handle multiple selections? I think that having real selection is something that we should strive for. <span>s will probably not look and behave the same.

@oleq
Copy link
Member Author

oleq commented Aug 24, 2016

do we lose something by introducing multiple <iframe>s to handle multiple selections?

Yes. A looooot. Iframes were the source of so much evil in v4 and thus we decided that by deisgn v5 is to be as iframe–less as possible. The evil included asynchronousness in DOM selection, DOM focus, cumbersome event propagation, problems with native types like Node or HTMLElement (because the don't equal in global and iframe windows), problems with editor bootstraping and setting data, which also becomes async, and many, maaaaany more. Madness.

So introducing framed editing just for the sake of minor visual annoyance is a no–go. We're faking selection or ignoring the issue.

@Reinmar
Copy link
Member

Reinmar commented Aug 24, 2016

Just FYI, it works in classic editor in v4, because editable is enclosed in an iframe, which is technically a separate window and a separate focus context.

And it doesn't work in inline editor, but no one sees that because we use dialogs which cover the whole view. So this raises this issue's priority in comparison to CKEditor 4.

Is is possible to implement something similar to what Find dialog does in v4? (fake selection using )

Yes, it is. In fact, it should be very (or reasonably) simple. Once editable loses focus we just need to convert the selection to a span element. On focus, the operation should be reverted.

Can we react fast enough to changes in DOM like selection change using mouse or keyboard, command execution, etc. to get rid of fake selection and replace it with a real one so the features never get confused because of it?

Features should only read selection from the model. They also need to be aware that they can only access DOM selection if editable has focus. If not, they should access the selection span(s) to which selection will be converted. We can partially hide this under the view abstraction, but I don't think this will make it easier to understand and work with.

Most importantly, though, we need to fake selection only if there's no focus in the editable, so we don't have to react fast to anything.

Most importantly, is it worth our effort? None of the editors in the market seems to bother. Well, maybe except Google Docs, but they always render selection using in their custom editing environment, which is something completely different.

I don't think that it's worth for iteration 3, but I think that it's a must-have for 1.0.0. I actually thought about "1.0 candidate" label so we can keep track of such issues.

@Reinmar
Copy link
Member

Reinmar commented Aug 24, 2016

I'd ask from different side: do we lose something by introducing multiple <iframe>s to handle multiple selections? I think that having real selection is something that we should strive for. <span>s will probably not look and behave the same.

Adding to what Olek wrote.

We partially support iframe-based editables in CKEditor 5 already, because @pjasiun was aware of them when developing parts of the engine responsible for rendering. However, we've never tested them. We'll need to propose editors which use iframed editables at some point because there are use cases in which they should be used.

The main use case for iframed editables are scoped styles. You can inject editor anywhere in your CMS or website and keep full control over the content styling. However, being biggest advantage its also the biggest no-go for many, simply because they want to integrate the editor in-place.

Additionally, iframes are also heavier. They require more code and load slower. E.g. beside issues which Olek mentioned, you have problems like autoresizing. Div changes its height when you put more content into it. Iframe does not and you need complicated solutions to workaround this.

@fredck
Copy link
Contributor

fredck commented Aug 25, 2016

We must be sure that this will not end up in the data model. This should be a "converter" feature of selection itself. Basically, if it is not possible to render a native selection (e.g. the editable doesn't have focus), the selection should be faked. Such feature could be then extended by features for faking "object" selection, like images, tables, etc.

@pjasiun
Copy link

pjasiun commented Aug 25, 2016

Note that such feature is needed for collaborative editing anyway, to show other users selection. I was already thinking about it, and displaying it is not a problem, we can add to view whatever we want, during the conversion. Handling it later is more tricky (see https://github.com/ckeditor/ckeditor5-engine/issues/484), because future changes will know nothing about that fake selection, we need to remove it as soon as possible, what, in this case, also should not be a big issue.

@oleq
Copy link
Member Author

oleq commented Aug 26, 2016

Features should only read selection from the model.

@Reinmar: @pjasiun told me exactly the opposite when I consulted him a couple of days ago about the mechanics of pinning a ballon panel to an element/selection in DOM. AFAIR he recommended observing editing view render event (because of some arcane stuff) and checking state of native DOM to decide where to pin it, which could be distorted by such fake selection span element.

So either I'm not getting it right or there are problems waiting for us in the future.

@pjasiun
Copy link

pjasiun commented Aug 26, 2016

@pjasiun told me exactly the opposite when I consulted him a couple of days ago about the mechanics of pinning a ballon panel to an element/selection in DOM. AFAIR he recommended observing editing view render event (because of some arcane stuff) and checking state of native DOM to decide where to pin it, which could be distorted by such fake selection span element.

Not native DOM, but the view (virtual DOM), but the rest is correct.

@pjasiun
Copy link

pjasiun commented Aug 26, 2016

My point was to make all of the balloon feature working on the view level. It should know nothing about the model.

@fredck
Copy link
Contributor

fredck commented Aug 26, 2016

Er... If the ballon depends exclusively on the selection, I expect that the model will at least tell me if my interested element is selected. Then, when it comes to positioning, styling, etc., then for sure the view is the place to look at, right?

@Reinmar
Copy link
Member

Reinmar commented Aug 26, 2016

Er... If the ballon depends exclusively on the selection, I expect that the model will at least tell me if my interested element is selected. Then, when it comes to positioning, styling, etc., then for sure the view is the place to look at, right?

+1, that's how I'd imagine it. I use the model to understand when to pop up and for which feature and then the view to decide where to render itself visually.

@Reinmar
Copy link
Member

Reinmar commented Jun 22, 2017

Ticket in the engine: https://github.com/ckeditor/ckeditor5-engine/issues/976. If we agree to use a marker for this it may turn out that we're able to implement this quite easily.

@Reinmar
Copy link
Member

Reinmar commented Mar 11, 2018

There's a funny side effect of link highlighting:

image

When you edit an existing link, you see precisely what you edit.

Unfortunately, that still doesn't happen when you create a new link.

@mlewand mlewand transferred this issue from ckeditor/ckeditor5-link Oct 9, 2019
@mlewand mlewand added this to the nice-to-have milestone Oct 9, 2019
@mlewand mlewand added domain:ui/ux This issue reports a problem related to UI or UX. module:ux status:discussion type:question This issue asks a question (how to...). package:link labels Oct 9, 2019
@Reinmar Reinmar removed the module:ux label Jan 15, 2020
@jodator jodator modified the milestones: nice-to-have, iteration 33 May 27, 2020
@jodator jodator self-assigned this Jun 15, 2020
@jodator
Copy link
Contributor

jodator commented Jun 15, 2020

The first implementation seems to do the trick:

Selection on text:
Collapsed selection

I've used a marker for that (the colors are to be tuned later) and while doing it I've came to a conclusion that adding a visual representation for a collapsed selection might be also nice.

cc @oleq , @panr WDYT?

@panr
Copy link
Contributor

panr commented Jun 15, 2020

Looks good 👌 Just wonder whether we should add an info about the collapsed selection - that the entered URL will extend selected text with its value (example.com) 🤔 (probably not a scope of the ticket).

@jodator
Copy link
Contributor

jodator commented Jun 15, 2020

From sync:

  • use the CSS value for selection color
  • don't move the text
  • use CSS value also for collapsed selection

@oleq
Copy link
Member Author

oleq commented Jun 15, 2020

For the collapsed caret you can use a pulsing animation like in the type around feature

animation: ck-widget-type-around-fake-caret-pulse linear 1s infinite normal forwards;

oleq added a commit that referenced this issue Jun 16, 2020
Feature (link): A fake caret (selection) should be displayed in the content when the link input has focus and the browser does not render the native caret (selection). Closes #4721.

Feature (theme-lark): Added styles for the fake link caret (selection) (see #4721).
Aeness added a commit to Aeness/ckeditor5-insert-image that referenced this issue Apr 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:link status:discussion type:question This issue asks a question (how to...).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants