I/6150: Make table cell properties UI aware of mult-cell selection.

[jodator]

Suggested merge commit message (convention)

Enhancement: Add multi-cell selection to table cell properties UI. Closes ckeditor/ckeditor5#6150.

---

Additional information

This PR adds the ability to style selected table cells with basic logic for command value. I've extracted a follow for for multi-cell values handling as it looks optional and requires decisions.

@Reinmar: is this follow up OK? ckeditor/ckeditor5#6320

[coveralls]

Coverage remained the same at 100.0% when pulling 3713344 on i/6150 into 379c022 on master.

[Reinmar]

cc @ckeditor/qa-team I'd love to have your 👀 on this

[Mgsy]

Looks good, we haven't found any issues 👍

[Reinmar]

Why not using the first range and adding there getSelectedElement()?

[jodator]

If you refer to the selection.getSelectedElement() (https://github.com/ckeditor/ckeditor5-engine/blob/bb4def6aac9d221034045f96c1be1f422ff67393/src/model/selection.js#L613-L623) then it is only checking for nodeAfter or nodeBefore whereas in table UI case we have two cases:

1. A collapsed selection inside tableCell (here findAncestor( position ) works fine
2. A multi-range selection in which ranges are set 'on' a tableCell - here it is the nodeAfter first position - but I can lookup API for an existing method on Range/Selection.

[Reinmar]

I'm unsure about this. What if there are two paragraphs in one td? Won't it return this cell twice?

Also, I don't like that there are actually two helper functions introduced in this single PR. Can't we have one, generic enough to cover all those cases? Optionally, two functions getSelectedTableCell( range ) and getSelectedTableCells( selection ) next to each other.

[jodator]

They have a different purpose so renaming them might be better.

The getSelectedTableCell (described below) is used because selection might be collapsed inside table cell or set as a range on a table cell.

The getSelectedTableCells( selection ) is used to get all selected table cells from a document.selection and might be worth to revisit it for the cases found yesteraday for the purpose of TableSelection#hasMultiRangeSelection.

ps.: I'm going to revist those later today

[jodator]

OK so this one could be removed in favor of TableSelection.getSelectedTableCells() - I think that would be OK to use TableSelection plugin for this. The getSelecteTableCells() can be improved then (ie. in the ticke that touches undo problems).

buuut this would make TableSelection obligatory for TableProperties which I'm not sure how to handle.

Probably a generally accessible util method which all commands/plugins could use to get selected table cells (I'm thinking about other commands that should be now aware of the new option of selecting table cells (collapsed selection in TC or multi-range selection on TCs).

This way we will be having one way to get selected TC as an util (independent of loaded plugins) which will work both cases of selection. This could also invalidate need for getSelectedTableCell() which returns only one - you can always get one from all selected.

[Reinmar]

As commented. I think we need more ordnung with these methods.

[oleq]

So the idea is that somewhere in ckeditor/ckeditor5#6320 this helper will return 'multiple'?

[jodator]

The idea is to have another property for multiple values (Command#values). But I didn't give a second thought to the 6320.

So this method might not change for the multiple display. I think that we should expose multiple values (or some getter to check for that case) in the command and implement how to show this in the UI. Command#values might be usable for other features and showing "multiple" in the form looks like a narrow case of the UI.

[oleq]

Committing all these suggestions will make my GH contribution graph black for the day :P

[jodator]

I've removed the export and made that utility method private in the UI tools where it belongs.

[Reinmar]

I reported followups for the mentioned issues in ckeditor/ckeditor5#6358 and ckeditor/ckeditor5#6357.