Skip to content

[Feature] Collapsible/Expandable sequence of small events #518

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 4 commits into
base: main
Choose a base branch
from

Conversation

tyreseluo
Copy link
Contributor

PR content

By default, a series of two or more adjacent small events should be collapsed into an abbreviated view that takes up much less vertical space. This will make it easier for the user to focus on important content like text and image messages from other users, rather than less important metadata.

Related PR

@tyreseluo tyreseluo force-pushed the feature/collapsible_expandable_sequence_of_small_events_#118 branch from d3b0dd8 to 2bc04ee Compare June 13, 2025 10:42
@tyreseluo tyreseluo marked this pull request as ready for review June 17, 2025 08:02
@ZhangHanDong ZhangHanDong added the waiting-on-author This issue is waiting on the original author for a response label Jun 17, 2025
@tyreseluo
Copy link
Contributor Author

It has been optimized to the summarize function, but I still don't think it's a good way to introduce too much code, but it's currently a huge improvement in performance, and UI rendering over the previous code.

@tyreseluo tyreseluo added waiting-on-review This issue is waiting to be reviewed and removed waiting-on-author This issue is waiting on the original author for a response labels Jun 19, 2025
@tyreseluo
Copy link
Contributor Author

Note: change label to waitting on review.

Copy link
Member

@kevinaboos kevinaboos left a comment

Choose a reason for hiding this comment

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

I appreciate your attempt at this, but unfortunately the fundamental approach here just cannot work.

We can't possibly iterate over the entire timeline multiple times (with lots of huge allocations) in order to pre-calculate a conversion between the regular timeline indexes and the "grouped" indexes. That is just unbelievably inefficient -- imagine if the timeline had millions of items in it. Not to mention, it's even worse because you're doing it on every draw routine.

The good news is that you don't have to do that at all! You can make this algorithm much, much simpler, without the need to pre-determine the set of grouped indexes.

All you have to do is maintain a range of indexes that are grouped together, and as you iterate over each timeline item, you will determine whether each next item you encounter is a candidate to be combined into that group or not. If it is able to be made part of the group (e.g., it is a smallstateevent-like item), then you extend that range to include that item's index, and draw that item as part of the group. If it cannot be part of that group (e.g., it's a message), then the group ends there, and that item will be drawn as an individual regular timeline.

The overall idea is that you never want to look at more items in the timeline than you absolutely have to. We should only ever look at the items/indexes that the PortalList asks for (via next_visible_item()`).


In addition, when you're redesigning this, please move as much of the DSL components and Rust code into a separate module. The RoomScreen is already too big.

@kevinaboos kevinaboos added waiting-on-author This issue is waiting on the original author for a response and removed waiting-on-review This issue is waiting to be reviewed labels Jun 20, 2025
@tyreseluo
Copy link
Contributor Author

oh, yes, thanks. good idea to do this.

@tyreseluo tyreseluo force-pushed the feature/collapsible_expandable_sequence_of_small_events_#118 branch from 3df9ccc to 714c309 Compare July 17, 2025 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on-author This issue is waiting on the original author for a response
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants