-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Comments
I'd ask from different side: do we lose something by introducing multiple |
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. |
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.
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.
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.
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. |
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. |
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. |
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. |
@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 So either I'm not getting it right or there are problems waiting for us in the future. |
Not native DOM, but the view (virtual DOM), but the rest is correct. |
My point was to make all of the balloon feature working on the view level. It should know nothing about the model. |
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. |
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. |
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). |
From sync:
|
For the collapsed caret you can use a pulsing animation like in the type around feature ckeditor5/packages/ckeditor5-theme-lark/theme/ckeditor5-widget/widgettypearound.css Line 132 in 5948bcf
|
…yed. As asked for link in ckeditor/ckeditor5#4721.
Related issue https://github.com/ckeditor/ckeditor5-link/issues/2.
At this moment, if some content is selected in editable
and the user clicks "Link", which opens the panel and focuses an input inside, the original selection is visually lost in editable
which can be considered an UX problem.
<span>
)<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.
The text was updated successfully, but these errors were encountered: