flutter-quill icon indicating copy to clipboard operation
flutter-quill copied to clipboard

Numbered list wrong numeration after increase + decrease indent

Open Alspb opened this issue 1 year ago • 18 comments

Is there an existing issue for this?

Flutter Quill version

master

Steps to reproduce

  1. Increase the numbered list indent 3 times, until the numbering style is the same again.
  2. For the last list item decrease its indent 3 times.

Expected results

The last item numeration start from 1.

Actual results

The last item numeration start from 2.

Code sample

Code sample
[Paste your code here]

Screenshots or Video

Screenshots / Video demonstration

Numbered list numeration bug

Logs

Logs
[Paste your logs here]

Alspb avatar Jul 05 '24 05:07 Alspb

@AtlasAutocode , could you please take a look since you might be familiar with this code section?

Alspb avatar Jul 05 '24 05:07 Alspb

Part of the confusion might be that indents are limited to 5 levels (no idea why). Ordered lists seem to have only 3 levels so it repeats the sequence. When you indent 3 times you get back to '1.' However, this is not the same as the un-indented '1.' So, the next outdented list item will be '2.'. That is correct (but not what you want!)

It appears that the first list item is implicitly set to 1 even if it is shown as an indented value. For example: start a list and indent to give the first item 'a: text'. Enter another list item, outdent it and it will be '2. next'.

I am not familiar with the code, but I guess that there is an overlap between indentation of text and indentation of list items. Am I correct that you want to create a normal list, but have it indented?

I see several approaches:

  1. Resolve the confusion between indentation and list-depth
  2. Have 5 levels of list indentation: 1, a, i, and then what?
  3. I am sure there will be some other way....

AtlasAutocode avatar Jul 05 '24 13:07 AtlasAutocode

lists indentation

Sounds reasonable. Since list indentation is used to create nested lists, the editor thinks that there's an item number "1" before the "text" item. But it looks strange of course, especially for the user who simply wants to use different numbering style for the first list (i.e., "text")

I think that the connection between list indentation and list depth is very useful, so it's better not to change it. No sure what is the proper way of handling such issues. Maybe "text" item should also be unindented together with "next".

Also, delta to HTML converter treats such cases as if "text" is not indented too.

As for having 5 types of list numeration, I think it's reasonable anyway. One can take the same styles as in type attribute of the ol HTM tag, it will also make the conversion to HTML more precise.

Alspb avatar Jul 07 '24 11:07 Alspb

I was thinking that it might be more logical if indentation of the first item could be handled as an indentation of the whole list. The first item would then always be '1.' and you would not be able to 'outdent' the second item. (Avoids the issue in your screen shot) Indentation of subsequent items would then follow the expected pattern 'a', 'i' ... Allowing the user to pick a different starting label would be tricky. But word processors allow starting a list at '4' (for example). I often have to do this when continuing a list after an interruption.

Of course, it is easy for me to suggest this, as I have no idea if this can be done...

AtlasAutocode avatar Jul 07 '24 13:07 AtlasAutocode

Sounds reasonable, but not sure it's easy to implement - we should introduce a new logic of changing/not-changing other block indents in response of changing current block indent. Also, if more than one block is indented, this logic should also work fine. And probably the trickiest thing is that everything should work properly when pasting indented text (not only when pressing "Indent"/"Outdent" toolbar buttons). Not sure that this bug worth such an effort.

I'd say that from implementation point of view it should be easier to just fix the numeration in the case above to have a. text 1. next So, indentation logic remains the same, just numbers representation changes a bit.

Alspb avatar Aug 22 '24 21:08 Alspb

I guess we need to add more variables that reference the list indentation level to fix this more easily. I did this in flutter_quill_to_pdf and it solved exactly the same problem as this one since it only updates the list number depending on the indentation it has and resets it the same way.

It is something like:

    if (blockAttributes?['list'] != null) {
      if (indent != null) {
        // validate if the last indent is different that the current one
        //
        // if it is, then must reload the specific index counter to avoid generate
        // a bad index for the current item
        if (lastListIndent != indent) {
          if (indent == 1) numberIndent1List = 0;
          if (indent == 2) numberIndent2List = 0;
          if (indent == 3) numberIndent3List = 0;
          if (indent == 4) numberIndent4List = 0;
          if (indent == 5) numberIndent5List = 0;
        }
        lastListIndent = indent;
        if (indent == 1) numberIndent1List++;
        if (indent == 2) numberIndent2List++;
        if (indent == 3) numberIndent3List++;
        if (indent == 4) numberIndent4List++;
        if (indent == 5) numberIndent5List++;
      } else {
        lastListIndent = 0;
        numberList++;
      }
    } else {
      lastListIndent = 0;
      numberList = 0;
      numberIndent1List = 0;
      numberIndent2List = 0;
      numberIndent3List = 0;
      numberIndent4List = 0;
      numberIndent5List = 0;
    }

CatHood0 avatar Aug 23 '24 00:08 CatHood0

Looks good - go for it.

My only question is: What if someone in the future changes the maximum allowed indentation level? Could you use numberIndentList[x] where the size of the array is a constant for max indentation also referenced in limiting indentation level.

That may also simplify the code a bit.

AtlasAutocode avatar Aug 23 '24 12:08 AtlasAutocode

@AtlasAutocode i fix it. I just needed to add this condition to clear correctly the indents:

if (prevNodeOl && attrs[Attribute.list.key] != Attribute.ol || attrs.isEmpty) {
   clearIndents = true;
}

This simple condition clear correctly the indent and fix the numeration of the list. I don't test if test remove some functionality or broke something (i did a small test, and nothing was wrong when i used indent in other parts)

CatHood0 avatar Aug 26 '24 18:08 CatHood0

There is a programming concept called 'Extreme Programming' which notes that: 'bugs are always found in code that was written'. Therefore: 'Write the least amount of code that solves the problem' 2 lines is good!

AtlasAutocode avatar Aug 26 '24 18:08 AtlasAutocode

@CatHood0 , thanks, #2146 looks cool. But how does it solve the issue? Should I use customLeadingBlockBuilder to configure ordered lists somehow?

Alspb avatar Sep 12 '24 22:09 Alspb

By the way, for some reason this looks weird (the numeration starts with 2):

    a. text
2. next    

while the indented version is ok (the numeration starts with "a"):

        i. text
    a. next    

Alspb avatar Sep 12 '24 23:09 Alspb

But how does it solve the issue?

It doesn't fix it directly. I mean that while creating that Feature, I happened to find how to fix the error. It was just necessary to add an additional check in _buildChildren of raw_editor_state so that it would be reset if necessary

Should I use customLeadingBlockBuilder to configure ordered lists somehow?

Not really, this parameter was added for those people who want to have a higher level of customization for their lists (whether default or custom)

By the way, for some reason this looks weird (the numeration starts with 2):

This should be fixed now (as far i know and i test)

CatHood0 avatar Sep 12 '24 23:09 CatHood0

This should be fixed now (as far i know and i test)

Do you mean that you have

    a. text
1. next    

instead?

Alspb avatar Sep 12 '24 23:09 Alspb

Another related question: does #2146 allow to change numbering style for ordered lists? Personally, I'd like to have "1", "A", "a", "I", "i" (as in HTML) for indents 1-5. But others might have their own preferences. Moreover, changing it editor-wise might be considered as a breaking change, which it's better to avoid if possible.

Alspb avatar Sep 12 '24 23:09 Alspb

Using customLeadingBlockBuilder you can use leadingConfigurations.getIndexNumberByIndent; to get the correct index for ordered lists (you can assume that leadingConfigurations is the param passed by customLeadingBlockBuilder that contains this method), and just use toUppercase method to adjust the numbering style to your preferences.

CatHood0 avatar Sep 12 '24 23:09 CatHood0

Do you mean that you have

Yeah, that's the result that i getting. But, if you have the lastest version of the package, and this persist, please show a video or steps to reproduce it.

CatHood0 avatar Sep 12 '24 23:09 CatHood0

Basically just indenting the first item causes the problem: untitled.webm

Alspb avatar Sep 13 '24 00:09 Alspb

I'll take a look. Thanks for reporting

CatHood0 avatar Sep 13 '24 00:09 CatHood0