Skip to content

Introduce a room thread list (wip)#5299

Open
stefanceriu wants to merge 10 commits intodevelopfrom
stefan/roomThreadList
Open

Introduce a room thread list (wip)#5299
stefanceriu wants to merge 10 commits intodevelopfrom
stefan/roomThreadList

Conversation

@stefanceriu
Copy link
Member

@stefanceriu stefanceriu commented Mar 23, 2026 •
edited
Loading

This series of patches introduces a new screen that works with a new RustSDK API to render a Room's list of threads.

Requires matrix-org/matrix-rust-sdk#6335

@stefanceriu stefanceriu requested a review from a team as a code owner March 23, 2026 16:08
@stefanceriu stefanceriu requested review from pixlwave and removed request for a team March 23, 2026 16:08
@stefanceriu stefanceriu added the pr-wip for anything that isn't ready to ship and will be enabled at a later date label Mar 23, 2026
pixlwave
pixlwave approved these changes Mar 23, 2026
View reviewed changes
Copy link
Member

@pixlwave pixlwave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exciting to see this 👏

ElementX/Sources/Services/Room/RoomSummary/RoomEventStringBuilder.swift
static func pinnedEventStringBuilder(userID: String) -> Self {
RoomEventStringBuilder(stateEventStringBuilder: .init(userID: userID,
shouldDisambiguateDisplayNames: false),
shouldDisambiguateDisplayNames: true),
Copy link
Member

@pixlwave pixlwave Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intentional?

Copy link
Member Author

@stefanceriu stefanceriu Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.. you said:

It’s for the stable IDs and we only need it when switching from one timeline (e.g. live) to another (e.g. detached) so that SwiftUI animations don’t go nuts. Outside of that it adds needless work.

but it seems to be used to build the string for the pinned events banner, and now for thread list events. Element Web disambiguates them too:

Screenshot 2026-03-24 at 08 25 55

So I left these here to remember to have a conversation about it: I'm inclined to always default this to true, the extra work is neglijable.

PreviewTests/Sources/__Snapshots__/PreviewTests/roomThreadListScreen.iPhone-en-GB-0.png
Copy link
Member

@pixlwave pixlwave Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not massively important but I wonder if:
a) We should use monospacedDigits on the count
b) We should reserve a minimum amount of space so that the small avatars remain aligned for at least e.g. 1-99.

Copy link
Member Author

@stefanceriu stefanceriu Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me have a play and see how this looks irl

@stefanceriu stefanceriu force-pushed the stefan/roomThreadList branch from 25be4aa to eae3748 Compare March 24, 2026 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pixlwave pixlwave pixlwave approved these changes

Assignees

No one assigned

Labels

pr-wip for anything that isn't ready to ship and will be enabled at a later date

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@stefanceriu @pixlwave