Skip to content

msglist: Show sender only when needed #303

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

Merged
merged 4 commits into from
Sep 12, 2023
Merged

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Sep 11, 2023

Fixes: #175

This will make things more convenient when we add another field
on these which MessageWithSender will want to consult.
Soon these will only sometimes show the sender.
Copy link
Collaborator

@chrisbobbe chrisbobbe left a comment

Choose a reason for hiding this comment

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

Great, LGTM! Merging.

messages: List.generate(100, (i) => eg.streamMessage(id: 950 + i, sender: eg.selfUser)));
check(itemCount(tester)).equals(101);
messages: List.generate(200, (i) => eg.streamMessage(id: 950 + i, sender: eg.selfUser)));
check(itemCount(tester)).equals(201);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Huh, interesting: I guess when there are a lot of consecutive short messages with the same sender and recipient, we now save a lot of vertical space, so this test needs more messages to cover as much vertical space as it wants to.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, exactly. Without this change, these messages take up so little vertical space that the message list promptly decides it needs to fetch more older messages, disrupting the sequence this test was going for.

@chrisbobbe chrisbobbe merged commit 7834dce into zulip:main Sep 12, 2023
@gnprice gnprice deleted the pr-same-sender branch September 12, 2023 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Coalesce same-sender messages
2 participants