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

Markers #3856

Closed
pjasiun opened this issue Oct 20, 2016 · 2 comments · Fixed by ckeditor/ckeditor5-engine#700
Closed

Markers #3856

pjasiun opened this issue Oct 20, 2016 · 2 comments · Fixed by ckeditor/ckeditor5-engine#700
Assignees
Labels
package:engine type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Milestone

Comments

@pjasiun
Copy link

pjasiun commented Oct 20, 2016

It seems to be the last big piece of this puzzle.

What is the problem?

There are some fancy features, changes on the document one may want to apply which are not regular model attributes:

  • showing selections of other users in collaborative editing,
  • marking comments (similar to google docs),
  • showing find results (Chome style),
  • underline misspelled words.

Let's call them "markers".

Markers vs attributes

What is the main difference between markers and attributes (like bold or link)?

Attributes are assigned to letters. It does not matter if one broke paragraph or moved part of the text: bolded letters should still be bolded. Markers, on the other hand, need to be consistent, part of the marker should not be moved in the different part of the document.

Moreover, when an attribute is applied it does not matter what will happen with it later. Finding all texts with a certain attribute is an edge case and can be slow. At the same time, for markers, we need to be able to observe changes on then and react on these changes. Finding marker in the document should be very quick and should not require whole document scanning.

Implementation

It looks that LiveRanges and LivePositions will fit perfectly here. Document should have a collection of named markers, which are pairs of the name and LiveRange/LivePosition:

document.markers.set( 'findResult', new LiveRange( ... ) );

document.markers should fire events whenever a marker is set, changed or removed.

Then, it should be possible to easily create converters from markers to view attributes, for instance:

converterBuilder.for( dipatcher ).fromMarker( 'findResult' ).to( 'span', { color: 'yellow' ) );
converterBuilder.for( dipatcher ).fromCollapsedMarker( 'extraSelection' ).to( 'span', { color: 'red' ) );

These converters should handle inserting, removing and moving content from the marked fragment. They should also handle updating marker (creating a new marker with the same name).

Additional requirements

Should work with OT

It should be possible to transform markers the same way operations are transformed. When the marker is created on one document it should be possible to recreate it on the different state of the document. Then marker should keep the synchronization with the document (it should happen automatically thanks to LiveRanges).

Groups of markers

It should be possible to create markers names using namespaces: (selection:john, selection:marry, ...) and listen to the generic event (markerCreate:selection).

Markers should not break text nodes

This is not an obligated requirement.

Because markers do not have a semantic information about the content, it would be nice if they would not break text nodes in the DOM and not break the native spell checking.

@scofalik
Copy link
Contributor

scofalik commented Oct 20, 2016

This fits what we have discussed, so, generally, 👍 from me.

I wonder how difficult it will be to find and remove "old" view element created by marker. Let's take your selections example. If I have a marker at position A and it is changed to position B, I have to remove <span> from view. I have to find that <span> somehow. Other than that, view elements created by markers should not mess up with model<->view bindings that operate on lengths. But this probably should not be a problem.

Markers should not break text nodes

This however will be very difficult to implement, and, really, it's converter's job to convert marker properly to view. I would not bother with this requirement.

@pjasiun
Copy link
Author

pjasiun commented Oct 20, 2016

I wonder how difficult it will be to find and remove "old" view element created by marker. Let's take your selections example. If I have a marker at position A and it is changed to position B, I have to remove from view. I have to find that somehow. Other than that, view elements created by markers should not mess up with model<->view bindings that operate on lengths. But this probably should not be a problem.

We may consider some view writer improvements to handle it. On the other hand, unwrap might be enough if we know where the old range was.

This however will be very difficult to implement, and, really, it's converter's job to convert marker properly to view. I would not bother with this requirement.

Yea, this is why I started with "This is not an obligated requirement.". It's nice-to-have, but I do not believe we will be able to implement it.

@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the iteration 6 milestone Oct 9, 2019
@mlewand mlewand added type:feature This issue reports a feature request (an idea for a new functionality or a missing option). package:engine labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:engine type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants