T/ckeditor5 widget/95: Remove the getTopMostBlocks() method

[jodator]

Suggested merge commit message (convention)

Other: Remove the selection.getTopMostBlocks(). Closes ckeditor/ckeditor5#4609. Closes ckeditor/ckeditor5#3276.

BREAKING CHANGE: The selection.getTopMostBlocks() was removed from the public API. Use selection.getSelectedBlocks() instead.

BREAKING CHANGE: The selection.getSelectedBlocks() does not return blocks nested in other blocks now.

---

Additional information

Changes in other repositories:

• indent: Align code to the latest changes in engine API. ckeditor5-indent#17
• block-quote: Align code to the latest changes in engine API. ckeditor5-block-quote#39

• After some tests & digging I came to mind that getSelectedBlocks() should work as getTopMostBlock() so it does not return blocks nested in other blocks but it will search for blocks if they are nested in other non-block elements.

• most of the tests were refactored from the old ones - I've used the element#contents notation in order to properly check the cases.

I've requested you both SC & PK for reviews since I'd like to have double feedback :)

[coveralls]

Coverage decreased (-5.9%) to 94.142% when pulling e1f7e8d on t/ckeditor5-widget/95 into 2210696 on master.

[jodator]

Oh.. I'll check what is wrong with the build.

[jodator]

OK.. the block-quote have to be checked-out also :/ Tests run locally just fine.

[scofalik]

There's one thing that should be noted about the changes.

Previously, when you made selection like this: <p>Fo[o</p><table>...</table><p>B]ar</p> all the paragraphs in cells would be returned too. This means that, if you, for example, set a heading with such selection, all paragraphs in the table would become headings.

Now, it won't happen because always the top blocks are returned. <table> in this case.

I think that's actually for good. I didn't like the previous behavior even though you could argue that it is technically correct (those cells were selected, sooo... 🤷‍♂they got changed to heading 🤷‍♂).

However, this can be discussed if we are not missing something with this change of behavior. Other than that R+ from me.

[jodator]

@Reinmar so the main work is done. I have only one doubt with @scofalik comments on this case:

If I understood the code correctly, in following example:

<block>
  <blockA>[a</blockA>
  <blockB>b]</blockB>
</block>

Blocks A and B will be returned? It makes obvious sense, so I'd add it to examples. However what is returned in this scenario:

<blockX>
  <blockA>[a</blockA>
  <blockB>b</blockB>
</blockX>
<blockY>y]</blockY>

A, B, Y?

As at this moment it will return A, B, Y and also:

<blockX>
   <blockA>[a</blockA>
   <blockB>b</blockB>
</blockX>
<blockY>
   <blockC>c]<blockC>
</blockY>

it will return A, B, C (so will omit the X & Y).

Now I have no opinion on that - is it OK or not? We do not encurage such constructs but only the isLimit or isObject elements would cause to post-fix such selection (like in tables). But nothing prevents bare blocks to be seleted like this AFAIR.

Sooo maybe the getSelectedBlocks() should check for isLimit rather then block in block case? (I' almost certain that this is how this should work and not the way I've implemented this ATM).

[Reinmar]

I'm not 100% sure what should be the expected behaviour here regarding nested blocks. Since it's so hard to understand this, let's simply wait for use cases.