Ckeditor5: Introduce a toolbar for the selected table widget

[ma2ciek]

The current toolbar is dedicated for table content. Toolbar for the whole widget is missing and it could be used for comments or other block-related things.

But the question is how the new toolbar should be named and if the current toolbar should be renamed?

cc @Reinmar @pjasiun @jodator

[jodator]

Huh.. to be consistent other block-level toolbars configs are on feature.toolbar namespace and are named FeatureToolbar. AFAICS table would be the first toolbar with hybrid toolbar _(visually)_.

From the user perspective the toolbar will be on the same spot (am I right?) - so its contents will (_visually_) change.

Taking the above maybe we just need to defined contexts for the same toolbar? Or it is easier/cleaner to use two different toolbars?

The configuration _could_ look like this then:

table: {
    toolbar: {
        widget: [ 'comments' ]
        content: [ 'tableColumn', 'tableRow' ]
    }
}

But I'm not sure if I like it, though :/

The second option would be to have 2 toolbars:

// A: backwards-compatible
table: {
    toolbar: [ 'tableRow', ... ] 
    widgetToolbar: [ 'comments' ]
}

// B: semantically correct (-ish)
table: {
    contentToolbar: [ 'tableRow', ... ] 
    widgetToolbar: [ 'comments' ]
}

then the name should be derived from the config we choose IMHO.

[ma2ciek]

I was thinking about an option with 2 toolbars in the table option to not complicate it. But the question is whether it should be backward compatible or not as you mentioned.

Do we plan to create a deprecated() util so we can support e.g.

table: {
    toolbar: [ 'tableRow', ... ],
    contentToolbar: [ 'tableRow', ... ] 
    widgetToolbar: [ 'comments' ]
}

mark toolbar as deprecated and drop its support in the future?

[Reinmar]

Oopsie. Pity we have missed that previously :(

I'm not sure yet which option is best, but we can deprecate the current table.toolbar option and remove it in the next major release if that's what we want. That would be ok.

[pjasiun]

I can not find anything what I really like, but I think that this could be fine:

table: {
    tableToolbar: [ ... ],
    cellToolbar: [ ... ]
}

I would not do a toolbar namespace, because we need to keep toolbar backward compatible and because of reasons mentioned here.

I know that table.tableToolbar is unfortunate, but I think it is better than just table.toolbar. It says clearly that this is the whole table level toolbar.

I do not like using widget in the configuration option, because of the fact that table is a widget is an implementation detail.

[jodator]

So I'd go with contentToolbar/widgetTooblar and deprecating in the docs the toolbar namespace from config.

ps.: @ma2ciek those would be 2 different classes then or the Toolbar's content will be adaptable? I'm asking because I don't know how the toolbar API changes. Also I'm OK with two classes as there will be less overhead now then with now toolbar factory.

[jodator]

@pjasiun you've got the point about widget part. For the cellToolbar vs contentToolbar - it is not cell per se - it is rather selectionInTableToolbar but the cellToolbar might be better for future. I can imagine adding a rowToolbar and columnToolbar in the future so the cellToolbar might be better then contentToolbar. Also I can live with table.tableToolbar as I don't see better name then widgetToolbar (or tableSelectedToolbar) :)

[Reinmar]

I would not do a toolbar namespace, because we need to keep toolbar backward compatible and because of reasons mentioned here.

👍 I wanted to point to your comment too :)

So I'd go with contentToolbar/widgetTooblar and deprecating in the docs the toolbar namespace from config.

👍

I think that contentToolbar is better than cellToolbar because in multi-cell selections some buttons make sense too. If we'd have explicit col/row selections then the "cell" name would not look that good.

[ma2ciek]

ps.: @ma2ciek those would be 2 different classes then or the Toolbar's content will be adaptable? I'm asking because I don't know how the toolbar API changes. Also I'm OK with two classes as there will be less overhead now then with now toolbar factory.

It will be handled easily by only one class with the new API.

[pjasiun]

I think that contentToolbar is better than cellToolbar because in multi-cell selections some buttons make sense too. If we'd have explicit col/row selections then the "cell" name would not look that good.

I do not insist on cellToolbar. So we are going to go with tableToolbar and contentToolbar? Sounds good to me.

[ma2ciek]

It will be handled easily by only one class with the new API.

I was a little bit wrong there. In the previous API version, the TableToolbar had to be imported explicitly to make the toolbar work. So it needs to still exist as a class without making a BC.