Collapse runs of deleted messages in the timeline#7111
Conversation
Collapse a run of three or more consecutive deleted messages into a single
expandable group, reusing the existing state-change grouping (the same way
element-web does it) instead of showing one "Message removed" placeholder per
deletion. The header shows the count only ("N deleted messages"); the Rust SDK
does not expose who performed the redaction, so we don't attribute it.
On by default. Runs shorter than three are left as individual placeholders, and
day dividers and read receipts are preserved (a run is broken by any
non-redacted item, so a day is never emptied).
|
Thank you for your contribution! Here are a few things to check in the PR to ensure it's reviewed as quickly as possible:
|
github-actions
Bot
added
the
Z-Community-PR
|
A quick note on the failing checks, in case it helps triage. The unit test job fails on one screenshot test, for the new preview added here, The other snapshot failures in that job ( |
|
Thanks for the PR. I'll test it, but the code seems fine at a first glance. Could you record the screenshot tests? (seems like we wrote both messages at the same time, the way to record screenshots is explained at the last point of the automatic PR-from-fork message above). Also, next time please try following the provided PR template so it's easier to check whether you tested compatibility or whether the PR was built with AI (I assume it was?). I'll mark the PR as blocked so I don't forget about adding the strings to localazy, once that's done I'll remove the label. |
|
I've updated the description to follow the template, including the step by step testing notes. On the screenshot tests: I'm not set up to record the Paparazzi goldens locally, and I can't run the record workflow from my fork. Would you be able to apply the |
Ah, that might be a side-effect of the new rules enforced by #7098. Sadly, we can't run workflows on your fork from our repo or using the GH workflow, so we'll have to fix the issue caused by the |
|
Just to double check, which kind of issue do you have when trying to record screenshots? Github won't let you even try to start the action? Do you have any kind of error log? |
|
Github does not let me start it at all, so there is no run log to share. Actions are enabled in the fork settings (permissions set to allow all actions), but no workflows are registered for the fork: the REST API Happy to try again once the checkout action fix lands on your side. |
|
Oh ok. f that's the case then I don't think the checkout changes will help, to be honest, it seems more like a GH hiccup/issue. Worst case scenario, we can merge this to a branch of ours and we run the record screenshots flow there. The reason why we need to record this using GH is different OS or even Java runtimes render screenshots slightly differently, so if we don't use the same GH runner we end up rewriting tons of screenshots for no good reason. |
|
Sounds like a plan, no rush at all. Thanks for jumping on it so fast. Just shout if you need anything from me. |
jmartinesp
left a comment
jmartinesp
left a comment
There was a problem hiding this comment.
I tested the feature and it works quite well, thank you!
Code-wise though, I believe this should be part of the TimelineGrouper implementation, it could even be the last action performed after we've grouped other state/membership events. Unless there is some reason we shouldn't do this?
| // Collapse runs of consecutive deleted messages into a single expandable group instead of | ||
| // showing each as its own placeholder, like element-web. Day dividers stay untouched, so a | ||
| // day is never emptied. | ||
| val visibleTimelineItems = remember(timelineItems) { | ||
| timelineItems.collapseRedactedRuns().toImmutableList() | ||
| } |
There was a problem hiding this comment.
Should this happen as part of TimelineItemGrouper.group instead? It seems like it would belong there and it's a bit weird to have it as a post-processing and synchronous step in the presenter.
Run the collapse as the final step of TimelineItemGrouper.group() instead of a separate pass in the presenter, as suggested in review. Kept as its own finalization step rather than via canBeGrouped() so the group stays all redacted and the count-only header is preserved.
|
Good call, done. It now lives in The only reason it started out outside the grouper was caution: this was my first change in this area, so I bolted it on after the grouper's output to keep it self contained and not poke at something so central. Now that it works and you're happy with it, putting it in the grouper where it belongs makes more sense. On whether to just fold redacted into
|
2 participants
Relates to #4417. The approach here was discussed and agreed with the team on that issue.
Content
Collapses runs of 3 or more consecutive deleted (redacted) messages in the timeline into a single, tappable group, instead of one "Message removed" tile per deletion. It reuses the existing grouped events mechanism (the same one behind the "N room changes" group), so a run renders as a single centred line, "N removed messages", that expands and collapses on tap. On by default, with no setting. Runs of one or two deletions are left as individual tiles, and day separators and read receipts are preserved.
Motivation and context
Element Web has collapsed runs of redacted messages for a long time and it is a real readability win in busy, heavily moderated rooms. Element X had nothing like it, so large deletion runs dominate the scrollback. This is the recurring ask in #4417.
Why this is not a repeat of #4434: that PR hid redacted events outright, which can mask moderation and broke day dividers and read receipts. This one collapses instead of hiding (nothing is removed, the count is shown, the full run is one tap away), a run is broken by any non-redacted item so a day separator is never swallowed, and the run's read receipts are aggregated onto the group the same way state-change groups already do.
The header shows only a count because the Rust SDK surfaces a redaction as a bare
MsgLikeKind.Redactedwith no redacter, so the app cannot tell who removed the messages; showing the original authors would be misleading. If the SDK later exposes the redacter, this header is the natural place to add "removed by X" like Web.Screenshots / GIFs
The collapsed state is covered by a Composable Preview,
TimelineItemRedactedMessagesGroupPreview, so it shows up in the file diff and the screenshot tests. The header looks like the existing "N room changes" group, with the label "N removed messages".Tests
Tested devices
Checklist