Ckeditor5: Implement the Highlight feature

[jodator]

This issue is a follow up of talks that have taken:

• https://github.com/ckeditor/ckeditor5/issues/631
• https://github.com/ckeditor/ckeditor5/issues/640
• https://github.com/ckeditor/editor-recommendations/issues/63

The feature should be developed in the dependency injection–friendly way (see example):

Model and data

1. The feature should output <mark> element with optional class.
2. The feature should work similar as inline tags in basic-styles (ie bold) - it should add highlight attribute to text nodes.
3. Markers should not overlap - at least in MVP.
4. There will be two types of highlighters:
   
   
   
   1. "marker" - which will change background color of highlighted text,
   
   
   2. "pen" - which will change foreground color of highlighted text.
   
   
5. There should be configuration for the users to define marker styles.
   js ClassicEditor .create( { highlight: { markers: [ { class: 'marker-important', title: 'Important', color: '#' } ], pens: [] } } ) .then( ... ) .catch( ... );

UI

1. The feature has one "smart" button.
2. Icon changes to either pen or marker, depending on my last use.
3. My last used color is displayed in the icon as well.
4. If I click in the icon side of the button, the selection is highlighted as the icon shows.
5. If I click on the button arrow, it opens the options where we show larger beautiful boxes. For pens it shows a pen-like line with that color and a letter or two styled that way. For markers, it shows a marker-like stroke in the color.
6. one of the tools in button must be a highlight eraser (rubber).

It's a TODO because ATM there's no such possibility. Things to consider:

1. A new UI component (dropdown) that groups other components is needed.

---

Things to consider:

• how to define configuration for Hightlight fetaure - most important the visual style. I've proposed two sets: markers and pens to have them separated. I'm not sure if passing a color variable (besides class name to apply) ie for styling a button, is a good way of doing it as the content CSS must be aligned with highlight representation in editor.

[fredck]

The feature should output element with optional class.

I don't think we can make this optional. It would not make sense, because the class name is anyway required (see bellow).

{ class: 'marker-important', title: 'Important', color: '#' }

Just as a note to remember... the highlight command should expect the class name of the marker to be applied. The title cannot be used because it must be localizable.

I'm not sure if passing a color variable

Hum... the only other way I see is sniffing the CSS to retrieve the color from it. It is a much nicer way, but is it worth the additional code?

I've proposed two sets: markers and pens to have them separated.

What about making it a configuration property instead, just to make it look simpler?

Btw, as we're in the crazy CSS sniffing thing, we could even sniff this property as well :)

[fredck]

The engine implementation of this feature seems to be very simple. The complex part is the UI. Here we can even thinking about separate milestones, if you wish.

[jodator]

I don't think we can make this optional.

I mean in minimalistic configuration it would be just <mark>Some imporant text</mark> in data. It could be assumed that default title is "Highlight". So one could define:

ClassicEditor
    .create( {
        highlight: {
            markers: [ { class: '' }] // I can already see what's not nice here
        }
    } );

so it would have only one, default marker - maybe it not make sense but that was initial idea.

Btw, as we're in the crazy CSS sniffing thing, we could even sniff this property as well :)

I don't see from where we could sniff that option. It would require some contentCSS to be present to sniff that from somewhere. At this point I don't know if we have such feature in CKE 5. I only checked configuration options. Maybe in MVP we should go with proposed configuration?

The complex part is the UI

Yep, I'd go with engine part in this ticket and the UI in follow up.

What about making it a configuration property instead, just to make it look simpler?

So in other words configuration should be:

const configA = {
    highlight: [
        { class: 'marker-important', title: 'Important', color: '#555', type: 'marker' },
        { class: 'pen-typo', title: 'Typo', color: '#555', type: 'pen' }
    ]
};
// OR
const configB = {
    highlight: [
        { class: 'marker-important', title: 'Important', background: '#555' },
        { class: 'pen-typo', title: 'Typo', foreground: '#555' }
    ]
};

[szymonkups]

The feature should work similar as inline tags in basic-styles (ie bold) - it should add highlight attribute to text nodes.

Shouldn't this feature use model markers and highlighting it in the view as @scofalik mentioned it in https://github.com/ckeditor/ckeditor5/issues/631#issuecomment-339631589?

[fredck]

Shouldn't this feature use model markers and highlighting it in the view as @scofalik mentioned it in ckeditor/ckeditor5#631 (comment)?

No, the highlighted text will have the markup inlined in the output HTML anyway, so why the complication? Just because of the feature name (markers) :)

[jodator]

@szymonkups & @scofalik That is a good question for you guys actually as you have more experience with engine. The output of Highlight feature will be:

<p>
    This is some text <mark class="marker-important">that is cool</mark>.
</p>

Give above it looks for me as regular <span> like element. Now what's best to use on model?

[fredck]

@szymonkups & @scofalik please don't overcomplicate this features... it is exactly like the bold feature, so please give a nice answer to @jodator that will not make him crazy :)

[fredck]

maybe it not make sense but that was initial idea

I would KISS in the first version. This can be brought later if we'll ever need to support it.

[fredck]

I don't see from where we could sniff that option.

Well, we would have to be creative... like creating a hidden element with the classes applied and checking their styles or anything like that.

Maybe in MVP we should go with proposed configuration?

Yes, ofc... I was just opening the possibility if we want to make it more complex. But this is the kind of thing that can land later if we want.

[szymonkups]

it is exactly like the bold feature, so please give a nice answer to @jodator that will not make him crazy :)

😄

If we treat this feature the same as for example Bold feature then this is fine. Model markers and highlighting mechanism could be used when there will be a need for overlapping or image highlighting.

I brought this up just to be sure that we are purposefully want to implement this without using model markers. 👌

[jodator]

Well, we would have to be creative... like creating a hidden element

What I've meant was that we do not have external (content) CSS stylesheet to apply those classes. How to sniff is rather simple and doable. As for now we lack that contentCSS that would provide CSS that is used to style content.

[scofalik]

It depends on what you want to achieve.

Markers are ranges. If you have, for example:

Lorem ipsum dolor sit amet

and paste "xyz" inside "ipsum" you will get...

With marker:

Lorem ipsxyzum dolor sit amet

Without marker:

Lorem ipsxyzum dolor sit amet

Same, if you cut & paste ipsum, you will get...

With marker:

Lorem dolor sit ipsum amet

Without marker:

Lorem dolor sit ipsum amet

BTW. It's easier to operate on markers if you want to, for example, remove the whole style, or remove all styles at given position. It's also easier to find all occurrences of given style.

BTW2. I still hate the fact that there will be "invisible" spans in the final output and that somebody may have see them in the source.

[pjasiun]

If we use, model markers it will be always one consistent range. Whatever happens, it will not split. If we use attributes it will be attached to text, like bold. It will be possible to move part of the highlighted text.

In this case, I believe, it should work like bold feature, as @fredck mentioned to make it simpler. Markers are designed for different cases, where you need to be able to find or change marked fragment easily. They are more CPU-consuming since they need to recalculate with each change, and does not work well with undo. They are designed to work well with external logic which will handle them, like comments or external users selection. They are not created to just mark text, for such case attributes should be used.