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

Feature: Wrap nested lists in previous listitem instead of in a new listitem #2951

Open
Placeprom opened this issue Sep 6, 2022 · 24 comments
Labels
enhancement Improvement over existing feature lists Relates to Lexical Lists

Comments

@Placeprom
Copy link

Placeprom commented Sep 6, 2022

Lexical version: 0.3.11

Current solution

Currently nested lists are added wrapped inside a new listitem:

<ol>
   <li>one</li>
   <li>
      <ol>
         <li>one.one</li>
         <li>one.two</li>
      </ol>
   </li>
   <li>two</li>
</ol>

This makes things like prefixes for nested list (1.1, 1.2 [...]) way harder to achieve. We can´t easily make use of incrementing css counters and autoincrements won´t work correctly since there is another list item in the first level list. Actually we need to set list-style-type:"none"; on nested lists to get rid of multiple (and incorrect) counters.

Alternative solution

An alternative solution would be to wrap the nested list inside the previous listitem.

<ol>
   <li>one
      <ol>
         <li>one.one</li>
         <li>one.two</li>
      </ol>
   </li>
   <li>two</li>
</ol>

This would allow us to achieve things like prefixes way easier since we can just use css counters which won´t get mixed up by a wrong number of listitems. We can also make use of the autoincrement since there is no wrong number of listitems in the previous list level.

@Placeprom Placeprom added the enhancement Improvement over existing feature label Sep 6, 2022
@YousefED
Copy link

+1 for this one. Besides CSS / styling issues, I'd also argue the structure of the internal editor state is now also "incorrect" for nested lists:

      └ (12) list  
        ├ (11) listitem  
        | └ (14) ac-text  "a"
        ├ (15) listitem  
        | └ (17) ac-text  "b"
        └ (96) listitem  
          └ (97) list  
            └ (18) listitem  
              └ (20) ac-text  "nested item"

The nested list is visually a child of "b", but in the node hierarchy it's a sibling of "b"

@surya
Copy link

surya commented Dec 23, 2022

+1. This is a blocker in situations where we can't apply styles to the rendered output.

@acywatson
Copy link
Contributor

acywatson commented Dec 23, 2022

I've been looking at re-working our nested lists logic, partially to fix this problem. One issue is that, AFAICT, it's impossible to get browsers to render a list with an indented first item using valid, semantic HTML (without using additional styles):

https://codesandbox.io/s/hardcore-voice-53j9vz?file=/index.html

In fact, you'll run into the same issue any time you try to indent a list item more than one level away from it's parent indentation level. The only way around this while preserving the free-from indentation behavior that currently exists is to directly nest UL elements, which is not valid HTML. Another option is to rely on styling to apply indentation, which compounds both the semantics problem and the rendering problem.

We could constrain users to indenting only one level below the parent, but I don't think that's the right solution - ultimately it should be possible to implement an unrestricted indent experience.

One thing I am going to try is seeing if we can change how we nest in the basic case outlined above, where you just have a single level of indentation. This is, in fact, incorrect in that it should be a child rather than a sibling of the thing it's indented "under". However, I still don't see a better way of handling more than one level of indentation here.

+1. This is a blocker in situations where we can't apply styles to the rendered output.

I'm not sure this is a blocker, given that it's possible to render the EditorState to HTML according to whatever logic you want. There's no requirement to use Lexical's HTML rendering implementation.

@surya
Copy link

surya commented Dec 28, 2022

@acywatson thanks for looking into this! I'm guessing the spec isn't explicit about this behavior.

Here's what I observed:

  • Lexical playground's theme has list-style-position: inside; in its class styles
  • TinyMCE inlines list-style-type: none
  • Trix editor doesn't let you do it.
  • GMail uses semantically invalid HTML by inserting ol/ul outside a list item

I agree with changing the behavior for single level indentation to make it a sibling. We can then write our own logic to customize multiple levels of nesting.

@GermanJablo
Copy link
Contributor

The more I think about lists and indents, the more convinced I am that ul and ol were an HTML error from the start. In my opinion, the standard should make those two tags legacy, and instead add an indent attribute, which can be applied, not only to li but also to paragraphs or headings.

I have sent this proposal to the organization that is in charge of standardizing HTML. I know that in the very difficult case that it is seriously considered it would take years to implement, but hey... at least I tried 😂🤷‍♂️

@gaurav21r
Copy link

@acywatson this IS a blocker because Copy Paste is not behaving properly even amongst lexical instances #3740

@GermanJablo
Copy link
Contributor

@acywatson and @thegreatercurve, I'm following the conversation on #3740, where it looks like the copy-paste issue has already been fixed (at #3757).

Regarding the problem of prefixes in enumerated lists, I can't find a case where it is counter-intuitive with the current implementation. However, you said in that thread that you will change the list's behavior.

What is the change you plan to make (and why)? Forbid a li from indenting more than one level from her parent?

@acywatson
Copy link
Contributor

What is the change you plan to make (and why)? Forbid a li from indenting more than one level from her parent?

Selected quotes from my comment above:

With respect to "Forbid a li from indenting more than one level from her parent?"

We could constrain users to indenting only one level below the parent, but I don't think that's the right solution - ultimately it should be possible to implement an unrestricted indent experience.

So, no, I don't think we should do that.

With respect to "What is the change you plan to make (and why)?":

One thing I am going to try is seeing if we can change how we nest in the basic case outlined above, where you just have a single level of indentation. This is, in fact, incorrect in that it should be a child rather than a sibling of the thing it's indented "under". However, I still don't see a better way of handling more than one level of indentation here.

So, my current thinking is that we should try to change it such that a single level of indentation is semantically correct. We can do this within the HTML spec. What we can't do is create semantically correct structures with more than one level of indentation relative to the parent. However, my hope is that making things work correctly in the base, single-level-of-indentation case will make it smoother without regressing the more complex case of multiple indentation levels. We shall see how the implementation goes.

@acywatson
Copy link
Contributor

The more I think about lists and indents, the more convinced I am that ul and ol were an HTML error from the start. In my opinion, the standard should make those two tags legacy, and instead add an indent attribute, which can be applied, not only to li but also to paragraphs or headings.

I think you might be right about this. I can't rightly think of anything that would be worse about that than the current nesting approach. And there are certainly some things that would be better/simpler. Bottom line is that I just don't think the expressive rich-text use cases covered by modern editors were top-of-mind when the HTML spec was created.

@HalvorHaugan
Copy link

I would say that, strictly speaking, nesting and indenting is not the same thing. As I see it, indenting is primarily a visual thing while nesting (of lists at least) is primarily a semantic feature to define a parent-children relationship. Then we use indentation to visualize that relationship. Hence imo, indenting should be done with CSS while nesting should be done semantically with HTML.

With this premise, I don't think it makes sense to use nesting to indent an item more than one level bellow the parent. It would technically make the item an orphan (or perhaps just make the parent nameless 😅).

I don't really understand why anyone would want to indent an item multiple levels, but that's just me. I respect that you want to make the editor as flexible as possible.

Maybe it would be possible to make the first indentation use nesting, while subsequent indentations only use CSS? Personally though, I would be perfectly happy with constraining to one level of indentation bellow the parent, as long as the HTML is valid and semantically correct. I would think that valid HTML/semantics would be a high priority to ensure accessibility, compatibility and consistent behavior across browsers.

@Awjin
Copy link
Contributor

Awjin commented Mar 6, 2023

I don't really understand why anyone would want to indent an item multiple levels, but that's just me. I respect that you want to make the editor as flexible as possible.

I am someone who heavily relies on indenting items multiple times. In my experience Lexical is the only RTE library that gets this functionality right, so disabling it would be a huge regression (for me).

@acywatson
Copy link
Contributor

I would think that valid HTML/semantics would be a high priority to ensure accessibility, compatibility and consistent behavior across browsers.

It is, which is exactly why this whole thing is a problem in the first place. If we didn't care about this, we would just do it the way other text editors do, which is by using CSS or just relying on user-agent styles produced by invalid HTML (i.e., directly nesting ul tags)

I would say that, strictly speaking, nesting and indenting is not the same thing. As I see it, indenting is primarily a visual thing while nesting (of lists at least) is primarily a semantic feature to define a parent-children relationship. Then we use indentation to visualize that relationship. Hence imo, indenting should be done with CSS while nesting should be done semantically with HTML.

This is, of course, true. The browser rendering of a list is just the conventional visualization of that semantic structure. The problem is that modern rich-text editing experiences almost universally support this type of multiple-level indentation, whereas the HTML spec simply doesn't. There's no clean way around it.

Maybe it would be possible to make the first indentation use nesting, while subsequent indentations only use CSS?

I think the best solution is ultimately different treatment of single vs multiple levels, but exactly how that implementation will work, I'm still not 100%.

@zurfyx zurfyx added the lists Relates to Lexical Lists label Apr 12, 2023
@Nantris
Copy link

Nantris commented Apr 25, 2023

Any updates on this?

It seems pretty well established that there's no "right" way to do it, but Lexical handles it differently than most other solutions I've seen and it is really difficult to work with from a styling standpoint.

Not having the ability to use CSS counters makes styling lists nicely a near impossibility since custom counters are used to overcome the shortcomings of HTML's in-built list styling.

In Slate using the old slate-edit-list plugin the structure is like this (the same as @Placeprom mentioned in the OP):

<ol class="NumberList">
    <li class="ListItemRoot">
        <p class="Paragraph"><span>ABC</span></p>
        <ol class="NumberList">
            <li class="ListItemRoot">
                <p class="Paragraph"><span>DEF</span></p>
            </li>
            <li class="ListItemRoot">
                <p class="Paragraph"><span>GHI</span></p>
            </li>
        </ol>
    </li>
</ol>

I have found this structure to be extremely flexible to work with. It displays properly in all modern browsers. It supports CSS list counters. It's easy to conceptualize.

I see this issue has been around for quite a while - I wonder what the odds are that the way lists are handled will change to accommodate this?

I wouldn't call this a hard blocker for us, but it's certainly a considerable issue which I see no workaround for without inventing our own new list plugin - but that seems unsustainable and a shame since the Lexical team is already maintaining one that's so close to being everything we need.

@acywatson
Copy link
Contributor

I have found this structure to be extremely flexible to work with. It displays properly in all modern browsers. It supports CSS list counters. It's easy to conceptualize.

Yea, it works fine until you start to think about multiple levels of indentation. That's the crux of the issue. Would love to change this and probably will at some point, but I can't make guarantees about timeline.

What are you trying to do that you can't do now?

@Nantris
Copy link

Nantris commented Apr 25, 2023

The primary issue is that I don't see how CSS counters can be applied to multi-level lists the lists as they're structured right now. Maybe I'm just not being imaginative enough - but I don't see any way to avoid the wrong list counters from the previous level of the list when indenting.

Yea, it works fine until you start to think about multiple levels of indentation.

I'm not clear on that. I guess you mean directly indenting a child twice from a parent? Because as far as I can tell it nests beautifully as long as you don't allow that.

And I think that that would be an extremely niche scenario, far more than expecting CSS counters to work with the lists. I feel like double indenting would constitute an orphan, as @halvor07 put it. But if it were necessary, couldn't it be achieved with an additional className or data attribute to indicate the block needs to be double/triple/quadruple indented?

I mean - there's no way to do both things at the same time it seems - but breaking a crucial styling feature seems very problematic to me.

Edit: example of the problem with CSS counters:

image

@acywatson
Copy link
Contributor

acywatson commented Apr 25, 2023

Is this what you want? Just clarifying
Screenshot 2023-04-25 at 3 39 25 PM

@Nantris
Copy link

Nantris commented Apr 25, 2023

Yes indeed. Am I overlooking a way to achieve that with CSS counters using either ::before or ::marker?


Edit: We also use use the nesting for styling purposes - eg to display decorations on all the child list elements. That seems possible with either format, but with the format Lexical uses now it seems quite difficult and expensive - whereas with the other structure it's simple and performant via CSS - no JS required.

image

@acywatson
Copy link
Contributor

I made this work in the playground by just deleting this line:

This is using theming though, so per our other thread may or may not be as easy for you.

@Nantris
Copy link

Nantris commented Apr 25, 2023

@acywatson I hate to add on but I just ninja-edited before you commented that. Would you have any suggestions for how to handle our other issue in a reasonable way?

I'm giving your suggestion a test now, and thank you kindly for it.

@acywatson
Copy link
Contributor

eg to display decorations on all the child list elements

I guess you can do this the same way - using those nested levels theme classes. Perhaps you could use level-based inline styles applied in createDOM/updateDOM.

@Nantris
Copy link

Nantris commented Apr 25, 2023

Just FYI, I was attempting to implement the counter styles via ::before - but I guess it's easier these days to use ::marker for our purpose. It is a bit more limited, but I think it will work. I do not believe a ::before solution using custom CSS list counters can be implemented as Lexical is currently structured - and certainly one that uses images for the counter cannot be implemented.


I'm not sure if I was clear in my original explanation of the styled list children - the nesting styles would only be applied to items that are children of the currently focused list. I suppose this can still be achieved though, with li#active, li#active + li li - but that's a more expensive rule for a large document than with a rule like li#active, li#active li which works with OPs structure. I'm not sure it matters in the real world.

@acywatson
Copy link
Contributor

Yea I would have to consider that for a bit - I don't think there is much (functionally) that can't be achieved in some way with the current setup. That said, I do understand the limitations and have acknowledged those above. I would like to remedy those, but everything is a matter of prioritization.

@msdrigg
Copy link
Contributor

msdrigg commented May 26, 2023

For those just wanting their lists to work right, the playground solves this here

  1. .PlaygroundEditorTheme__nestedListItem {

For me with tailwindcss, this was as easy as adding this theme

{
    list: {
        nested: {
            listitem: "list-none",
        },
    }
}

@rzrotem
Copy link

rzrotem commented Jan 16, 2024

Hello! I'm having a problem with implementing a list behaviour
image
because of the extra list item eleemt. Is there an example of how I can achieve it?
This is the best i was able to get with CSS counter
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement over existing feature lists Relates to Lexical Lists
Projects
None yet
Development

No branches or pull requests

13 participants