Collapse runs of deleted messages in the timeline

ISSUE_4417_COMMENT.md

@@ -0,0 +1,41 @@
+<!-- Comment to post on element-hq/element-x-android#4417 -->
+
+I had a go at this. Instead of fully *hiding* redacted messages (as in #4434), I went with
+collapsing runs of 3 or more consecutive deletions into a single tappable group, reusing
+the existing state-change grouping UI:
+
+> **Deleted 11 messages from Alice, Bob**
+
+Tap to expand or collapse; runs of 1 or 2 stay inline. Same idea as Element Web.
+
+The header lists the original authors, up to 3 names. If more than 3 people are involved
+it shows the first few and then "and others", e.g. "Deleted 30 messages from Alice, Bob,
+Carol and others".
+
+I think this avoids the two reasons #4434 was declined:
+
+- It collapses rather than hides. Deletions stay visible and auditable (count plus original
+  authors, with the full run one tap away), so nothing is removed and moderation activity
+  isn't masked.
+- Day dividers and read receipts are preserved. A run is broken by any non-redacted item,
+  so day separators are never swallowed, and the run's read receipts are aggregated onto
+  the group exactly as the existing state-change grouping already does.
+
+It's opt-in behind an advanced setting, off by default.
+
+One honest limitation: I can show the original authors of the deleted messages, but not
+who deleted them. The Rust SDK surfaces `MsgLikeKind.Redacted` as a bare object with no
+redacter or reason (and `EventTimelineItem` exposes no redaction metadata either).
+
+That's also why I didn't build the smarter behaviour floated on #4434 (collapse self/DM
+deletions but keep moderator/admin removals visible): without the redacter, the app can't
+tell a self-deletion apart from a moderator/admin redaction. So "removed by the moderator"
+and any self-vs-moderator logic need the SDK to expose that data first. It would be a clean
+follow-up, and this header is the obvious place for it.
+
+The implementation is ready and green (pure timeline transform, no SDK changes, unit tests
+plus previews; I have before/after/expanded screenshots to share).
+
+Before I open a PR: would you accept this app-side, or is the team's position still that it
+belongs in the Rust SDK? Happy to go either way. I can open the PR as-is, or treat this as
+reference UX for an SDK-side version.

ISSUE_4417_REPLY.md

@@ -0,0 +1,15 @@
+<!-- Reply to mxandreas on element-hq/element-x-android#4417 -->
+
+Thanks, glad it looks good!
+
+Agreed on making it the default with no setting, I'll drop the toggle and have it always on.
+
+On showing who removed the messages: I agree that's nicer, and you're right that showing the original authors can be misleading, it reads as if they removed their own messages when it might have been a moderator. The catch is that the redacter doesn't seem to be available to the app right now. As far as I can tell the Rust SDK gives a redaction as a bare `MsgLikeKind.Redacted` with no `redacted_because` / redacter, so unlike Web we just can't tell who did the removal.
+
+So for a first version I'd keep the header to a plain count, like "11 deleted messages", with no names or avatars. That sidesteps the misleading attribution and still gets rid of the wall. Worth noting the timeline doesn't show who removed a message today either, each "Message removed" just sits under its original sender, so count-only doesn't lose anything that's shown now, it only tidies up the clutter. Once the SDK exposes the redacter we can show "removed by X" like Web, and this header is the obvious place for it.
+
+For what it's worth, I personally lean towards showing a bit more (in my own build I went with author names, small avatars and a toggle, and I like it), but I don't want to argue the point with the team. I'm happy to go with count-only and default-on as you prefer.
+
+Does count-only work for you as a v1? And would you be open to the SDK change later so we can show who removed them?
+
+On iOS: I'd keep this PR to Android for now, that's where I'm working. I can't pick up the iOS side myself, but the logic is small and easy to port, so I'm happy to write up the approach for whoever works on that app.

ISSUE_4417_REPLY_MX2.md

@@ -0,0 +1,7 @@
+<!-- Reply to mxandreas on #4417 confirming the plan -->
+
+Thanks a lot, I appreciate the quick and detailed feedback on this.
+
+I'll make the changes accordingly: count-only header, on by default, no toggle or config flag, and keeping the day separators. Once that's done I'll open the PR and link it here.
+
+And good to know about the plan to collapse state events across dates as well, I'll keep this consistent with that so it fits in nicely later.

ISSUE_4417_REPLY_VOG.md

@@ -0,0 +1,11 @@
+<!-- Reply to vog on element-hq/element-x-android#4417 -->
+
+@vog
+
+> Would it be feasible to make this configurable? ... day separators could be "swallowed" as well, at least for days which contain nothing but removed messages
+
+Thanks! Good point for notes-style rooms.
+
+For this change I'd lean towards keeping the day separators though. Not swallowing them is intentional: it's what keeps the day dividers (and read receipts) correct, and that's part of why this works without touching the SDK. The direction here is also to ship it as the default with no setting, so I'd rather not add a config flag for it.
+
+Swallowing days that contain only removed messages could be a nice separate follow-up later. I'd just keep it out of this one to stay focused.

ISSUE_DRAFT.md

@@ -0,0 +1,77 @@
+# [Enhancement] Collapse runs of deleted (redacted) messages in the timeline
+
+<!--
+  For element-meta (product/design enhancement), filed before the implementation PR,
+  same flow as e.g. the timeline multi-select request #3255.
+  Platform: Element X (Android first; applies to iOS too).
+-->
+
+## The problem
+
+When a bunch of messages are deleted in a row (a moderator clearing spam, a bot purge, or
+a user removing a burst of their own messages), the timeline renders **one "Message
+removed" placeholder per deletion**. A 30-message spam wave becomes 30 grey placeholders
+the user has to scroll through to reach real content.
+
+This is most painful in exactly the rooms where it matters most: large, public, actively
+moderated communities.
+
+## Prior art
+
+Collapsing runs of redacted messages is a long-established feature in Element **Web**, and
+it's a clear readability win. Element X has no equivalent today.
+
+Related: #4417 ("hide / collapse redacted messages").
+
+## Proposed behaviour
+
+Collapse a run of **3 or more consecutive deleted messages** into a single, tappable group,
+reusing the same grouping UI already used for state-change events (joins, leaves, name
+changes). One header line replaces the wall:
+
+> **Deleted 30 messages from Alice, Bob, Carol and others**
+
+- Tap to expand the full run, tap again to collapse.
+- Runs of 1 or 2 stay as normal inline "Message removed" markers (no wall to hide).
+- The header shows the **count**, the **original authors** (up to 3 names, then "and
+  others"), and a small leading **avatar stack**.
+
+### Why "collapse", not "hide"
+
+Deletions stay visible and auditable; anyone can expand to see how many and from whom. We
+just stop them dominating the scrollback. This sidesteps the moderation concern that fully
+hiding redactions could mask abuse.
+
+### Opt-in
+
+Proposed as an **advanced setting, off by default** ("Collapse deleted messages"), so the
+default timeline is unchanged. It could be on by default to match Web if product prefers;
+that's a one-line change.
+
+## Known limitation: showing *who* deleted
+
+Element Web can sometimes attribute the deletion ("removed by the moderator"). On Element X
+we **can't do this today**. The Rust SDK FFI surfaces a redaction as `MsgLikeKind.Redacted`,
+a bare object with **no redacter and no reason**. So we reliably know the **original
+author** of each deleted message, but not the account that deleted it, and not whether it
+was a self-deletion or a moderator/admin action.
+
+Showing the original authors is the accurate version shippable today. Attributing the
+redacter is a natural follow-up once the SDK exposes redaction metadata, and the header
+above is the obvious place to add "removed by @mod" with no UI rework.
+
+## Scope / status
+
+- The Android implementation is ready (pure timeline-item transform, no SDK or sync
+  changes, reuses the existing `GroupedEvents`; unit tests plus previews; behind the
+  advanced setting).
+- Happy to open the PR as soon as there's product/design buy-in on the approach, the label
+  wording, and the default.
+
+## Questions for product / design
+
+1. Is a threshold of **3** to collapse OK?
+2. Default **off** (advanced setting) or **on** (matching Web)?
+3. Label wording: "Deleted N messages from A, B, C and others", and confirming we drop
+   per-author counts (they read as noise in testing).
+4. Avatar stack in the header: keep it, or go count-only?

PR_DRAFT.md

@@ -0,0 +1,138 @@
+# Collapse runs of deleted messages in the timeline
+
+<!-- PR title (= release note): Collapse runs of deleted messages in the timeline -->
+
+Relates to #4417. The approach here was discussed and agreed with the team on that issue.
+
+## Summary
+
+When a bunch of messages get deleted in a row (a moderator clearing spam, a bot purge, or
+someone removing a burst of their own messages), the timeline shows one "Message removed"
+placeholder for each one. Ten deletions become ten grey tiles you have to scroll past to
+get back to the actual conversation.
+
+This PR collapses runs of 3 or more consecutive deleted messages into a single, tappable
+group. It reuses the mechanism that already groups state-change events (joins, leaves,
+name changes), so one line replaces the wall:
+
+> 11 removed messages
+
+Tap it to expand the whole run, tap again to collapse. Runs of one or two deletions are
+left alone, so the occasional "Message removed" still reads fine in context. It is on by
+default, with no setting, the same as the existing state-change grouping.
+
+## What it looks like
+
+In place of a run of consecutive "Message removed" tiles, the timeline shows a single
+centred line, "N removed messages", styled like the existing "N room changes" group. Tap
+it to expand the run back into the individual tiles, tap again to collapse.
+
+## Motivation
+
+Collapsing runs of redacted messages is a long-standing feature in Element Web, and it's
+a real readability win in busy, heavily-moderated rooms. Element X has nothing like it
+today, so large deletion runs just dominate the scrollback. This is the recurring ask in
+#4417.
+
+### Why this isn't #4434 again
+
+The earlier PR (#4434) simply hid redacted events and was declined, for two fair reasons:
+hiding deletions outright can mask moderation activity, and rewriting the timeline in the
+app can break day dividers and read receipts.
+
+This one is different on both points:
+
+- It collapses instead of hiding. Deletions stay visible and auditable, the count is shown
+  and the full run is one tap away. Nothing is removed.
+- Day dividers and read receipts survive (details below). 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 it.
+
+## Why the header shows only a count
+
+The header reads "N removed messages" and intentionally does not say who the messages
+belonged to or who removed them.
+
+Element Web can sometimes attribute the deletion ("removed by the moderator"), but that
+data isn't available to the app today: the Rust SDK surfaces a redaction as a bare
+`MsgLikeKind.Redacted` with no `redacted_because` / redacter, so we can't tell who
+performed the removal. Showing the original authors instead would be misleading, since it
+reads as if they removed their own messages when it may have been a moderator. So a plain
+count is the honest thing to show for now.
+
+If the SDK later exposes the redacter, this header is the obvious place to add "removed by
+X" like Web, with no UI rework. (Discussed and agreed with the team on #4417.)
+
+## How it works
+
+It reuses the existing grouping machinery rather than adding a new UI primitive.
+
+A pure helper, `List<TimelineItem>.collapseRedactedRuns()`, walks the already-built
+timeline, finds maximal runs of consecutive redacted events
+(`TimelineItemRedactedContent`), and replaces any run of `MIN_REDACTED_RUN_SIZE` (3) or
+more with a `TimelineItem.GroupedEvents`, the same type used for state-change collapsing.
+Shorter runs and everything else pass through untouched.
+
+A couple of things the #4434 review flagged, and how they're handled:
+
+- Day dividers: the run only accumulates consecutive redacted events. Anything else (a
+  real message, a state change, or a day separator) flushes the current run and is added
+  unchanged, so a run never spans or absorbs a day boundary.
+- Read receipts: the receipts of the collapsed events are aggregated onto the
+  `GroupedEvents`, which already renders them, exactly like state-change groups.
+
+`TimelinePresenter` applies the transform to the timeline it exposes, and the redacted
+group reuses the existing `GroupHeaderView` and `TimelineItemGroupedEventsRow` with a
+count label. Expansion state reuses the existing `rememberSaveable`, so there is no new
+state plumbing. Since it runs on the already-resolved timeline item list, it's a thin,
+contained change with nothing in the SDK or sync path touched.
+
+## Tests
+
+- `CollapseRedactedRunsTest` (12): the pure transform. Empty list, no redactions, a run
+  below the threshold (kept inline), a run at/over the threshold (collapsed), several
+  independent runs with distinct group ids, a run at the end of the list, runs broken by a
+  real message / a day separator / an existing group, read-receipt aggregation, and events
+  stored oldest-first with a stable group id.
+- `GroupabilityTest`: redacted-group detection (all redacted, state changes, mixed, empty).
+- `TimelinePresenterTest`: a run of redacted events collapses into a single group.
+- A preview for the collapsed header, added to the Konsist preview-name exceptions.
+
+## Notes for reviewers
+
+- New string is in `localazy.xml` (EN source); I didn't touch `translations.xml`, since
+  Localazy fills those. No towncrier entry and no DCO sign-off, per this repo's
+  conventions; the PR title is the release note.
+- The change is scoped to `features/messages/impl`. No public API or SDK changes, and no
+  new setting.
+- iOS: this PR is Android only. I don't work in Swift, so I haven't written the iOS side,
+  but the logic is small and self-contained. The porting notes below map it to the iOS
+  equivalents.
+
+### Porting to iOS
+
+The Android change is a post-processing pass over the already-built timeline plus a header
+label, with no SDK or data changes, so it should mirror cleanly. The shape:
+
+1. Add the equivalent of `collapseRedactedRuns`: walk the assembled timeline items, collect
+   maximal runs of consecutive redacted items, and replace any run of 3 or more with the
+   existing collapsible group that already backs the "N room changes" group. Leave shorter
+   runs and every non-redacted item, day separators included, untouched, so a run never
+   crosses a day boundary.
+2. When a group is made entirely of redacted items (the equivalent of
+   `isRedactedMessagesGroup()`), render the header as a count-only plural, "N removed
+   messages", and keep the existing chevron and expand or collapse behaviour.
+3. Aggregate the collapsed items' read receipts onto the group, the same way the
+   state-change group already does.
+4. Add the matching localized plural (the iOS counterpart of
+   `screen_room_timeline_redacted_messages`).
+5. The same SDK limitation applies: the redaction arrives without a redacter, so the header
+   stays count-only on iOS too. If the SDK later exposes it, that header is where "removed
+   by X" would go, as on Android.
+
+## Open questions
+
+1. Threshold: 3 felt right to me (2 isn't really a wall). Happy to change the constant or
+   expose it.
+2. The redacter ("removed by X") is left for a follow-up once the SDK exposes it, as
+   agreed on #4417.

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt

@@ -21,6 +21,7 @@ import io.element.android.features.messages.impl.timeline.model.TimelineItemRead
 import io.element.android.features.messages.impl.timeline.model.TimelineItemThreadInfo
 import io.element.android.features.messages.impl.timeline.model.anAggregatedReaction
 import io.element.android.features.messages.impl.timeline.model.event.TimelineItemEventContent
+import io.element.android.features.messages.impl.timeline.model.event.TimelineItemRedactedContent
 import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemStateEventContent
 import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemTextContent
 import io.element.android.features.messages.impl.timeline.model.virtual.aTimelineItemDaySeparatorModel
@@ -250,6 +251,23 @@ internal fun aGroupedEvents(
     )
 }
 
+internal fun aRedactedMessagesGroupedEvents(
+    id: UniqueId = UniqueId("redacted_group"),
+    count: Int = 4,
+): TimelineItem.GroupedEvents {
+    val events = (0 until count).map { index ->
+        aTimelineItemEvent(
+            eventId = EventId("\$redacted_$index"),
+            content = TimelineItemRedactedContent,
+        )
+    }
+    return TimelineItem.GroupedEvents(
+        id = id,
+        events = events.toImmutableList(),
+        aggregatedReadReceipts = persistentListOf(),
+    )
+}
+
 internal fun aTimelineRoomInfo(
     name: String = ROOM_NAME,
     isDm: Boolean = false,