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

List Tool 2.0 #75

Closed
4 of 7 tasks
neSpecc opened this issue Jul 24, 2024 · 14 comments · Fixed by #104
Closed
4 of 7 tasks

List Tool 2.0 #75

neSpecc opened this issue Jul 24, 2024 · 14 comments · Fixed by #104
Assignees

Comments

@neSpecc
Copy link
Contributor

neSpecc commented Jul 24, 2024

We're gonna improve list creation experience:

  • Merge List, NestedList, Checklist into a single tool
  • Old list tool and checklist will be archived
  • NestedList will be renamed to List
  • We're gonna update major version

Steps

  • Discover code
  • Implement the new configurable style — "checkbox"
  • Think about data model (it should support nesting and check-states)
  • Make nesting optional (by maxLevel config option)
  • Improve tabulation UX
  • Support Ol list attributes (type, reversed, start)
  • Tool icon should respect defaultStyle config option Tools Icon #70
@Sozialarchiv
Copy link

Sozialarchiv commented Jul 26, 2024

I think merging List and NestedList is a very good idea.

I think it's worth considering whether the checklist should also be a part of it.

From a programming point of view, there are certainly synergies. On the other hand, the checklist becomes part of the additional code.

From the user's point of view, they are all lists. But you can also argue that checklists are a different kind of list.

Maybe typescript would be a nice thing for the plugin.

@neSpecc
Copy link
Contributor Author

neSpecc commented Jul 26, 2024

But you can also argue that checklists are a different kind of list

@Sozialarchiv could you explain it? Are there any disadvantages from the end user perspective?

@Sozialarchiv
Copy link

I assume that there would only be menu entry "List" in the add menu. I asked whether the checklist would then be easily overlooked. But maybe it's just a matter of getting used to it.

From my point of view, it would perhaps be good if you could deactivate the checklist.

@neSpecc
Copy link
Contributor Author

neSpecc commented Jul 26, 2024

List tool can export several entries at the toolbox

image

@Sozialarchiv
Copy link

List tool can export several entries at the toolbox

Great. Then I take the argument back. :)

@cmeeren
Copy link

cmeeren commented Jul 31, 2024

We currently have a feature request to support nested lists in our product, so we are considering supporting this nested list block. Is there any timeframe for the merge described in this issue? We need to know if we should wait or not.

Also, what is the thinking behind using "type" : "list" for the nested list block? That value is also used for the normal list, and the schemas are incompatible. This means that type can't be used as a discriminator, which I think is the intention with type in the first place. (Not sure how list/nested list handles this internally, but it can be important for other custom parsers.) This is an issue since we currently have lots of data with the normal list tool. Will this be alleviated with list tool 2.0?

@Sozialarchiv
Copy link

I would suggest a step-by-step approach so that the improvements reach users in the foreseeable future:

  1. Merging list and nested-list. To do this, we would actually only have to add the compatibility PR into nested-list. Then we could set list to depracted and rename nested-list to list.

  2. Fixing important bugs from the issue list and work through the PR list.

  3. (optional) Typescript migration.

  4. Integration of the Checkbos plugin.

@cmeeren
Copy link

cmeeren commented Jul 31, 2024

I can confirm that of those four points, the first point would unblock us (with the caveat that I haven't looked at the open bug issues).

However, I would like to know more about the point I made regarding the type discriminator and backwards compatibility (i.e., not just for this tool itself, but for all other 3rd party editor.js parsers, which may have trouble with incompatible schema for any given discriminator value).

@Sozialarchiv
Copy link

However, I would like to know more about the point I made regarding the type discriminator and backwards compatibility (i.e., not just for this tool itself, but for all other 3rd party editor.js parsers, which may have trouble with incompatible schema for any given discriminator value).

I agree that this is an important point.

I also see the point you have made here: #15 (comment)

My proposal would be to use another type value, such as list2, nested-list, or similar.

However, I am not sure whether a new type name is really necessary.

list and nested-list have been using the same type (list) for a long time. This means that parsers have already had to implement a distinction in the past.

In addition checklist could be integrated relatively easily into the data structure of nested-list, without breaking it. Here an suggestion:

current:

{
    "type" : "list",
    "data" : {
        "style" : "unordered",
        "items" : [
            {
              "content": "Apples",
              "items": [
                {
                  "content": "Red",
                  "items": []
                },
                {
                  "content": "Green",
                  "items": []
                },
              ]
            },
            {
              "content": "Bananas",
              "items": [
                {
                  "content": "Yellow",
                  "items": []
                },
              ]
            },
        ]
    }
}

suggestion

{
    "type" : "list",
    "data" : {
        "items" : [
            {
              "content": "Apples",
              "checked" : true
              "items": [
                {
                  "content": "Red",
                  "checked" : false
                  "items": []
                },
                {
                  "content": "Green",
                 "checked" : true
                  "items": []
                },
              ]
            },
            {
              "content": "Bananas",
              "items": [
                {
                  "checked" : true
                  "content": "Yellow",
                  "items": []
                },
              ]
            },
        ]
    }
}

What is not quite clear to me is how you would make sure that the plugin also reads the existing blocks with type checklist. Would you load the plugin for both the type list and the type checklist? But wouldn't the menu entries then appear twice? Or is there a more user-friendly way?

@neSpecc
Copy link
Contributor Author

neSpecc commented Aug 1, 2024

We currently have a feature request to support nested lists in our product, so we are considering supporting this nested list block. Is there any timeframe for the merge described in this issue? We need to know if we should wait or not.

We have already started. As always you can support development on Open Collective, it will increase the development speed.

Also, what is the thinking behind using "type" : "list" for the nested list block? That value is also used for the normal list, and the schemas are incompatible. This means that type can't be used as a discriminator, which I think is the intention with type in the first place. (Not sure how list/nested list handles this internally, but it can be important for other custom parsers.) This is an issue since we currently have lots of data with the normal list tool. Will this be alleviated with list tool 2.0?

Actually, tools do not care about the 'type' fields. What remains is on the user side; you can use any type you want by specifying the corresponding key in the tools parameter of the editor config.

import EditorJS from '@editorjs/editorjs';
import NestedList from '@editorjs/nested-list';

const editor = EditorJS({
  // ...
  tools: {
    ...
    myListToolType: NestedList
  },
});

@Sozialarchiv
Copy link

Sozialarchiv commented Aug 1, 2024

Actually, tools do not care about the 'type' fields. What remains is on the user side; you can use any type you want by specifying the corresponding key in the tools parameter of the editor config.

That is true.

But most plugins give an example with a type name.
https://github.com/editor-js/nested-list/blob/main/README.md?plain=1#L39

This example name is used by the most of parsers. That is imho why the question of @cmeeren is relevant. But imho I think we should give list as type example and provide data compatibly to (unnested) list and checklist.

@neSpecc
Copy link
Contributor Author

neSpecc commented Aug 1, 2024

The new List tool should support old formats, that's true. But your backend should adopt the new format too.

@cmeeren
Copy link

cmeeren commented Aug 1, 2024

I agree that this is an important point.

Thank you for hearing me out!

list and nested-list have been using the same type (list) for a long time. This means that parsers have already had to implement a distinction in the past.

While the existence of 3rd party parser implementations of the nested list tool depends on on how popular it is and the contexts in which it is used, I agree with the underlying sentiment that preserving backward compatibility is important.

In addition checklist could be integrated relatively easily into the data structure of nested-list, without breaking it.

I agree that given nested lists's format, it is simple to implement additional per-node data such as a checked state. To be clear, I never argued against nested list's format per se. I was merely arguing for using a different value than list for type. But given the need to preserve backward compatibility, this is likely not a good suggestion.

We have already started. As always you can support development on Open Collective, it will increase the development speed.

Thank you. I did not mean to request that development be sped up for our sake. To be more specific, I was just wondering if the timeframe of a new, stable format is likely closer to a week, a month or a year away. That information will help us decide on the path forward.

Actually, tools do not care about the 'type' fields. What remains is on the user side; you can use any type you want by specifying the corresponding key in the tools parameter of the editor config.

Thank you for this clarification, I did not know this. I will have to weigh the pros and cons of a non-standard type against implementation complexity of not being able to use type as a discriminator.

@amoydavid
Copy link

I am a new user of Editor.js. In the process of using it, I also found some shortcomings with List/NestedList/CheckList. I am very touched that someone is trying to update this important plugin.

During my use, I encountered the following problem. When I use EditorJS with the editorjs-drag-drop plugin, I hope to change the order of blocks by dragging the setting-button. At this time, the subitems of NestedList cannot be dragged because they do not have a setting-button. If I want to solve this issue, where can I start?

@e11sy e11sy mentioned this issue Nov 3, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants