There should be only one widget toolbar

[jodator]

Right now widget toolbar allows multiple toolbars to be "shown" at once. It goes through all registerd toolbars and shows each one that returns true for visibleWhen check. After enabling images in tables we allow widget inception but checks might be simplistic.

As an example: table & image

• table toolbar for content (when selection is in table cell) checks if selection is inside table
• image checks if selection is on image
• if there is selection on image that is inside table both condition are true

This can be check by table feature easily (I'm adding additional check if other widget is not selected already) but I think that we might think about another approach for such scenarios - ie electing only one toolbar to be shown (first/last registered, priorities?).

[oleq]

• Why there must be a single toolbar only?
• What does it look like ATM? (screenshot please)
• What cases fail at first glance because of multiple toolbars ATM? (screenshots please)

[jodator]

@oleq Screencast will be better:

When I click on image both toolbars returns true for visibleWhen check.

Edit: The behavior is such that one of them "wins" - here table toolbar. The image toolbar is shown on editor blur.
Edit 2: Also the position of the one that "wins" (table) is wrong - taken from image toolbar.

[oleq]

This is similar to #852 but the reason is different. Anyway, do you think we could display the innermost toolbar only at this moment (the one attached to the element deepest in the tree)?

[jodator]

@oleq Well I think that it should work this way but currently there is no information back to the widget toolbar logic that indicates to which element the toolbar attaches. It is only true/false for current view selection returned from visibleWhen() callback. Thus when 2 or more toolbars return true the race condition will occur.

Right now I see that the table toolbar could have the logic updated as table feature might expect that toolbar could be shown in certain scenarios. But this will cause other problems - like when image toolbar is disabled and table feature could show a toolbar when image toolbar is not available.

---

Proposition

We could add some additional mechanism to elect the best toolbar to be shown besides returning true/false on visibleWhen() callback:

https://github.com/ckeditor/ckeditor5-widget/blob/031e37014487dd4359d4972e23c38a5c59d8a78b/src/widgettoolbarrepository.js#L138-L143

The first thing I can think of is returning the view element rather then true. Then the widget toolbar can choose which toolbar show from all truthy values.

Let's say we have two toolbars (image and table) and selection is on image in table cell.

Current behavior:

• table toolbar returns true
• image toolbar returns true
• race condition as both toolbar will be run and displayed

Proposed:

• table toolbar returns td < tr < table ( < - means child of)
• image toolbar returns image < td < tr < table
• widget toolbar can elect image as it is deeper in the view tree - no reace condition nor double toolbar logic run

[oleq]

@jodator I came back to this topic and I noticed that you fixed the issue in ckeditor/ckeditor5-table@56adacf (at least for tables and images) but it looks like a hack (checking if some widget is selected inside) and not very generic at all. I'll try to propose something better.

[jodator]

@oleq yep, I had to do something there as we do not have a generic solution :(