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

Better interaction between formatting options in lists #2261

Closed
2 tasks done
holtwick opened this issue Dec 10, 2021 · 8 comments
Closed
2 tasks done

Better interaction between formatting options in lists #2261

holtwick opened this issue Dec 10, 2021 · 8 comments
Labels
Type: Bug The issue or pullrequest is related to a bug

Comments

@holtwick
Copy link

holtwick commented Dec 10, 2021

What’s the bug you are facing?

If I'm inside a bullet list, changing to "paragraph", "heading" or "blockquote" does not work.

How can we reproduce the bug on our side?

Just try it in your demo editor.

Can you provide a CodeSandbox?

No response

What did you expect to happen?

All "block level" actions should interact correctly with each other. Especially those that exclude each other like list and heading.

Anything to add? (optional)

No response

Did you update your dependencies?

  • Yes, I’ve updated my dependencies to use the latest version of all packages.

Are you sponsoring us?

  • Yes, I’m a sponsor. 💖
@holtwick holtwick added the Type: Bug The issue or pullrequest is related to a bug label Dec 10, 2021
@philippkuehn
Copy link
Contributor

Converting list items to text blocks like paragraphs or headings should work better now with this patch.

But blockquotes behave differently, since they can contain block nodes themselves. What would you expect here?

  1. Should the listItem be converted to a blockquote and then contain a paragraph with its text? (should be easy)
  2. Or should the entire listItem be moved to a blockquote. For this the moved listItem must be wrapped into a new list. (more complex)

Feel free to create a new issue for the blockquotes.

@holtwick
Copy link
Author

Thanks a lot for the quick fix!

I'm not 100% sure which blockquote idea might be the best. Some use cases I saw:

  1. Selecting multiple blocks (like headers, paragraphs or list items, which in this case I would consider blocks as well) I would expect blockquote to wrap the whole thing and vice versa
  2. Without multi block selection I might expect to just convert to a blockquote

I understand blockquote more being a wrapper around a group of content. You might have similar features in TipTap somewhere for grouping content? To avoid complexity with lists, the blockquote might be automatically get extended to the whole list element? Remembers me a bit of #166 ;)

@philippkuehn
Copy link
Contributor

The behavior of blockquotes across multiple nodes is also interesting. Because a NodeRange with a specific depth is calculated for the ProseMirror wrapIn command, in this case also the paragraph and the whole bulletList is wrapped instead of paragraph and the first listItem:

CleanShot.2021-12-13.at.14.52.49.mp4

So a third option would be to wrap the entire list when trying to wrap a listItem. What do you think?

@holtwick
Copy link
Author

I like it the way it behaves in the video 👍

@philippkuehn
Copy link
Contributor

Just to be clear: This is how it already works now. See:

CleanShot.2021-12-13.at.15.20.28.mp4

The calculation of the NodeRange depth is only wrong if the same is tried within a list. But I could fix that so that it behaves the same within lists.

@holtwick
Copy link
Author

I see. Indeed, that list only blockquote bug is one I experienced as well. A fix would be very welcome. Thanks!

@philippkuehn
Copy link
Contributor

philippkuehn commented Dec 13, 2021

Haha. I was more trying to say that I don’t like to split the list 😅

It seems like a lot of work for a case that has only been reported once.

@holtwick
Copy link
Author

holtwick commented Dec 13, 2021

You don't have to split the list. You showed a selection that started and ended inside the list. Click on blockquote did nothing. I consider this to instead extend the scope to the whole list (<ol> if you like instead of <li>) as it does, if one end of the selection is outside the list.

I think it's the details that make a text editor great 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug The issue or pullrequest is related to a bug
Projects
None yet
Development

No branches or pull requests

2 participants