Ckeditor5: Document colors section for font/background color dropdowns

[mlewand]

Is this a bug report or feature request?

🆕 Feature request

💻 Version of CKEditor

CKEditor v5 @ 12.1.0

📋 Steps to reproduce

The document colors section of a dropdown should pick up colors present in the editor contents at a time of opening the dialog.

This means that it will simply contain unique colors used throughout the document. The colors should appear in the same order as in the document (starting from top). Also the colors should be unique within the sections (they can duplicate colors present in the generic dropdown).

Config

There should be a config property allowing to customize the number of items in document colors section or allowing to disable it by setting it's value to 0. FontColorConfig#documentColors will do.

ClassicEditor
    .create( {
        fontColor: {
            documentColors : 0 // Disable document colors section.
        }
    } )

📃 Other details that might be useful

We initially wanted to add this feature in 12.1.0 but ended up in extracting it to this dedicated issue and pushing it back for future release.

Edit: Down the discussion you might see references to "recent colors" features, which is a different features that we considered - but ultimately we proceeded with document colors feature.

[mlewand]

I wanted to report paste handling as a separate ticket, but this would actually be handled by this particular ticket.

[mlewand]

One important aspect to think about here is collaboration.

Should colors used by other users be propagated to local user's dropdown?

The color is applied to the document, so logically it should be "consumed" and displayed by the recent colors. I'm only afraid that it could lead to the situation where local user will get frustrated because he or she will lose recent colors as other users paste/use different colors.

Please share your opinions.

[scofalik]

If you are interested in colors that were in the editor after the data was loaded, you could set a listener on Editor#event:ready (or maybe .once() on model.Document#event:change but I am not sure that would work ;)).

In that listener, you can simply scan the whole model (make a range on root and get through .getItems()) and check all returned text proxies if they have an attribute.

You could do that every time dropdown is opened to get the exact data all the time but this may be not effective in case of long documents.

If you want to dynamically update the list (pasting, collaboration) you would have to listen to model.Document#event:change and use editor.model.document.differ.getChanges() to get the changes. Then scan insertions (deeply) and attribute changes to add new colors to "recent colors" section.

The one problem with this solution is that it is more complicated to dynamically remove colors because you would have to count the occurrences of each color. You can check that a text with given color was removed but you don't know if that was the last occurrence of a text with that color in the document. @oleq gave a good example of someone pasting a text with multiple colors which would bloat/overwrite recent color section and then executing undo.

A nice thing about having this working with collaboration is that you and other collaborators won't start coloring stuff with similar but different colors.

[Reinmar]

If you are interested in colors that were in the editor after the data was loaded, you could set a listener on Editor#event:ready (or maybe .once() on model.Document#event:change but I am not sure that would work ;)).

We're interested in the colors which are in the document when the panel is being opened. So it's a no-brainer.

[Reinmar]

Quoting myself from https://github.com/ckeditor/ckeditor5-font/issues/32

There are 2 colors used in the document so they should be visible in the "document colors" section in each panel.

Currently, on the colors which I applied during the current editing session will appear there. Whcih means that I won't be able to reuse colors which were pasted or which someone else applied or which I applied in the previous session.

[mlewand]

There's couple of design decisions that we need to clarify before moving forward.

One thing that has been brought up recently is the way how exactly the recent colors are picked. There are two strategies:

• Currently the colors are picked up continuously as content is modified. This means that recent colors are added to the set in order they were used, but it affects performance as it requires computing on each diff. https://github.com/ckeditor/ckeditor5-font/issues/28#issuecomment-478051755
• Other option would be to do "lazy load" of recent colors, so that the colors are only loaded when the color dropdown is open. This means that the colors are displayed in order of the document. (https://github.com/ckeditor/ckeditor5-font/issues/28#issuecomment-478922387)

Please share your opinions here or on a F2F meeting.

Other matters to consider:

• Recent colors behavior with collaboration? Should colors used by other users also affect recent colors set?
• Should the amount of recently used colors be configurable (including opting out the feature)?

[mlewand]

My take on strategy is that the approach of loading colors upon each dropdown open provides very little use for the end user. Assuming you have used multiple colors in the document, and you're doing something at the end of the document, it's possible that you won't see your color at all.

Having the colors added in order that they _actually_ were used seems logical to me. If the performance is the problem I think we should do a research to determine how could we adjust/provide the API to make it in a performance-friendly way. For sure similar cases will occur in the future.

[scofalik]

Recent colors behavior with collaboration? Should colors used by other users also affect recent colors set?

I personally think this is a very good UX. No problem with reusing "that exact gray" the other person used. Depending on the solution this will be either very difficult or "free".

Should the amount of recently used colors be configurable (including opting out the feature)?

Why not? We could set some default limit and have a config option to change it.

It's possible that you won't see your color at all.

With limiting, yes, I agree, good point.

[jodator]

I see this boils down to a recent vs document colors.

For me recent colors are colors that I've chosen from the Dropdown. So:

• only my colors are there as I use the app
• might be empty on first load.

The other cases described are document colors:

• picking up colors from document
• collaboration colors (picked up from document).

So I think that either recent colors do too much or are named inaccurately...

Opting-out from this could be added.

I personally think this is a very good UX. No problem with reusing "that exact gray" the other person used.

I'd see format painter to be used there.

[oleq]

One thing that has been brought up recently is the way how exactly the recent colors are picked. There are two strategies:

• Currently the colors are picked up continuously as content is modified. This means that recent colors are added to the set in order they were used, but it affects performance as it requires computing on each diff. #28 (comment)
• Other option would be to do "lazy load" of recent colors, so that the colors are only loaded when the color dropdown is open. This means that the colors are displayed in order of the document. (#28 (comment))

A hybrid idea: Can we load document colors once (let's say when drop–down first shows up) and then push user (recent) colors up–front as they use the dropdown to colorize the content?

If the performance is an issue, let's stick to the scenario in which only direct usage of the color in the dropdown adds to the list.

• Recent colors behavior with collaboration? Should colors used by other users also affect recent colors set?

It makes sense to me. But again, if this is a performance issue, we do not need that so badly to sacrifice the UX.

• Should the amount of recently used colors be configurable (including opting out the feature)?

👍

[Reinmar]

• Other option would be to do "lazy load" of recent colors, so that the colors are only loaded when the color dropdown is open. This means that the colors are displayed in order of the document.

I'm for this solution (and calling it "document colors") because:

• it's simple (cheap to code)
• it's cheap performance-wise
• it works with collaboration (you can see all peoples' colors)
• it works on fresh editor load automatically too
• unless you have 5+ different colors in the document, you won't even see a difference
• we can change it in the future if we get feedback that it doesn't work

[mlewand]

I can see that the feature is going to be cheaper in terms of development. But I don't think that users will find this useful/predictable.

Anyway, we need to go with _something_ and since the other option would take us a much more time to ship then be it, I'll have to 🙈 on this one. Let's continue with _document colors_ feature.

I have updated the issue summary.

UX

BTW should we put a label before this colors section? cc @oleq @dkonopka

Config

I was thinking of a simple config name for document colors feature and it should be as simple as: documentColors.

We shouldn't go with documentColorsCount as it's even longer and prevents us from extending it down the road.

Extensibility-wise though we'll use numbers today, there's nothing holding us back to make it an object in future if there will ever be a need to, say:

ClassicEditor
    .create( {
        fontColor: {
            documentColors: {
                label: false,
                count: 12,
                isVisibble: () => this.colors.count() >= 3
            }
        },
        fontBackgroundColor: {
            documentColors: 0 // Disable document colors section.
        }
    } )

[oleq]

BTW should we put a label before this colors section? cc @oleq @dkonopka

I'd need to see it in the mockups to tell. Probably yes.

[mlewand]

I'd need to see it in the mockups to tell. Probably yes.

@dkonopka can I ask you to put a small mockup for this one?

[dkonopka]

IMHO we should definitely use a label here (I've already pushed it to the t/28 branch).

[mlewand]

Thanks for the design @dkonopka. I wonder whether the spacing might be changed so that there's less margin below the "Document colors" label (top margin looks fine). It would reduce the height and make "Document colors" appear to be visually more related to the colors section below.

[dkonopka]

@mlewand What is interesting, there is no bottom/top padding for this label, it's just all based on line-height of this div. Changing this value looks for me like a little CSS hack, but it's not a no-go.

Changing line-height of label adding padding-top

.ck.ck-color-grid__label {
  /* We need to overwrite line-height calulated via `.ck-reset`. */
  line-height: 1em;

  padding: var(--ck-spacing-standard) var(--ck-spacing-standard) 0;
}

Current state

@oleq WDYT?

[msamsel]

FYI: @Reinmar, @mlewand
Approach with document colors can also have some flaws.
When document won't contain colors, then we will have to iterate over entire model.

[mlewand]

When document won't contain colors, then we will have to iterate over entire model.

@msamsel searching entire model for a given attribute should not be such an inefficient operation. So I don't think we should be concerned about this case.

Thanks for refreshed designs @dkonopka. Since @oleq is off for couple of days. @msamsel please proceed with initial proposal. We'll resolve the alignment later on.

[msamsel]

There appeared bad behaviour of UI, where currently selected color had double selection in dropdown panel. This happens, when user use default color in text, so both color grids had selected the same color.

After consulting with @mlewand & @dkonopka, we decide to remain selection only in Document Colors, if this feature is available.

[mlewand]

This feature recently was merged to the master branch 🎉

The only thing you need to do is enable the Font plugin, as it's document colors section is enabled by default there.

More info will be available in the docs.

Sample screencast: