Skip to content
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

[NEW] Group messages from the same user #552

Merged
merged 4 commits into from
Aug 12, 2019

Conversation

Shailesh351
Copy link
Collaborator

Closes #535

Upstream issue: RocketChat#712

Changes:

  • Take in consideration the setting (Message_GroupingPeriod -> default is 900 seconds)
  • When there's a day separator, can't be sequential
  • When a message is a system message (message removed, pinned message, permission changes, etc) don't group
  • Users need to be the same in order to group it

Widechat's Message_GroupingPeriod is 300 seconds

@Shailesh351 Shailesh351 self-assigned this Jul 24, 2019
if (a.senderAlias == b.senderAlias && a.sender?.id == b.sender?.id && !a.isSystemMessage() && !b.isSystemMessage()) {
val date1 = a.getDate()
val date2 = b.getDate()
return date1.isSameDay(date2) && date1.timeInMillis - date2.timeInMillis < messageGroupingPeriod * 1000
Copy link
Collaborator

Choose a reason for hiding this comment

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

its an edge case but, if I send a message at 11:59 PM and then another at 12:01 AM on the next day, I would expect these to group together, but this would not occur given the above logic. Is there a reason for the isSameDay check?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes It will not group message with different days. In the upstream issue it is mentioned that "When there's a day separator, can't be sequential", so logic is based on that.

If you want to group message even if different day then we can remove "isSameDay" condition. But that will not look good because there will be day separator.

What do you think?

Copy link

Choose a reason for hiding this comment

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

fwiw I think as a user I would actually expect/want to see the separator upon the day change, so I don't see the need for getting grouping to work in that case

@bizzbyster
Copy link
Collaborator

message delivery checkmark is only showing up for first message in a group

image

@jaytat0 jaytat0 merged commit 0572ec2 into WideChat:veranda Aug 12, 2019
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.

Group messages from the same user
3 participants