-
Notifications
You must be signed in to change notification settings - Fork 742
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
#2782: Collapse server ACLs update events #5249
Conversation
Matrix SDKIntegration Tests Results:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this PR will be really appreciated once in production.
Just a remark about a class name, else LGTM!
@@ -82,7 +83,7 @@ class MergedHeaderItemFactory @Inject constructor(private val activeSessionHolde | |||
event: TimelineEvent, | |||
eventIdToHighlight: String?, | |||
requestModelBuild: () -> Unit, | |||
callback: TimelineEventController.Callback?): MergedMembershipEventsItem_? { | |||
callback: TimelineEventController.Callback?): DefaultMergedEventsItem_? { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK for the renaming, but maybe something like SimilarEventsMergedItem
would be more appropriate. Default
prefix is reserved for default implementation of interfaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I renamed the class to MergedSimilarEventsItem
to be consistent with existing MergedRoomCreationItem
. Is that okay?
...in/java/im/vector/app/features/home/room/detail/timeline/helper/TimelineDisplayableEvents.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update!
Type of change
Content
Merge of multiple successive received server ACLs updates in a room into a single collapsable/expandable item.
Motivation and context
See #2782
Screenshots / GIFs
Tests
Tested devices
Checklist