Skip to content

Don't render deleted messages #236

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

andrii-i
Copy link
Collaborator

@andrii-i andrii-i commented Jun 18, 2025

Problem

Currently when message is deleted it is still rendered in UI as (message deleted). It seems to not bring any useful information and increase visual clutter.

Proposed solution

Do not render deleted messages.

Before this PR:

Screenshot 2025-06-18 at 5 21 05 PM
Screenshot 2025-06-18 at 5 21 18 PM
Screenshot 2025-06-18 at 5 22 10 PM

After this PR:

Screenshot 2025-06-18 at 5 24 29 PM
Screenshot 2025-06-18 at 5 24 33 PM
Screenshot 2025-06-18 at 5 25 03 PM

@andrii-i andrii-i added the enhancement New feature or request label Jun 18, 2025
Copy link
Contributor

Binder 👈 Launch a Binder on branch andrii-i/jupyter-chat/no_show_deleted_msgs

@andrii-i andrii-i force-pushed the no_show_deleted_msgs branch from 43d0317 to 5ace066 Compare June 18, 2025 23:40
@ellisonbg
Copy link
Collaborator

Nice! Could you create another screenshot to show the view before the messages are deleted?

@andrii-i
Copy link
Collaborator Author

andrii-i commented Jun 19, 2025

@ellisonbg it was already present in "Problem" section unless I misunderstand. I moved it to the "Proposed solution" section and added "Before this PR:" note just before the screenshot for more visibility.

@ellisonbg
Copy link
Collaborator

Clarification: I was hoping you could show the messages before the user hit the delete button to delete the messages in the first place. Looking for before and after the user hits delete, rather than only before and after the PR.

@andrii-i
Copy link
Collaborator Author

andrii-i commented Jun 19, 2025

@ellisonbg thank you for the clarification, updated screenshots based on it.

@brichet
Copy link
Collaborator

brichet commented Jun 19, 2025

Thanks @andrii-i for working on this.

I think this is a matter of opinion, and agree that in the context of a conversation with an AI it doesn't make sense to keep this track.
In a regular use of the chat, I always find it frustrating when I saw an information and cannot retrieve any track of it in the history (like in Slack I think). In most of the others chat that I know, the message deleted is kept (gitter, zulip, whatsapp...).

Since this PR doesn't actually remove the message from the shared document (only avoid displaying it), I would add a user setting for it.

@andrii-i
Copy link
Collaborator Author

andrii-i commented Jun 19, 2025

Thank you for looking into this @brichet. I agree that this is a matter of opinion, that both approaches have tradeoffs, and indeed believe that approach presented in this PR in the context of a conversation with an AI makes sense.

In a regular use of the chat, I always find it frustrating when I saw an information and cannot retrieve any track of it in the history (like in Slack I think). In most of the others chat that I know, the message deleted is kept (gitter, zulip, whatsapp...).

Since this PR doesn't actually remove the message from the shared document (only avoid displaying it), I would add a user setting for it.

Would you be open to merging this PR and then following up with another PR later on the settings? Another way to deal with being able to retrieve messages could be be wiring up undo/redo so users can undo deleted messages to recover them.

@andrii-i
Copy link
Collaborator Author

@brichet I've created #237 as a placeholder for task on rendering deleted messages setting

@andrii-i andrii-i force-pushed the no_show_deleted_msgs branch from 0d4ebbc to e6eac31 Compare June 19, 2025 18:06
@brichet
Copy link
Collaborator

brichet commented Jun 19, 2025

@andrii-i what I meant was not being able to retrieve the message (currently the content is deleted anyway and I think it's fine), but rather being able to know that message has been deleted. The current behavior looks fine for me.
The settings I mentioned could be only a boolean, whether to display or not the deleted messages, to let users choose the behavior.

@andrii-i
Copy link
Collaborator Author

andrii-i commented Jun 20, 2025

@brichet makes sense, I've updated #237 description to track the need of adding rendering of deleted messages as a boolean setting. Are we good to merge this PR and follow up with another PR on the setting?

@brichet
Copy link
Collaborator

brichet commented Jun 20, 2025

@brichet makes sense, I've updated #237 description to track the need of adding rendering of deleted messages as a boolean setting. Are we good to merge this PR and follow up with another PR on the setting?

Why not adding this settings in this PR ? It doesn't seem to fix anything broken, it's only a matter of opinion...

@ellisonbg
Copy link
Collaborator

I would like to think a bit more about this and do some reading in the UX/HCI literature. Looks like there are some subtle UX questions here. The existing research tends to focus mostly on human-human chat, and there are definitely new dimensions when you add AI. I have definitely run into situations where having the "This message was deleted" was really painful, but there are other situations where it may make more sense. It isn't obvious that the two modes are a per-person preference, but may be more contextual. Some other ideas to explore:

  • We could handle it differently if the deleted message was an AI message versus your own human one.
  • We could include information about who did the deleting "Brian deleted this message".
  • We could show the "This message was deleted" for a certain amount of time and then have it go away for good.
  • We could show the "This message was deleted" but have an additional interaction for dismissing that for good.
  • We could collapse multiple consecutive "This message was deleted" into a single "These messages were deleted" to take up less space (@brichet as an initial start, are you open to this simplification?)

Cheers

@brichet
Copy link
Collaborator

brichet commented Jun 20, 2025

  • We could collapse multiple consecutive "This message was deleted" into a single "These messages were deleted" to take up less space

👍 IMO it's a better user experience than the current one when consecutive messages are deleted.

  • We could handle it differently if the deleted message was an AI message versus your own human one.

I don't have a strong opinion on it, and maybe we could remove them from the document, if we think it doesn't bring any information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants