search

ckeditor / ckeditor5

Powerful rich text editor framework with a modular architecture, modern integrations, and features like collaborative editing.
https://ckeditor.com/ckeditor-5
Other
9.54k stars 3.7k forks source link

Styles dropdown does not correctly find all elements on which a style should be applied #12417

Closed scofalik closed 2 years ago

scofalik commented 2 years ago

📝 Provide detailed reproduction steps (if any)

  1. Select part of a content, where block elements (e.g. paragraphs) are inside container elements (e.g. htmlSection). 
    <htmlSection><paragraph>[Foo</paragraph></htmlSection>  
<htmlSection><paragraph>Bar</paragraph></htmlSection>  
<htmlSection><paragraph>Baz]</paragraph></htmlSection>
  2. Apply paragraph style.

✔️ Expected result

Paragraph style is applied to all paragraphs.

❌ Actual result

"Middle" paragraphs are not styled.

https://user-images.githubusercontent.com/1502228/188593456-0c60c6b6-bbea-42b9-89e7-59055be4fce8.mov

scofalik commented 2 years ago

What we found so far:

  1. blockQuote is treated differently than e.g. htmlSection. Block quote is not a block, custom elements are. Should they?
  2. Not sure if we should return all elements or only the deepest one (<div><div><div>Foo</div></div><div> case).
  3. What about table inside table? Should styling outside table impact nested tables?
  4. Note: there is htmlDivParagraph and htmlDiv and they are different (schema-wise).
  5. Anything we do should base on Schema, not on element names.
  6. If no generic algorithm is found, we can have different treatment for different element-types (schema-wise).
  7. schemadefinitions.js in GHS -- can this be overwritten by integrator or maybe integrator can only choose from available options? Context: paragraphLikeModel.
scofalik commented 2 years ago

Answering the most important question: it seems that htmlSection and similar should not be blocks but containers instead.

We can start with this ticket: ckeditor/ckeditor5#12430.

scofalik commented 2 years ago

The issue no longer occurs after fixing #12430.

From the questions that we asked earlier, I think that the most important is:

Not sure if we should return all elements or only the deepest one (<div><div><div>Foo</div></div><div> case).

I will add another issue for this.

Edit: actually, I just checked and the style is applied to deepest found element. I think this behavior makes sense, so I am not adding a new issue.