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

Confusion and frustration when dealing with empty list items (aka "Bullet List Limbo") #3128

Closed
2 tasks done
domjost opened this issue Aug 26, 2022 · 21 comments · Fixed by #4210
Closed
2 tasks done
Labels
Type: Bug The issue or pullrequest is related to a bug
Milestone

Comments

@domjost
Copy link

domjost commented Aug 26, 2022

What’s the bug you are facing?

Exiting lists in TipTap can be done by hitting Enter, which is standard behavior in all editors. But the way this editor deals with Backspace in an empt list item suggests that users can exit lists also using Backspace. The problem is that Backspace only visually removes the bullet, suggesting to the user that they are about to write a paragraph, when in fact they are still writing bullet items.

This leads to lots of unwanted formatting issues, confusion and frustrating editing experience.

Which browser was this experienced in? Are any special extensions installed?

Safari, Version 15.6.1 (17613.3.9.1.16)
macOS, 12.5.1

How can we reproduce the bug on our side?

I used https://tiptap.dev/examples/default and recorded the issue as well as my expected behavior here: https://www.loom.com/share/001d5b52095a4caa889db51c58a44932

This is what causes "Bullet List Limbo".

  1. You need a list with at least two items, the last one being empty:

CleanShot 2022-08-26 at 18 37 57@2x

  1. Place the cursor on the second item (the empty one), and hit backspace. What you get looks like this:

CleanShot 2022-08-26 at 18 38 31@2x

  1. At this point, lots of users believe they have exited the bulleted list and continue as if it where a paragraph:

CleanShot 2022-08-26 at 18 39 50@2x

  1. After finishing the paragraph, they hit enter and are surprised to see this:

CleanShot 2022-08-26 at 18 40 25@2x

At this stage the editing experience goes in all sorts of wrong directions, because they struggle to get the editor to do what they want it to do. In other words: the state of the editor is no more in sync with the mind of the user, causing immense frustration and believing the editor is broken.

Can you provide a CodeSandbox?

No response

What did you expect to happen?

Hitting backspace on an empty list item should completely remove the row and move the cursor to the item above.

In this scenario...
CleanShot 2022-08-26 at 18 43 56@2x

hitting backspace should result in this:
CleanShot 2022-08-26 at 18 44 38@2x

and not this:
CleanShot 2022-08-26 at 18 44 23@2x

Explained here in the Loom, and comparing to Apple notes: https://www.loom.com/share/001d5b52095a4caa889db51c58a44932?t=141

Anything to add? (optional)

This behaviour might be intended, or technically correct, but it causes lots of our users to believe that the editor is broken. I assume because it breaks with their learned experience from other editors.

This issue might be small, but once a user is in this state of "Bulleted List Limbo", a lot of other issues occur with indentation and formatting, resulting in a horrendous writing experience.

Fixing this would make our lives so much more enjoyable :-)

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. 💖 (on behalf of Doist, makers of Todoist and Twist)
@domjost domjost added the Type: Bug The issue or pullrequest is related to a bug label Aug 26, 2022
@bdbch
Copy link
Contributor

bdbch commented Sep 1, 2022

Thanks for this issue. Will look into this!

@github-actions
Copy link
Contributor

github-actions bot commented Dec 1, 2022

This issue is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the Info: Stale The issue or pullrequest has not been updated in a while and might be stale label Dec 1, 2022
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 9, 2022
@domjost
Copy link
Author

domjost commented Dec 9, 2022

Hi @bdbch, is there a chance that this will be addressed?

@bdbch
Copy link
Contributor

bdbch commented Dec 13, 2022

Hey. Yes, we just weren't able to get to it yet. But we have similar issues with the Delete key behavior which all stem from how Prosemirror handles content joining / deletion. I started to work on fixes for the delete key behavior but lists in general are a different topic.

I can't give you a specific ETA for a fix right now but I'll see that we prioritize it higher on our board.

@bdbch bdbch reopened this Dec 13, 2022
@github-actions github-actions bot removed the Info: Stale The issue or pullrequest has not been updated in a while and might be stale label Dec 14, 2022
@bdbch bdbch added this to the 2.0.0 Release milestone Feb 24, 2023
@bdbch bdbch modified the milestones: 2.0.0 Release, 2.1.0 Release Mar 28, 2023
@Nantris
Copy link
Contributor

Nantris commented May 15, 2023

Thanks for your great work on TipTap @bdbch. I wonder if this is indeed planned for fixing in the 2.1.0 release as it's currently categorized? I saw 2.1.0 already has a few release candidates but there's no PRs mentioning this issue.

This is a pretty substantial usability problem and I wonder if we could get some sort of rough ETA on it?

@Nantris
Copy link
Contributor

Nantris commented May 18, 2023

I tested the 2.1.0-rc.4 release and this issue seems to be unchanged in that release, but all of the related issues are still marked for the 2.1.0 milestone. Will these issues really be fixed in 2.1.0?

@Nantris
Copy link
Contributor

Nantris commented May 23, 2023

Note: These findings are for 2.1.0-rc.4 but they appear to generalize to 2.0.3 as well

The limbo problem appears to be far more serious than I initially believed. When copying/pasting you can corrupt the editor such that undo is not possible and throws errors doing something similar to what I do in this video with copy/paste. In this video it didn't error and corrupt the editor state, but I made it happen in my prior test.

Screencast.2023-05-23.18.31.22.mp4

This really needs some comment from the maintainers at this point. I'm afraid to actually rely on TipTap because it's not clear to me that this problem will be resolved (or even can be resolved?) and has a high likelihood to irreversibly corrupt data in the event of failures.

The most broken states look like this:

image

Failed undo error:
Uncaught RangeError: Invalid content for node listItem: <bulletList(listItem(paragraph, bulletList(listIte at NodeType.checkContent (index.js:2200:1)

With great care it's often possible to restore the document to a valid state using a combination of Backspace, Delete, and Shift+Tab but the undo history is irreversibly broken and any content which may have been mangled during the problematic list pasting will result in permanent data loss.

image

None of these problems are apparent in the basic ProseMirror demo: https://prosemirror.net/examples/basic/

Edit: Some but not all of these problems are apparent in the basic ProseMirror demo. There have become too many separate variances for me to remember which occurs how and where exactly between this comment and my second one.

@bdbch

@Nantris
Copy link
Contributor

Nantris commented May 23, 2023

Further findings:

I will add any new findings to this comment.

One:

It looks like in some cases this case result in li improperly having two ul nodes nested within it:

image

Two:

You can grab that same interstitial area between nodes and drag it to between some existing listItem (easiest using DropCursor) and that will produce slightly broken output - nothing as egregious as when copying/pasting it seems.

Three:

Backspacing away expanded selections on lists fails. It should unindent the children after the selection until they become the new top-level listItems but instead it leaves behind garbage listItems and bulletLists

Before:
image

After:
image

Additionally, the resulting list cannot be removed via further backspacing by the user:

Screencast.2023-05-30.18.16.41.mp4

Four:

Probably related to three, if there's a child list under the list you're trying to backspace or delete, the selection seems to toggle to a node selection of the parent, and then back to the child item again.

cannot-backspace-or-delete-lists-if-there-is-a-child-list.mp4

@Nantris
Copy link
Contributor

Nantris commented May 30, 2023

Friendly bump.

Will there be any replies to the list issues, or is the community on its own? Can we please get a modicum of communication on this topic?

I noticed this issue was originally raised by a sponsor almost a year ago.

@Nantris
Copy link
Contributor

Nantris commented May 31, 2023

Some good news at least; I believe the one issue in the video Screencast.2023-05-23.18.31.22.mp4 may be resolved in 2.1.0-rc.8

It appears to be related to some trailing whitespace that's present in the version from my video and maybe not present in newer versions. But I hadn't thought any list changes made it into those versions so perhaps there's some step in reproduction I'm overlooking now.

The issue still does occur for empty listItems though.

@Nantris
Copy link
Contributor

Nantris commented Jun 1, 2023

@marijnh I've been working with TipTap and was surprised by how lists are handled in many circumstances. I originally thought many of these were particular to TipTap and not generalizable to ProseMirror because I had an insufficient understanding of the circumstances required to reproduce the issues.

Would it be possible for you to explain which (if any) of my findings you'd consider to be bugs? I feel like surely the one in the video cannot-backspace-or-delete-lists-if-there-is-a-child-list.mp4 should be considered a bug, right? Backspacing results in a node-level selection and unremovable listItems.

I'd like to gain a better understanding of where design choices end and possible bugs begin so we can get any bugs fixed and then implement our desired behaviors on top of any possible ProseMirror patches that may be in order.

@bdbch
Copy link
Contributor

bdbch commented Aug 5, 2023

I'm coming back to those issues because I'm currently a bit worried about my implementation and what happens with them on future Prosemirror updates.

I added a new comment on Prosemirror (see ProseMirror/prosemirror#370)

It could be that we remove my fixes and try to stay with vanilla Prosemirror to avoid future conflicts or errors introduced with Prosemirror updates.

There is also another issue where Marjin went a bit into why list items are not removed if a range around list items is deleted when there are child list items inside:

ProseMirror/prosemirror#1259

@Nantris
Copy link
Contributor

Nantris commented Oct 26, 2023

Are the examples still running on TipTap v2.0? Them being so out of date makes it hard to test whether bugs are particular to our environment. On the examples it's exceedingly easy to produce the limbo state so I'm guessing it's not updated. Will they ever be updated?

I wanted to report a new limbo state that wasn't reported before, but I can't do that without a bit of extra work/clutter since it requires making a new CodeSandbox each time.

It would be really helpful if the examples explicitly stated the version they're running @bdbch.


Edit: Actually I see 2.1.x still has the list limbo issue in the CodeSandbox. I'm not clear on what the state of this issue is. I thought it was resolved since the #3819 is merged and this issue is closed, but the behavior doesn't appear to be any different than it was six months ago.

The unresolved limbo issue (besides all the others which are apparently also unresolved) is visible in the video below. The paragraphs inside of a listItem are left behind when the listItem has multiple paragraphs inside of it with any form of expanded selection. This occurs even though we manually implemented the apparently unmerged list limbo fixes.

@kalda341 are you supporting multiple paragraphs inside of listItem? Have you encountered this issue and do you have any solution for it?

Unaddressed.limbo.state.mp4

@kalda341
Copy link

@slapbox We technically support multiple paragraphs in a listItem (as in we allow it in our schema) but we don't really have an easy way for a user to get into that state. I just simulated your video by pasting multiple paragraphs into the list item with 2.1.7 + the list fixes and I couldn't replicate your issue.

@dbousamra
Copy link

Any more updates. Would love to have a timeline on this

@Nantris
Copy link
Contributor

Nantris commented Feb 14, 2024

As far as I know this is addressed @dbousamra.

@dbousamra
Copy link

As far as I know this is addressed @dbousamra.

Ahh. I shall try upgrading to latest. I still see same behaviour on the Tiptap demo website though

@Nantris
Copy link
Contributor

Nantris commented Feb 14, 2024

Yeah... I don't know that they've ever updated the version on the site oddly and it's hard for me to say with certainty that it's fixed since we've made a lot of additional modifications - but I'm pretty sure it's fixed in 2.1.x.

@dbousamra
Copy link

dbousamra commented Feb 14, 2024

Yeah... I don't know that they've ever updated the version on the site oddly and it's hard for me to say with certainty that it's fixed since we've made a lot of additional modifications - but I'm pretty sure it's fixed in 2.1.x.

Hmm. Just did a codesandbox with the latest 2.2.2 and it behaves the same as the OP:
https://codesandbox.io/p/sandbox/cocky-bose-7jyf7h

Edit: Apologies. It seems adding the list-keymap extension solves it. Hmm, never seen this extension, time to read up

@Nantris
Copy link
Contributor

Nantris commented Feb 15, 2024

Ah yes I did forget about list-keymap. I think the list changes initially didn't require that but since it constituted a breaking change they were eventually (at least in part) rolled into that extension.

@bdbch
Copy link
Contributor

bdbch commented Feb 15, 2024

Yeah we tried to not fiddle to much with the default Prosemirror list keymap behavior as Marjin already put a lot of thoughts into it.

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

Successfully merging a pull request may close this issue.

5 participants