Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Document lists] Research the approach to the conversion in GHS #11284

Closed
niegowski opened this issue Feb 15, 2022 · 2 comments
Closed

[Document lists] Research the approach to the conversion in GHS #11284

niegowski opened this issue Feb 15, 2022 · 2 comments
Assignees
Labels
package:html-support package:list squad:core Issue to be handled by the Core team. type:task This issue reports a chore (non-production change) and other types of "todos".

Comments

@niegowski
Copy link
Contributor

niegowski commented Feb 15, 2022

  • Try exposing conversion helper from DocListProps
    • Check feasibility
    • PoC of it used in GHS to handle attrs of ul/ol and li
@niegowski niegowski added squad:core Issue to be handled by the Core team. package:list type:task This issue reports a chore (non-production change) and other types of "todos". labels Feb 15, 2022
@niegowski niegowski self-assigned this Feb 16, 2022
@Reinmar
Copy link
Member

Reinmar commented Feb 17, 2022

🟢 Scenario 1 (that works)

Set data:

<paragraph listIndent=0>x</paragraph>
<paragraph listIndent=1>y</paragraph>

Conversion to view:

  • listIndent=0 -> wraps the <p> in <li> and <ul>
  • listIndent=1 -> wraps the <p> in <li> and <ul> and <li> and <ul>

At some point we have this in the view:

<ul><li><p>x</p></li></ul>
<ul><li><ul><li><p>y</p></li></ul></li></ul>

Thanks to ul and li being attribute elements, the uls and lis that are "similar" are merged into:

<ul><li><p>x</p>  [</li></ul>
<ul><li>]   <ul><li><p>y</p></li></ul></li></ul>

(everything between [] is deleted on this merge)

Finally, we get this:

<ul><li>
    <p>x</p>
    <ul><li>
        <p>y</p>
    </li></ul>
</li></ul>

🟢 Scenario 2 (that still works)

Set data:

<paragraph listItemId=a listIndent=0>x1</paragraph>
<paragraph listItemId=a listIndent=0>x2</paragraph>
<paragraph listItemId=b listIndent=1>y</paragraph>

... conversion happens

At some point we have this in the view:

<ul><li _id=a><p>x1</p></li></ul>
<ul><li _id=a><p>x2</p></li></ul>
<ul><li _id=a><ul><li _id=b><p>y</p></li></ul></li></ul>

.... merging happens

Finally, we get this:

<ul><li>
    <p>x1</p>
    <p>x2</p>
    <ul><li>
        <p>y</p>
    </li></ul>
</li></ul>

🟢 Scenario 3 (that still works)

Set data:

<paragraph listItemId=a listIndent=0 listStyle=Q>x1</paragraph>
<paragraph listItemId=a listIndent=0 listStyle=Q>x2</paragraph>
<paragraph listItemId=b listIndent=1 listStyle=Q>y</paragraph>

... base conversion of lists happens

At some point we have this in the view:

<ul><li _id=a><p>x1</p></li></ul>
<ul><li _id=a><p>x2</p></li></ul>
<ul><li _id=a><ul><li _id=b><p>y</p></li></ul></li></ul>

Then, listStyle conversion kicks in. It wraps e.g. <p>x1</p> in another <ul style=Q> structure. But since the outermost uls are "similar" (<ul style=Q><ul> -> <ul style=Q>) we get this:

<ul style=Q><li _id=a><p>x1</p></li></ul>
<ul style=Q><li _id=a><p>x2</p></li></ul>
<ul style=Q><li _id=a><ul style=Q><li _id=b><p>y</p></li></ul></li></ul>

.... then merging happens

Finally, we get this:

<ul style=Q><li>
    <p>x1</p>
    <p>x2</p>
    <ul style=Q><li>
        <p>y</p>
    </li></ul>
</li></ul>

🔴  Scenario 4 (that doesn't work)

Set data:

<paragraph listItemId=a listIndent=0 listStyle=Q htmlLiAttributes=W>x1</paragraph>
<paragraph listItemId=a listIndent=0 listStyle=Q htmlLiAttributes=W>x2</paragraph>
<paragraph listItemId=b listIndent=1 listStyle=Q>y</paragraph>

... conversion happens

At some point we have this in the view:

<ul><li _id=a><p>x1</p></li></ul>
<ul><li _id=a><p>x2</p></li></ul>
<ul><li _id=a><ul style=Q><li _id=b><p>y</p></li></ul></li></ul>

Then, listStyle conversion kicks in. It wraps e.g. <p>x1</p> in another <ul style=Q><li> structure. But since outermost lis and uls are "similar" we get this:

<ul style=Q><li _id=a><p>x1</p></li></ul>
<ul style=Q><li _id=a><p>x2</p></li></ul>
<ul style=Q><li _id=a><ul style=Q><li _id=b><p>y</p></li></ul></li></ul>

Then GHS's converter kicks in. It wraps those blocks again in another ul/lis. Situation before GHS's elements get merged with their siblings:

<ul style=Q><li _id=a W><li _id=a><p>x1</p></li></ul>
<ul style=Q><li _id=a W><li _id=a><p>x2</p></li></ul>
<ul style=Q><li _id=a W><li _id=a><ul style=Q><li _id=b><p>y</p></li></ul></li></ul>

We'd like <li _id=a W><li _id=a> to get merged into a single <li W _id=a> but IDs prevent merging.

PROBLEM ☝️

@Reinmar
Copy link
Member

Reinmar commented Feb 17, 2022

Ideas:

  • Maybe we could stop using IDs at all and instead override isSimilar() so it uses some custom property.
  • We found out that IDs are handled differently in case of nested and sibling elements. In case of siblings, if IDs are equal, elements get merged. This doesn't happen for nested ones.
    • This seems buggy.
    • Let's try to merge in both cases.
    • Identical IDs mean that both attribute elements came from the same "source" (e.g. the same comment). Then, why having it twice in the model?

@Reinmar Reinmar added the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Feb 21, 2022
@Reinmar Reinmar closed this as completed Mar 11, 2022
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Mar 11, 2022
@CKEditorBot CKEditorBot added this to the upcoming milestone Mar 11, 2022
@CKEditorBot CKEditorBot modified the milestones: upcoming, iteration 52 Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:html-support package:list squad:core Issue to be handled by the Core team. type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

No branches or pull requests

3 participants