Skip to content

Fix reactions summary crash #5472

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.

Sign up for GitHub

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
ouchadam merged 2 commits into from
Mar 9, 2022
Merged

Fix reactions summary crash #5472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c24944b
Fix reactions summary crash
ariskotsomitopoulos Mar 9, 2022
84b3f63
Format code
ariskotsomitopoulos Mar 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/5463.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix reactions summary crash
8 changes: 7 additions & 1 deletion ...src/main/java/im/vector/app/features/home/room/detail/timeline/TimelineEventController.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import im.vector.app.features.home.room.detail.timeline.item.DaySeparatorItem
import im.vector.app.features.home.room.detail.timeline.item.DaySeparatorItem_
import im.vector.app.features.home.room.detail.timeline.item.ItemWithEvents
import im.vector.app.features.home.room.detail.timeline.item.MessageInformationData
import im.vector.app.features.home.room.detail.timeline.item.ReactionsSummaryEvents
import im.vector.app.features.home.room.detail.timeline.item.ReadReceiptData
import im.vector.app.features.home.room.detail.timeline.item.ReadReceiptsItem
import im.vector.app.features.home.room.detail.timeline.url.PreviewUrlRetriever
Expand Down Expand Up @@ -415,7 +416,12 @@ class TimelineEventController @Inject constructor(private val dateFormatter: Vec
partialState = partialState,
lastSentEventIdWithoutReadReceipts = lastSentEventWithoutReadReceipts,
callback = callback,
eventsGroup = timelineEventsGroup
eventsGroup = timelineEventsGroup,
reactionsSummaryEvents = ReactionsSummaryEvents(
onAddMoreClicked = { reactionListFactory.onAddMoreClicked(callback, event) },
Copy link
Member

@ganfra ganfra Mar 9, 2022

Choose a reason for hiding this comment

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

I would avoid creating a new data class to be pass everywhere. Would prefer having ReactionsSummaryCallback as interface with empty default implementation. Then let TimelineEventController.Callback inherits this interface and just delegate to reactionListFactory in the case of onShowLess and onShowMore

Copy link
Contributor Author

@ariskotsomitopoulos ariskotsomitopoulos Mar 9, 2022

Choose a reason for hiding this comment

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

This was my first attempt. Let me check if I can enhance it fast for the release.

onShowLessClicked = { reactionListFactory.onShowLessClicked(event.eventId) },
onShowMoreClicked = { reactionListFactory.onShowMoreClicked(event.eventId) }
)
)
modelCache[position] = buildCacheItem(params)
numberOfEventsToBuild++
Expand Down
21 changes: 14 additions & 7 deletions ...src/main/java/im/vector/app/features/home/room/detail/timeline/factory/CallItemFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import im.vector.app.features.home.room.detail.timeline.helper.MessageItemAttrib
import im.vector.app.features.home.room.detail.timeline.item.CallTileTimelineItem
import im.vector.app.features.home.room.detail.timeline.item.CallTileTimelineItem_
import im.vector.app.features.home.room.detail.timeline.item.MessageInformationData
import im.vector.app.features.home.room.detail.timeline.item.ReactionsSummaryEvents
import org.matrix.android.sdk.api.session.Session
import org.matrix.android.sdk.api.session.events.model.EventType
import org.matrix.android.sdk.api.session.room.model.RoomSummary
Expand Down Expand Up @@ -61,7 +62,8 @@ class CallItemFactory @Inject constructor(
highlight = params.isHighlighted,
informationData = informationData,
isStillActive = callEventGrouper.isInCall(),
formattedDuration = callEventGrouper.formattedDuration()
formattedDuration = callEventGrouper.formattedDuration(),
reactionsSummaryEvents = params.reactionsSummaryEvents
)
} else {
null
Expand All @@ -78,7 +80,8 @@ class CallItemFactory @Inject constructor(
highlight = params.isHighlighted,
informationData = informationData,
isStillActive = callEventGrouper.isRinging(),
formattedDuration = callEventGrouper.formattedDuration()
formattedDuration = callEventGrouper.formattedDuration(),
reactionsSummaryEvents = params.reactionsSummaryEvents
)
} else {
null
Expand All @@ -94,7 +97,8 @@ class CallItemFactory @Inject constructor(
highlight = params.isHighlighted,
informationData = informationData,
isStillActive = false,
formattedDuration = callEventGrouper.formattedDuration()
formattedDuration = callEventGrouper.formattedDuration(),
reactionsSummaryEvents = params.reactionsSummaryEvents
)
}
EventType.CALL_HANGUP -> {
Expand All @@ -111,7 +115,8 @@ class CallItemFactory @Inject constructor(
highlight = params.isHighlighted,
informationData = informationData,
isStillActive = false,
formattedDuration = callEventGrouper.formattedDuration()
formattedDuration = callEventGrouper.formattedDuration(),
reactionsSummaryEvents = params.reactionsSummaryEvents
)
}
else -> null
Expand All @@ -133,10 +138,11 @@ class CallItemFactory @Inject constructor(
highlight: Boolean,
isStillActive: Boolean,
formattedDuration: String,
callback: TimelineEventController.Callback?
callback: TimelineEventController.Callback?,
reactionsSummaryEvents: ReactionsSummaryEvents?
): CallTileTimelineItem? {
val userOfInterest = roomSummary.toMatrixItem()
val attributes = messageItemAttributesFactory.create(null, informationData, callback).let {
val attributes = messageItemAttributesFactory.create(null, informationData, callback, reactionsSummaryEvents).let {
CallTileTimelineItem.Attributes(
callId = callId,
callKind = callKind,
Expand All @@ -151,7 +157,8 @@ class CallItemFactory @Inject constructor(
readReceiptsCallback = it.readReceiptsCallback,
userOfInterest = userOfInterest,
callback = callback,
isStillActive = isStillActive
isStillActive = isStillActive,
reactionsSummaryEvents = reactionsSummaryEvents
)
}
return CallTileTimelineItem_()
Expand Down
4 changes: 3 additions & 1 deletion ...ain/java/im/vector/app/features/home/room/detail/timeline/factory/EncryptedItemFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ class EncryptedItemFactory @Inject constructor(private val messageInformationDat
messageContent = event.root.content.toModel<EncryptedEventContent>(),
informationData = informationData,
callback = params.callback,
threadDetails = threadDetails)
threadDetails = threadDetails,
reactionsSummaryEvents = params.reactionsSummaryEvents
)
return MessageTextItem_()
.layout(informationData.messageLayout.layoutRes)
.leftGuideline(avatarSizeProvider.leftGuideline)
Expand Down
5 changes: 3 additions & 2 deletions ...in/java/im/vector/app/features/home/room/detail/timeline/factory/EncryptionItemFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class EncryptionItemFactory @Inject constructor(
}
val algorithm = event.root.content.toModel<EncryptionEventContent>()?.algorithm
val informationData = informationDataFactory.create(params)
val attributes = messageItemAttributesFactory.create(null, informationData, params.callback)
val attributes = messageItemAttributesFactory.create(null, informationData, params.callback, params.reactionsSummaryEvents)

val isSafeAlgorithm = algorithm == MXCRYPTO_ALGORITHM_MEGOLM
val title: String
Expand Down Expand Up @@ -80,7 +80,8 @@ class EncryptionItemFactory @Inject constructor(
itemClickListener = attributes.itemClickListener,
itemLongClickListener = attributes.itemLongClickListener,
reactionPillCallback = attributes.reactionPillCallback,
readReceiptsCallback = attributes.readReceiptsCallback
readReceiptsCallback = attributes.readReceiptsCallback,
reactionsSummaryEvents = attributes.reactionsSummaryEvents
)
)
.highlighted(params.isHighlighted)
Expand Down
7 changes: 4 additions & 3 deletions .../main/java/im/vector/app/features/home/room/detail/timeline/factory/MessageItemFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ class MessageItemFactory @Inject constructor(

if (event.root.isRedacted()) {
// message is redacted
val attributes = messageItemAttributesFactory.create(null, informationData, callback, threadDetails)
val attributes = messageItemAttributesFactory.create(null, informationData, callback, params.reactionsSummaryEvents)
return buildRedactedItem(attributes, highlight)
}

Expand All @@ -177,7 +177,7 @@ class MessageItemFactory @Inject constructor(
}

// always hide summary when we are on thread timeline
val attributes = messageItemAttributesFactory.create(messageContent, informationData, callback, threadDetails)
val attributes = messageItemAttributesFactory.create(messageContent, informationData, callback, params.reactionsSummaryEvents, threadDetails)

// val all = event.root.toContent()
// val ev = all.toModel<Event>()
Expand Down Expand Up @@ -413,7 +413,8 @@ class MessageItemFactory @Inject constructor(
itemClickListener = attributes.itemClickListener,
reactionPillCallback = attributes.reactionPillCallback,
readReceiptsCallback = attributes.readReceiptsCallback,
emojiTypeFace = attributes.emojiTypeFace
emojiTypeFace = attributes.emojiTypeFace,
reactionsSummaryEvents = attributes.reactionsSummaryEvents
)
)
.callback(callback)
Expand Down
2 changes: 2 additions & 0 deletions ...ava/im/vector/app/features/home/room/detail/timeline/factory/TimelineItemFactoryParams.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package im.vector.app.features.home.room.detail.timeline.factory

import im.vector.app.features.home.room.detail.timeline.TimelineEventController
import im.vector.app.features.home.room.detail.timeline.helper.TimelineEventsGroup
import im.vector.app.features.home.room.detail.timeline.item.ReactionsSummaryEvents
import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent

data class TimelineItemFactoryParams(
Expand All @@ -29,6 +30,7 @@ data class TimelineItemFactoryParams(
val partialState: TimelineEventController.PartialState = TimelineEventController.PartialState(),
val lastSentEventIdWithoutReadReceipts: String? = null,
val callback: TimelineEventController.Callback? = null,
val reactionsSummaryEvents: ReactionsSummaryEvents? = null,
val eventsGroup: TimelineEventsGroup? = null
) {

Expand Down
10 changes: 6 additions & 4 deletions .../java/im/vector/app/features/home/room/detail/timeline/factory/VerificationItemFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@ class VerificationItemFactory @Inject constructor(
// If it's not a request ignore this event
// if (refEvent.root.getClearContent().toModel<MessageVerificationRequestContent>() == null) return ignoredConclusion(event, highlight, callback)

val referenceInformationData = messageInformationDataFactory.create(TimelineItemFactoryParams(refEvent))
val referenceInformationData = messageInformationDataFactory.create(TimelineItemFactoryParams(event = refEvent))

val informationData = messageInformationDataFactory.create(params)
val attributes = messageItemAttributesFactory.create(null, informationData, params.callback)
val attributes = messageItemAttributesFactory.create(null, informationData, params.callback, params.reactionsSummaryEvents)

when (event.root.getClearType()) {
EventType.KEY_VERIFICATION_CANCEL -> {
Expand All @@ -100,7 +100,8 @@ class VerificationItemFactory @Inject constructor(
itemClickListener = attributes.itemClickListener,
itemLongClickListener = attributes.itemLongClickListener,
reactionPillCallback = attributes.reactionPillCallback,
readReceiptsCallback = attributes.readReceiptsCallback
readReceiptsCallback = attributes.readReceiptsCallback,
reactionsSummaryEvents = attributes.reactionsSummaryEvents
)
)
.highlighted(params.isHighlighted)
Expand Down Expand Up @@ -133,7 +134,8 @@ class VerificationItemFactory @Inject constructor(
itemClickListener = attributes.itemClickListener,
itemLongClickListener = attributes.itemLongClickListener,
reactionPillCallback = attributes.reactionPillCallback,
readReceiptsCallback = attributes.readReceiptsCallback
readReceiptsCallback = attributes.readReceiptsCallback,
reactionsSummaryEvents = attributes.reactionsSummaryEvents
)
)
.highlighted(params.isHighlighted)
Expand Down
3 changes: 2 additions & 1 deletion ...c/main/java/im/vector/app/features/home/room/detail/timeline/factory/WidgetItemFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ class WidgetItemFactory @Inject constructor(
userOfInterest = userOfInterest,
callback = params.callback,
isStillActive = isCallStillActive,
formattedDuration = ""
formattedDuration = "",
reactionsSummaryEvents = params.reactionsSummaryEvents
)
return CallTileTimelineItem_()
.attributes(attributes)
Expand Down
2 changes: 1 addition & 1 deletion .../im/vector/app/features/home/room/detail/timeline/helper/MessageInformationDataFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class MessageInformationDataFactory @Inject constructor(private val session: Ses
avatarUrl = event.senderInfo.avatarUrl,
memberName = event.senderInfo.disambiguatedDisplayName,
messageLayout = messageLayout,
reactionsSummary = reactionsSummaryFactory.create(event, params.callback),
reactionsSummary = reactionsSummaryFactory.create(event),
pollResponseAggregatedSummary = event.annotations?.pollResponseSummary?.let {
PollResponseData(
myVote = it.aggregatedContent?.myVote,
Expand Down
3 changes: 3 additions & 0 deletions ...a/im/vector/app/features/home/room/detail/timeline/helper/MessageItemAttributesFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import im.vector.app.features.home.room.detail.timeline.MessageColorProvider
import im.vector.app.features.home.room.detail.timeline.TimelineEventController
import im.vector.app.features.home.room.detail.timeline.item.AbsMessageItem
import im.vector.app.features.home.room.detail.timeline.item.MessageInformationData
import im.vector.app.features.home.room.detail.timeline.item.ReactionsSummaryEvents
import org.matrix.android.sdk.api.session.threads.ThreadDetails
import javax.inject.Inject

Expand All @@ -38,6 +39,7 @@ class MessageItemAttributesFactory @Inject constructor(
fun create(messageContent: Any?,
informationData: MessageInformationData,
callback: TimelineEventController.Callback?,
reactionsSummaryEvents: ReactionsSummaryEvents?,
threadDetails: ThreadDetails? = null): AbsMessageItem.Attributes {
return AbsMessageItem.Attributes(
avatarSize = avatarSizeProvider.avatarSize,
Expand All @@ -60,6 +62,7 @@ class MessageItemAttributesFactory @Inject constructor(
emojiTypeFace = emojiCompatFontProvider.typeface,
decryptionErrorMessage = stringProvider.getString(R.string.encrypted_message),
threadDetails = threadDetails,
reactionsSummaryEvents = reactionsSummaryEvents,
areThreadMessagesEnabled = preferencesProvider.areThreadMessagesEnabled()
)
}
Expand Down
29 changes: 16 additions & 13 deletions ...n/java/im/vector/app/features/home/room/detail/timeline/helper/ReactionsSummaryFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class ReactionsSummaryFactory @Inject constructor() {
return eventsRequestingBuild.remove(event.eventId)
}

fun create(event: TimelineEvent, callback: TimelineEventController.Callback?): ReactionsSummaryData {
fun create(event: TimelineEvent): ReactionsSummaryData {
val eventId = event.eventId
val showAllStates = showAllReactionsByEvent.contains(eventId)
val reactions = event.annotations?.reactionsSummary
Expand All @@ -43,21 +43,24 @@ class ReactionsSummaryFactory @Inject constructor() {
}
return ReactionsSummaryData(
reactions = reactions,
showAll = showAllStates,
onShowMoreClicked = {
showAllReactionsByEvent.add(eventId)
onRequestBuild(eventId)
},
onShowLessClicked = {
showAllReactionsByEvent.remove(eventId)
onRequestBuild(eventId)
},
onAddMoreClicked = {
callback?.onAddMoreReaction(event)
}
showAll = showAllStates
)
}

fun onAddMoreClicked(callback: TimelineEventController.Callback?, event: TimelineEvent) {
callback?.onAddMoreReaction(event)
}

fun onShowMoreClicked(eventId: String) {
showAllReactionsByEvent.add(eventId)
onRequestBuild(eventId)
}

fun onShowLessClicked(eventId: String) {
showAllReactionsByEvent.remove(eventId)
onRequestBuild(eventId)
}

private fun onRequestBuild(eventId: String) {
eventsRequestingBuild.add(eventId)
onRequestBuild?.invoke()
Expand Down
13 changes: 10 additions & 3 deletions ...src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsBaseMessageItem.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,18 +121,24 @@ abstract class AbsBaseMessageItem<H : AbsBaseMessageItem.Holder> : BaseEventItem
val showReactionsTextView = createReactionTextView(holder)
if (reactionsSummary.showAll) {
showReactionsTextView.setText(R.string.message_reaction_show_less)
showReactionsTextView.onClick { reactionsSummary.onShowLessClicked() }
showReactionsTextView.onClick {
baseAttributes.reactionsSummaryEvents?.onShowLessClicked?.invoke()
}
} else {
val moreCount = reactions.count() - MAX_REACTIONS_TO_SHOW
showReactionsTextView.text = holder.view.resources.getQuantityString(R.plurals.message_reaction_show_more, moreCount, moreCount)
showReactionsTextView.onClick { reactionsSummary.onShowMoreClicked() }
showReactionsTextView.onClick {
baseAttributes.reactionsSummaryEvents?.onShowMoreClicked?.invoke()
}
}
holder.reactionsContainer.addView(showReactionsTextView)
}
val addMoreReactionsTextView = createReactionTextView(holder)

addMoreReactionsTextView.text = holder.view.context.getDrawableAsSpannable(R.drawable.ic_add_reaction_small)
addMoreReactionsTextView.onClick { reactionsSummary.onAddMoreClicked() }
addMoreReactionsTextView.onClick {
baseAttributes.reactionsSummaryEvents?.onAddMoreClicked?.invoke()
}
holder.reactionsContainer.addView(addMoreReactionsTextView)
holder.reactionsContainer.setOnLongClickListener(baseAttributes.itemLongClickListener)
}
Expand Down Expand Up @@ -180,6 +186,7 @@ abstract class AbsBaseMessageItem<H : AbsBaseMessageItem.Holder> : BaseEventItem

// val memberClickListener: ClickListener?
val reactionPillCallback: TimelineEventController.ReactionPillCallback?
val reactionsSummaryEvents: ReactionsSummaryEvents?

// val avatarCallback: TimelineEventController.AvatarCallback?
val readReceiptsCallback: TimelineEventController.ReadReceiptsCallback?
Expand Down
3 changes: 2 additions & 1 deletion vector/src/main/java/im/vector/app/features/home/room/detail/timeline/item/AbsMessageItem.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ abstract class AbsMessageItem<H : AbsMessageItem.Holder> : AbsBaseMessageItem<H>
val emojiTypeFace: Typeface? = null,
val decryptionErrorMessage: String? = null,
val threadDetails: ThreadDetails? = null,
val areThreadMessagesEnabled: Boolean = false
val areThreadMessagesEnabled: Boolean = false,
override val reactionsSummaryEvents: ReactionsSummaryEvents? = null,
) : AbsBaseMessageItem.Attributes {

// Have to override as it's used to diff epoxy items
Expand Down
Loading