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

Todo items mixed with regular list items #166

Closed
holtwick opened this issue Jan 17, 2019 · 9 comments
Closed

Todo items mixed with regular list items #166

holtwick opened this issue Jan 17, 2019 · 9 comments

Comments

@holtwick
Copy link

holtwick commented Jan 17, 2019

I am currently looking into todo item support and as far as I understand the current implementation, it looks although it behaves like a list, it cannot be mixed with other list items of numbered or bulleted lists. Is that correct?

For my project I would like to mix these and therefore am considering two options, which might be interesting for TipTaps feature as well:

  1. Make the TodoList accept ListItem nodes. Difficulty: How can the ListItems become aware, that they should work like a todo entry?
  2. Don't wrap ListItem etc. at all and make them live side by side with Paragraph or Header on the top level and make their indentation just another attribute. Numbering could be achieved with some CSS tricks. Difficulty: Parsing and generating HTML.

What do you think? Is there some "golden" way to do it?

@holtwick
Copy link
Author

Another little issue with list is, that you cannot change the list type intuitively: https://tiptap.scrumpy.io/ This is an issue as well in the regular ProseMirror demo.

listchange

@exnihilo-creatio
Copy link

exnihilo-creatio commented Jan 18, 2019

@holtwick Changing between the list types given in #4 also, so that part is known.

@holtwick
Copy link
Author

I implemented my second idea and think it is quite alright for my specific needs. It needs some more fine tuning, but this is how it looks in action:

screenflow

In case this is interesting for TipTap as well I can share the code.

The drawbacks as mentioned are the parsing and DOM creation. I'm planning to look into these issues next.

@exnihilo-creatio
Copy link

@holtwick Nice, I would expect thought that increasing the indentation to also change the list type. For example, the first tab inscrease in the example to check the textbox to an a list and merge the a -> b etc.

@Alecyrus
Copy link
Sponsor Contributor

@holtwick Nice work !!! That is just what I wanna to implement. Could you share your code as a documentation example? That will be helpful.

@holtwick
Copy link
Author

I'm not super happy with the results as well yet, but the other approaches have different issues as well.

You can experiment with the current implementation here: https://collect-app.com/web/?ref=tiptap

The extensions and SCSS files are attached as ZIP file here TodoList.zip. It needs some handwork to adopt but the idea should become clear.

A side issue I experienced is, that the Vue based nodeView did cause troubles with complex embedded inline nodes I used in my project. They simply did not appear. I didn't want to dive much deeper into this at this point, therefore I extended the Editor class to allow definition of regular DOM based nodeViews, like this:

class AppEditor extends Editor {

    initNodeViews({parent, extensions, editable}) {
      let nodeViews = super.initNodeViews({parent, extensions, editable})
      extensions.filter(e => e.nodeView).forEach(e => {
        nodeViews = {
          ...nodeViews,
          [e.name]: (node, view, getPos, decorations) => new e.nodeView(node, view, getPos, decorations)
        }
      }) 
      return nodeViews
    }

}

@philippkuehn
Copy link
Contributor

@holtwick Really interesting solution! 👍Unfortunately the handling of lists is very hard with the standard implementation of Prosemirror. I would really like to see some improvements in the core. There are a lot of threads and issues out there waiting for a better handling of lists.

But I don't think unwrapping list items is the best solution. One of my main reasons to find a new editor for scrumpy.io was our previous editor Quill. Quill also do have a AST to store content but it's only one-dimensional, what means that something like nested lists are really painful to use/extend. They also store the indention level for that. I don't think that we should change the nested structure of HTML elements in our document.

For me it's important that we can copy nested lists into tiptap and vice versa.

But back to topic: I think the only way to get this working with the default list extensions of tiptap is that we have to rewrite the prosemirror-schema-list package. 😩

@holtwick
Copy link
Author

holtwick commented Feb 2, 2019

Hi @philippkuehn thank you very much for that extended and reasonable answer. I believe there are good arguments for both sides, but I agree that the structural nature of lists is to be nested, not to be just another indentation property.

I will very likely still go that way, because it is simpler to maintain on the code level for me right now. And parsing DOM does work already. The other way should also not be too hard. But all the key handling like ENTER and BACKSPACE needs a major rewrite, which will certainly be time consuming.

Anyway, I would also love to see an improvement in the prosemirror-schema-list package, but I personally will not be able to do it. Maybe this is a discussion that should shift to to the ProseMirror lists.

Once I finished my implementation I'll give a final feedback of the implementation details either as a motivation or as a warning. We'll see ;)

@philippkuehn
Copy link
Contributor

For now I do not plan to support that in the core packages.

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

No branches or pull requests

4 participants