diff --git a/ISSUE_4417_COMMENT.md b/ISSUE_4417_COMMENT.md new file mode 100644 index 00000000000..372b4118196 --- /dev/null +++ b/ISSUE_4417_COMMENT.md @@ -0,0 +1,41 @@ + + +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. diff --git a/ISSUE_4417_REPLY.md b/ISSUE_4417_REPLY.md new file mode 100644 index 00000000000..29425018525 --- /dev/null +++ b/ISSUE_4417_REPLY.md @@ -0,0 +1,15 @@ + + +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. diff --git a/ISSUE_4417_REPLY_MX2.md b/ISSUE_4417_REPLY_MX2.md new file mode 100644 index 00000000000..84f8bb1170f --- /dev/null +++ b/ISSUE_4417_REPLY_MX2.md @@ -0,0 +1,7 @@ + + +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. diff --git a/ISSUE_4417_REPLY_VOG.md b/ISSUE_4417_REPLY_VOG.md new file mode 100644 index 00000000000..28e3e9c5839 --- /dev/null +++ b/ISSUE_4417_REPLY_VOG.md @@ -0,0 +1,11 @@ + + +@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. diff --git a/ISSUE_DRAFT.md b/ISSUE_DRAFT.md new file mode 100644 index 00000000000..386ded19c04 --- /dev/null +++ b/ISSUE_DRAFT.md @@ -0,0 +1,77 @@ +# [Enhancement] Collapse runs of deleted (redacted) messages in the timeline + + + +## 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? diff --git a/PR_DRAFT.md b/PR_DRAFT.md new file mode 100644 index 00000000000..90f304c53cc --- /dev/null +++ b/PR_DRAFT.md @@ -0,0 +1,138 @@ +# 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.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. diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt index bd0b093ed53..5ca97e2b5ec 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt +++ b/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, diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemGroupedEventsRow.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemGroupedEventsRow.kt index b41e66ae82a..865542a5dbe 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemGroupedEventsRow.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/TimelineItemGroupedEventsRow.kt @@ -19,12 +19,14 @@ import io.element.android.features.messages.impl.R import io.element.android.features.messages.impl.timeline.TimelineEvent import io.element.android.features.messages.impl.timeline.TimelineRoomInfo import io.element.android.features.messages.impl.timeline.aGroupedEvents +import io.element.android.features.messages.impl.timeline.aRedactedMessagesGroupedEvents import io.element.android.features.messages.impl.timeline.aTimelineRoomInfo import io.element.android.features.messages.impl.timeline.components.event.TimelineItemEventContentView import io.element.android.features.messages.impl.timeline.components.group.GroupHeaderView import io.element.android.features.messages.impl.timeline.components.layout.ContentAvoidingLayoutData import io.element.android.features.messages.impl.timeline.components.receipt.ReadReceiptViewState import io.element.android.features.messages.impl.timeline.components.receipt.TimelineItemReadReceiptView +import io.element.android.features.messages.impl.timeline.groups.isRedactedMessagesGroup import io.element.android.features.messages.impl.timeline.model.TimelineItem import io.element.android.features.messages.impl.timeline.protection.TimelineProtectionEvent import io.element.android.features.messages.impl.timeline.protection.TimelineProtectionState @@ -146,12 +148,18 @@ private fun TimelineItemGroupedEventsRowContent( }, ) { Column(modifier = modifier.animateContentSize()) { + val count = timelineItem.events.size + // A group made entirely of redacted events is a collapsed run of deleted messages + // (element-web style); anything else is the regular run of room state changes. For the + // redacted case we show only the count: the SDK does not expose who performed the redaction, + // and showing the original authors would be misleading. + val headerText = if (timelineItem.isRedactedMessagesGroup()) { + pluralStringResource(R.plurals.screen_room_timeline_redacted_messages, count, count) + } else { + pluralStringResource(R.plurals.screen_room_timeline_state_changes, count, count) + } GroupHeaderView( - text = pluralStringResource( - id = R.plurals.screen_room_timeline_state_changes, - count = timelineItem.events.size, - timelineItem.events.size - ), + text = headerText, isExpanded = isExpanded, isHighlighted = !isExpanded && timelineItem.events.any { it.isEvent(focusedEventId) }, onClick = onExpandGroupClick, @@ -256,3 +264,31 @@ internal fun TimelineItemGroupedEventsRowContentCollapsePreview() = ElementPrevi eventSink = {}, ) } + +@PreviewsDayNight +@Composable +internal fun TimelineItemRedactedMessagesGroupPreview() = ElementPreview { + // A collapsed run of deleted messages, shown as a single "N deleted messages" header. + TimelineItemGroupedEventsRowContent( + isExpanded = false, + onExpandGroupClick = {}, + timelineItem = aRedactedMessagesGroupedEvents(count = 11), + timelineMode = Timeline.Mode.Live, + timelineRoomInfo = aTimelineRoomInfo(), + timelineProtectionState = aTimelineProtectionState(), + focusedEventId = null, + isLastOutgoingMessage = false, + displayThreadSummaries = false, + onClick = {}, + onLongClick = {}, + onLinkLongClick = {}, + inReplyToClick = {}, + onUserDataClick = {}, + onLinkClick = {}, + onReactionClick = { _, _ -> }, + onReactionLongClick = { _, _ -> }, + onMoreReactionsClick = {}, + onReadReceiptClick = {}, + eventSink = {}, + ) +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/groups/Groupability.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/groups/Groupability.kt index 837692ae6f3..4da83ba2734 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/groups/Groupability.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/groups/Groupability.kt @@ -42,6 +42,14 @@ import io.element.android.libraries.matrix.api.timeline.item.event.StickerConten import io.element.android.libraries.matrix.api.timeline.item.event.UnableToDecryptContent import io.element.android.libraries.matrix.api.timeline.item.event.UnknownContent +/** + * Return true when every event in the group is a redacted (deleted) message, i.e. the group is a + * collapsed run of deleted messages rather than the usual run of room state changes. Used to pick + * the group header label. An empty group is not considered a redacted group. + */ +internal fun TimelineItem.GroupedEvents.isRedactedMessagesGroup(): Boolean = + events.isNotEmpty() && events.all { it.content is TimelineItemRedactedContent } + /** * Return true if the Event can be grouped in a collapse/expand block * When [canBeGrouped] returns a value, [canBeDisplayedInBubbleBlock] MUST return the opposite value. diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/groups/TimelineItemGrouper.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/groups/TimelineItemGrouper.kt index 6af4f8be6a7..f3f5a599139 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/groups/TimelineItemGrouper.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/groups/TimelineItemGrouper.kt @@ -12,6 +12,7 @@ import androidx.annotation.VisibleForTesting import dev.zacsweers.metro.Inject import dev.zacsweers.metro.SingleIn import io.element.android.features.messages.impl.timeline.model.TimelineItem +import io.element.android.features.messages.impl.timeline.model.event.TimelineItemRedactedContent import io.element.android.libraries.di.RoomScope import io.element.android.libraries.matrix.api.core.UniqueId import kotlinx.collections.immutable.toImmutableList @@ -47,7 +48,8 @@ class TimelineItemGrouper { if (currentGroup.isNotEmpty()) { result.addGroup(groupIds, currentGroup) } - return result + // Finally, fold runs of consecutive deleted messages into a single group (element-web style). + return result.collapseRedactedRuns() } } @@ -89,3 +91,51 @@ private fun MutableMap.getOrPutGroupId(timelineItems: List.collapseRedactedRuns(): List { + val result = mutableListOf() + val run = mutableListOf() + + fun flushRun() { + when { + run.isEmpty() -> Unit + run.size < MIN_REDACTED_RUN_SIZE -> result.addAll(run) + else -> result.add( + TimelineItem.GroupedEvents( + id = computeGroupIdWith(run.first()), + events = run.reversed().toImmutableList(), + aggregatedReadReceipts = run.flatMap { it.readReceiptState.receipts }.toImmutableList(), + ) + ) + } + run.clear() + } + + for (item in this) { + if (item is TimelineItem.Event && item.content is TimelineItemRedactedContent) { + run.add(item) + } else { + flushRun() + result.add(item) + } + } + flushRun() + return result +} diff --git a/features/messages/impl/src/main/res/values/localazy.xml b/features/messages/impl/src/main/res/values/localazy.xml index 173a4e78ad6..e4790feba0f 100644 --- a/features/messages/impl/src/main/res/values/localazy.xml +++ b/features/messages/impl/src/main/res/values/localazy.xml @@ -79,6 +79,10 @@ "Show more" "Show reactions summary" "New" + + "%1$d removed message" + "%1$d removed messages" + "%1$d room change" "%1$d room changes" diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/CollapseRedactedRunsTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/CollapseRedactedRunsTest.kt new file mode 100644 index 00000000000..4e437ca44f7 --- /dev/null +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/CollapseRedactedRunsTest.kt @@ -0,0 +1,167 @@ +/* + * Copyright (c) 2026 Element Creations Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.features.messages.impl.timeline + +import com.google.common.truth.Truth.assertThat +import io.element.android.features.messages.impl.timeline.components.receipt.aReadReceiptData +import io.element.android.features.messages.impl.timeline.groups.collapseRedactedRuns +import io.element.android.features.messages.impl.timeline.groups.computeGroupIdWith +import io.element.android.features.messages.impl.timeline.model.TimelineItem +import io.element.android.features.messages.impl.timeline.model.TimelineItemReadReceipts +import io.element.android.features.messages.impl.timeline.model.event.TimelineItemRedactedContent +import io.element.android.libraries.matrix.api.core.EventId +import io.element.android.libraries.matrix.api.core.UniqueId +import kotlinx.collections.immutable.toImmutableList +import org.junit.Test + +class CollapseRedactedRunsTest { + private fun redacted(id: String) = aTimelineItemEvent( + eventId = EventId(id), + content = TimelineItemRedactedContent, + ) + + @Test + fun `collapses a run of three or more redacted events into a single group`() { + val r1 = redacted("\$R1") + val r2 = redacted("\$R2") + val r3 = redacted("\$R3") + val result = listOf(r1, r2, r3).collapseRedactedRuns() + assertThat(result).hasSize(1) + val group = result.single() as TimelineItem.GroupedEvents + // Stored oldest-first (the input is newest-first), like the existing grouper. + assertThat(group.events.map { it.eventId }).containsExactly( + EventId("\$R3"), + EventId("\$R2"), + EventId("\$R1"), + ).inOrder() + // Group id is derived from the newest event of the run so it survives older pagination. + assertThat(group.id).isEqualTo(computeGroupIdWith(r1)) + } + + @Test + fun `leaves a run shorter than three as individual tiles`() { + val r1 = redacted("\$R1") + val r2 = redacted("\$R2") + val result = listOf(r1, r2).collapseRedactedRuns() + assertThat(result).containsExactly(r1, r2).inOrder() + } + + @Test + fun `keeps surrounding events and only collapses the redacted run between them`() { + val newest = aTimelineItemEvent(eventId = EventId("\$NEW")) + val r1 = redacted("\$R1") + val r2 = redacted("\$R2") + val r3 = redacted("\$R3") + val oldest = aTimelineItemEvent(eventId = EventId("\$OLD")) + val result = listOf(newest, r1, r2, r3, oldest).collapseRedactedRuns() + assertThat(result).hasSize(3) + assertThat(result.first()).isEqualTo(newest) + assertThat(result.last()).isEqualTo(oldest) + assertThat(result[1]).isInstanceOf(TimelineItem.GroupedEvents::class.java) + assertThat((result[1] as TimelineItem.GroupedEvents).events).hasSize(3) + } + + @Test + fun `a day separator breaks a run so each side is evaluated on its own`() { + val r1 = redacted("\$R1") + val r2 = redacted("\$R2") + val r3 = redacted("\$R3") + val sep = aTimelineItemDaySeparator() + val r4 = redacted("\$R4") + val r5 = redacted("\$R5") + val result = listOf(r1, r2, r3, sep, r4, r5).collapseRedactedRuns() + // First run (3) collapses, the separator passes through, the second run (2) stays as tiles. + assertThat(result).hasSize(4) + assertThat(result.first()).isInstanceOf(TimelineItem.GroupedEvents::class.java) + assertThat(result[1]).isEqualTo(sep) + assertThat(result[2]).isEqualTo(r4) + assertThat(result[3]).isEqualTo(r5) + } + + @Test + fun `does not collapse a run interrupted by a non-redacted event`() { + val r1 = redacted("\$R1") + val r2 = redacted("\$R2") + val between = aTimelineItemEvent(eventId = EventId("\$MID")) + val r3 = redacted("\$R3") + val result = listOf(r1, r2, between, r3).collapseRedactedRuns() + assertThat(result).containsExactly(r1, r2, between, r3).inOrder() + } + + @Test + fun `collapses a long run into a single group keeping every event`() { + val run = (1..5).map { redacted("\$R$it") } + val result = run.collapseRedactedRuns() + assertThat(result).hasSize(1) + assertThat((result.single() as TimelineItem.GroupedEvents).events).hasSize(5) + } + + @Test + fun `collapses each qualifying run independently`() { + val firstRun = (1..3).map { redacted("\$A$it") } + val separator = aTimelineItemEvent(eventId = EventId("\$SEP")) + val secondRun = (1..3).map { redacted("\$B$it") } + val result = (firstRun + separator + secondRun).collapseRedactedRuns() + assertThat(result).hasSize(3) + assertThat(result[0]).isInstanceOf(TimelineItem.GroupedEvents::class.java) + assertThat(result[1]).isEqualTo(separator) + assertThat(result[2]).isInstanceOf(TimelineItem.GroupedEvents::class.java) + // The two groups get distinct ids so their expand state does not bleed into one another. + assertThat((result[0] as TimelineItem.GroupedEvents).id) + .isNotEqualTo((result[2] as TimelineItem.GroupedEvents).id) + } + + @Test + fun `collapses a run that sits at the very end of the list`() { + val newest = aTimelineItemEvent(eventId = EventId("\$NEW")) + val run = (1..3).map { redacted("\$R$it") } + val result = (listOf(newest) + run).collapseRedactedRuns() + assertThat(result).hasSize(2) + assertThat(result.first()).isEqualTo(newest) + assertThat(result.last()).isInstanceOf(TimelineItem.GroupedEvents::class.java) + } + + @Test + fun `leaves a single redacted event untouched`() { + val r1 = redacted("\$R1") + assertThat(listOf(r1).collapseRedactedRuns()).containsExactly(r1) + } + + @Test + fun `aggregates the read receipts of every event in the collapsed group`() { + fun redactedWithReceipt(id: String, receiptIndex: Int) = aTimelineItemEvent( + eventId = EventId(id), + content = TimelineItemRedactedContent, + readReceiptState = TimelineItemReadReceipts( + receipts = listOf(aReadReceiptData(receiptIndex)).toImmutableList(), + ), + ) + val run = listOf( + redactedWithReceipt("\$R1", 0), + redactedWithReceipt("\$R2", 1), + redactedWithReceipt("\$R3", 2), + ) + val group = run.collapseRedactedRuns().single() as TimelineItem.GroupedEvents + assertThat(group.aggregatedReadReceipts).hasSize(3) + } + + @Test + fun `an existing grouped item breaks the run and is passed through untouched`() { + val stateGroup = aGroupedEvents(id = UniqueId("state")) + val run = (1..3).map { redacted("\$R$it") } + val result = (run + stateGroup).collapseRedactedRuns() + assertThat(result).hasSize(2) + assertThat(result.first()).isInstanceOf(TimelineItem.GroupedEvents::class.java) + assertThat(result.last()).isEqualTo(stateGroup) + } + + @Test + fun `is a no-op on an empty list`() { + assertThat(emptyList().collapseRedactedRuns()).isEmpty() + } +} diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt index af37fb61ef5..7b01ca8b206 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt @@ -43,6 +43,7 @@ import io.element.android.libraries.matrix.api.timeline.Timeline import io.element.android.libraries.matrix.api.timeline.item.event.EventReaction import io.element.android.libraries.matrix.api.timeline.item.event.ReactionSender import io.element.android.libraries.matrix.api.timeline.item.event.Receipt +import io.element.android.libraries.matrix.api.timeline.item.event.RedactedContent import io.element.android.libraries.matrix.api.timeline.item.virtual.VirtualTimelineItem import io.element.android.libraries.matrix.test.AN_EVENT_ID import io.element.android.libraries.matrix.test.AN_EVENT_ID_2 @@ -366,6 +367,28 @@ class TimelinePresenterTest { } } + @Test + fun `present - collapses a run of three or more redacted events into a single group`() = runTest { + val timeline = FakeTimeline( + timelineItems = flowOf( + (0 until 3).map { index -> + MatrixTimelineItem.Event( + uniqueId = UniqueId("redacted_$index"), + event = anEventTimelineItem(eventId = EventId("\$R$index"), content = RedactedContent), + ) + } + ), + ) + val presenter = createTimelinePresenter(timeline = timeline) + presenter.test { + val state = consumeItemsUntilPredicate { it.timelineItems.size == 1 }.last() + val group = state.timelineItems.single() + assertThat(group).isInstanceOf(TimelineItem.GroupedEvents::class.java) + assertThat((group as TimelineItem.GroupedEvents).events).hasSize(3) + cancelAndIgnoreRemainingEvents() + } + } + @Test fun `present - reaction ordering`() = runTest { val timelineItems = MutableStateFlow(emptyList()) diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/groups/GroupabilityTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/groups/GroupabilityTest.kt new file mode 100644 index 00000000000..fde84ca77a0 --- /dev/null +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/groups/GroupabilityTest.kt @@ -0,0 +1,60 @@ +/* + * Copyright (c) 2026 Element Creations Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.features.messages.impl.timeline.groups + +import com.google.common.truth.Truth.assertThat +import io.element.android.features.messages.impl.timeline.aGroupedEvents +import io.element.android.features.messages.impl.timeline.aTimelineItemEvent +import io.element.android.features.messages.impl.timeline.model.TimelineItem +import io.element.android.features.messages.impl.timeline.model.event.TimelineItemRedactedContent +import io.element.android.libraries.matrix.api.core.EventId +import io.element.android.libraries.matrix.api.core.UniqueId +import kotlinx.collections.immutable.persistentListOf +import kotlinx.collections.immutable.toImmutableList +import org.junit.Test + +class GroupabilityTest { + private fun redacted(id: String) = aTimelineItemEvent( + eventId = EventId(id), + content = TimelineItemRedactedContent, + ) + + private fun groupOf(vararg events: TimelineItem.Event) = TimelineItem.GroupedEvents( + id = UniqueId("group"), + events = events.toList().toImmutableList(), + aggregatedReadReceipts = persistentListOf(), + ) + + @Test + fun `a group made only of redacted events is a redacted messages group`() { + val group = groupOf(redacted("\$R1"), redacted("\$R2"), redacted("\$R3")) + assertThat(group.isRedactedMessagesGroup()).isTrue() + } + + @Test + fun `a group of state changes is not a redacted messages group`() { + // aGroupedEvents builds a run of state events. + assertThat(aGroupedEvents().isRedactedMessagesGroup()).isFalse() + } + + @Test + fun `a group mixing redacted and non-redacted events is not a redacted messages group`() { + val group = groupOf(redacted("\$R1"), aTimelineItemEvent(eventId = EventId("\$E")), redacted("\$R2")) + assertThat(group.isRedactedMessagesGroup()).isFalse() + } + + @Test + fun `an empty group is not a redacted messages group`() { + val group = TimelineItem.GroupedEvents( + id = UniqueId("group"), + events = persistentListOf(), + aggregatedReadReceipts = persistentListOf(), + ) + assertThat(group.isRedactedMessagesGroup()).isFalse() + } +} diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/groups/TimelineItemGrouperTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/groups/TimelineItemGrouperTest.kt index 726646f5e92..5d9466787f1 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/groups/TimelineItemGrouperTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/groups/TimelineItemGrouperTest.kt @@ -15,6 +15,7 @@ import io.element.android.features.messages.impl.timeline.aTimelineItemReactions import io.element.android.features.messages.impl.timeline.model.ReadReceiptData import io.element.android.features.messages.impl.timeline.model.TimelineItem import io.element.android.features.messages.impl.timeline.model.TimelineItemReadReceipts +import io.element.android.features.messages.impl.timeline.model.event.TimelineItemRedactedContent import io.element.android.features.messages.impl.timeline.model.event.TimelineItemStateEventContent import io.element.android.features.messages.impl.timeline.model.virtual.aTimelineItemDaySeparatorModel import io.element.android.libraries.designsystem.components.avatar.anAvatarData @@ -169,4 +170,30 @@ class TimelineItemGrouperTest { // Then assertThat(actualGroupId).isEqualTo(expectedGroupId) } + + @Test + fun `a run of three or more redacted events is collapsed into a single group as a final step`() { + val r0 = aGroupableItem.copy(id = UniqueId("r0"), content = TimelineItemRedactedContent) + val r1 = aGroupableItem.copy(id = UniqueId("r1"), content = TimelineItemRedactedContent) + val r2 = aGroupableItem.copy(id = UniqueId("r2"), content = TimelineItemRedactedContent) + val result = sut.group(listOf(r0, r1, r2)) + assertThat(result).isEqualTo( + listOf( + TimelineItem.GroupedEvents( + id = computeGroupIdWith(r0), + // Stored oldest-first (the input is newest-first), like the other groups. + events = listOf(r2, r1, r0).toImmutableList(), + aggregatedReadReceipts = emptyList().toImmutableList(), + ) + ) + ) + } + + @Test + fun `a run of fewer than three redacted events is left untouched`() { + val r0 = aGroupableItem.copy(id = UniqueId("r0"), content = TimelineItemRedactedContent) + val r1 = aGroupableItem.copy(id = UniqueId("r1"), content = TimelineItemRedactedContent) + val result = sut.group(listOf(r0, r1)) + assertThat(result).isEqualTo(listOf(r0, r1)) + } } diff --git a/screenshots/00-before.jpg b/screenshots/00-before.jpg new file mode 100644 index 00000000000..1b4e0cdf952 Binary files /dev/null and b/screenshots/00-before.jpg differ diff --git a/screenshots/01-toggle.jpg b/screenshots/01-toggle.jpg new file mode 100644 index 00000000000..8119aaf6f7f Binary files /dev/null and b/screenshots/01-toggle.jpg differ diff --git a/screenshots/02-collapsed.jpg b/screenshots/02-collapsed.jpg new file mode 100644 index 00000000000..41b645fdbd9 Binary files /dev/null and b/screenshots/02-collapsed.jpg differ diff --git a/screenshots/03-expanded.jpg b/screenshots/03-expanded.jpg new file mode 100644 index 00000000000..7d0482ba36b Binary files /dev/null and b/screenshots/03-expanded.jpg differ diff --git a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt index 433237cd13a..8102ede70e9 100644 --- a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt +++ b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistPreviewTest.kt @@ -156,6 +156,7 @@ class KonsistPreviewTest { "TimelineItemGroupedEventsRowContentCollapsePreview", "TimelineItemGroupedEventsRowContentExpandedPreview", "TimelineItemImageViewHideMediaContentPreview", + "TimelineItemRedactedMessagesGroupPreview", "TimelineItemVideoViewHideMediaContentPreview", "TimelineItemVoiceViewUnifiedPreview", "TimelineVideoWithCaptionRowPreview",