liveblocks
Resolving a Thread Leaves the Comment Styling in the Editor
Describe the bug
When I resolve a comment that works correctly but the styling of the line stays until a refresh
To Reproduce
Steps to reproduce the behavior:
- Add LiveBlocks to Lexical Editor
<RoomProvider id="dev-123">
<LiveblocksPlugin>
<ClientSideSuspense fallback={null}>
<ThreadsOverlay />
</ClientSideSuspense>
</LiveblocksPlugin>
</RoomProvider>
Where ThreadsOverlay is taken from the docs
2. Add a comment to a piece of text
3. Resolve the comment and notice that the highlight is still there and you can see an empty popover
Expected behavior Once a comment is resolved all indications that there's a comment should be removed too
Illustrations
https://github.com/user-attachments/assets/4cca561c-457d-4b0c-970a-36f0f0c9390a
Environment (please complete the following information):
- @liveblocks/[email protected]
- @liveblocks/[email protected]
- @liveblocks/[email protected]
- Brave Version 1.68.128 Chromium: 127.0.6533.73
Hi @francistogram, thank you for reporting this issue! This issue came up internally too a few weeks ago - we had a discussion but struggled to come up with an API that felt intuitive and didn't complicate our existing API. I will first try to briefly explain why the issue happens and then outline what we've discussed internally (on which we'd love your feedback!)
We cache all threads that are fetched via useThreads in memory and the LiveblocksPlugin only renders thread marks/annotations for threads that are present in this cache. A thread is removed (or marked as deleted) from cache only when the thread gets deleted. Marking a thread as resolved doesn't delete the thread from cache but simply updates a property of the thread in cache. That is why the editor displays the highlight also for resolved threads once if they are present in the cache.
One of the solutions we are discussing is adding a data-resolved attribute to a thread annotation/node to represent that the associated thread is resolved. With the solution, you could style a resolved thread so that it doesn't contain any style. Something similar to:
.lb-lexical-thread-mark[data-resolved] {
all: unset;
}
We are also considering other solutions like allowing users to pass a threads prop to LiveblocksPlugin which would be the source of truth to display thread highlight on the editor (instead of looking for all cached threads) or a showResolved prop that dictates if resolved threads should have a highlight on the editor on not. So far, we are leaning towards the first solution (adding a data-resolved attribute) because it is the quite simple to implement for users and doesn't complicate the API. But, we do also plan to spend some more time thinking about the latter solutions. What do you think?
I like the data-resolved attribute solution. Keeps things simple and I think makes the implementation from the user's side much simpler
As for the showResolved prop not sure you'd need this. Just want to save you from over engineering and thinking about other products
- Notion, on resolve simply disappears and no confirmation. As far as I know there's no way to see resolved comments ever
- Google Docs, on resolve hides the comment but there's also a section to see old / resolved comments
I guess my point is with the inline comments I don't know if it really makes sense to ever show it but I could see being useful to keep track of all of the comment history
Hey @nimeshnayaju just wanted to check and see if you had a rough timeline on when you'd have a fix live for this?
We're just trying to decide if we should wait for this to be completed or build something ourselves
Hi @francistogram, I meant to update you earlier but got distracted by other things - sincere apologies! We took some time to understand the effort required for this and I have separated some time next week to work on the issue and publish a fix as soon as possible. I will update you very soon!
Hi @francistogram, I wanted to provide an update regarding this issue - I've created two PRs today to outline two potential solutions to the problem. Here are the two PRs if you are interested: https://github.com/liveblocks/liveblocks/pull/1932 and https://github.com/liveblocks/liveblocks/pull/1933.
The goal is to decide on a solution, get it reviewed and merged by Monday. I'll keep you posted on any updated! Have an amazing weekend!
Hi @francistogram, we had another discussion today and we concluded that this is in fact a bug, and we wish to be opinionated about hiding resolved threads from the editor and our FloatingThreads and AnchoredThreads component. We've published the bug fix to the 2.7.1 release.
On your end, you'll only have to upgrade to this new version and resolved threads should be hidden by default. In future, we may provide ways to disable this default behaviour. In the meantime, we'll keep brainstorming ways to make some of these behaviour more predictable. Thank you so much for reporting the issue - let us know if you get a chance to test the fix!
Perfect, makes sense to me! Thanks again for the fix and the great communication 🙏