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

Feature/fga/reactions UI improvements #5204

Merged
merged 6 commits into from
Feb 11, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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/5204.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve UI of reactions in timeline, including quick add reaction.
3 changes: 3 additions & 0 deletions library/ui-styles/src/main/res/values/dimens.xml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@
<dimen name="chat_bubble_fixed_size">300dp</dimen>
<dimen name="chat_bubble_corner_radius">12dp</dimen>

<dimen name="chat_reaction_min_height">28dp</dimen>
<dimen name="chat_reaction_min_width">40dp</dimen>

<!-- Onboarding -->
<item name="ftue_auth_gutter_start_percent" format="float" type="dimen">0.05</item>
<item name="ftue_auth_gutter_end_percent" format="float" type="dimen">0.95</item>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,11 @@ internal class UIEchoManager(private val listener: Listener) {
return existingState != sendState
}

fun onLocalEchoCreated(timelineEvent: TimelineEvent): Boolean {
fun onLocalEchoCreated(timelineEvent: TimelineEvent): Boolean {
when (timelineEvent.root.getClearType()) {
EventType.REDACTION -> {
}
EventType.REACTION -> {
EventType.REACTION -> {
val content: ReactionContent? = timelineEvent.root.content?.toModel<ReactionContent>()
if (RelationType.ANNOTATION == content?.relatesTo?.type) {
val reaction = content.relatesTo.key
Expand Down Expand Up @@ -104,8 +104,8 @@ internal class UIEchoManager(private val listener: Listener) {
val updateReactions = existingAnnotationSummary.reactionsSummary.toMutableList()

contents.forEach { uiEchoReaction ->
val existing = updateReactions.firstOrNull { it.key == uiEchoReaction.reaction }
if (existing == null) {
val indexOfExistingReaction = updateReactions.indexOfFirst { it.key == uiEchoReaction.reaction }
if (indexOfExistingReaction == -1) {
// just add the new key
ReactionAggregatedSummary(
key = uiEchoReaction.reaction,
Expand All @@ -117,6 +117,7 @@ internal class UIEchoManager(private val listener: Listener) {
).let { updateReactions.add(it) }
} else {
// update Existing Key
val existing = updateReactions[indexOfExistingReaction]
if (!existing.localEchoEvents.contains(uiEchoReaction.localEchoId)) {
updateReactions.remove(existing)
// only update if echo is not yet there
Expand All @@ -128,7 +129,7 @@ internal class UIEchoManager(private val listener: Listener) {
sourceEvents = existing.sourceEvents,
localEchoEvents = existing.localEchoEvents + uiEchoReaction.localEchoId

).let { updateReactions.add(it) }
).let { updateReactions.add(indexOfExistingReaction, it) }
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1913,6 +1913,10 @@ class TimelineFragment @Inject constructor(
timelineViewModel.handle(RoomDetailAction.LoadMoreTimelineEvents(direction))
}

override fun onAddMoreReaction(event: TimelineEvent) {
openEmojiReactionPicker(event.eventId)
}

override fun onEventCellClicked(informationData: MessageInformationData, messageContent: Any?, view: View, isRootThreadEvent: Boolean) {
when (messageContent) {
is MessageVerificationRequestContent -> {
Expand Down Expand Up @@ -2117,7 +2121,7 @@ class TimelineFragment @Inject constructor(
openRoomMemberProfile(action.userId)
}
is EventSharedAction.AddReaction -> {
emojiActivityResultLauncher.launch(EmojiReactionPickerActivity.intent(requireContext(), action.eventId))
openEmojiReactionPicker(action.eventId)
}
is EventSharedAction.ViewReactions -> {
ViewReactionsBottomSheet.newInstance(timelineArgs.roomId, action.messageInformationData)
Expand Down Expand Up @@ -2239,6 +2243,10 @@ class TimelineFragment @Inject constructor(
}
}

private fun openEmojiReactionPicker(eventId: String) {
emojiActivityResultLauncher.launch(EmojiReactionPickerActivity.intent(requireContext(), eventId))
}

private fun askConfirmationToEndPoll(eventId: String) {
MaterialAlertDialogBuilder(requireContext(), R.style.ThemeOverlay_Vector_MaterialAlertDialog)
.setTitle(R.string.end_poll_confirmation_title)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import im.vector.app.features.home.room.detail.timeline.factory.TimelineItemFact
import im.vector.app.features.home.room.detail.timeline.factory.TimelineItemFactoryParams
import im.vector.app.features.home.room.detail.timeline.helper.ContentDownloadStateTrackerBinder
import im.vector.app.features.home.room.detail.timeline.helper.ContentUploadStateTrackerBinder
import im.vector.app.features.home.room.detail.timeline.helper.ReactionsSummaryFactory
import im.vector.app.features.home.room.detail.timeline.helper.TimelineControllerInterceptorHelper
import im.vector.app.features.home.room.detail.timeline.helper.TimelineEventDiffUtilCallback
import im.vector.app.features.home.room.detail.timeline.helper.TimelineEventVisibilityHelper
Expand Down Expand Up @@ -86,7 +87,8 @@ class TimelineEventController @Inject constructor(private val dateFormatter: Vec
@TimelineEventControllerHandler
private val backgroundHandler: Handler,
private val timelineEventVisibilityHelper: TimelineEventVisibilityHelper,
private val readReceiptsItemFactory: ReadReceiptsItemFactory
private val readReceiptsItemFactory: ReadReceiptsItemFactory,
private val reactionListFactory: ReactionsSummaryFactory
) : EpoxyController(backgroundHandler, backgroundHandler), Timeline.Listener, EpoxyController.Interceptor {

/**
Expand Down Expand Up @@ -138,6 +140,8 @@ class TimelineEventController @Inject constructor(private val dateFormatter: Vec
fun getPreviewUrlRetriever(): PreviewUrlRetriever

fun onVoiceControlButtonClicked(eventId: String, messageAudioContent: MessageAudioContent)

fun onAddMoreReaction(event: TimelineEvent)
}

interface ReactionPillCallback {
Expand Down Expand Up @@ -283,13 +287,15 @@ class TimelineEventController @Inject constructor(private val dateFormatter: Vec
super.onAttachedToRecyclerView(recyclerView)
timeline?.addListener(this)
timelineMediaSizeProvider.recyclerView = recyclerView
reactionListFactory.onRequestBuild = { requestModelBuild() }
}

override fun onDetachedFromRecyclerView(recyclerView: RecyclerView) {
timelineMediaSizeProvider.recyclerView = null
contentUploadStateTrackerBinder.clear()
contentDownloadStateTrackerBinder.clear()
timeline?.removeListener(this)
reactionListFactory.onRequestBuild = null
super.onDetachedFromRecyclerView(recyclerView)
}

Expand Down Expand Up @@ -383,7 +389,7 @@ class TimelineEventController @Inject constructor(private val dateFormatter: Vec
val event = currentSnapshot[position]
val nextEvent = currentSnapshot.nextOrNull(position)
// Should be build if not cached or if model should be refreshed
if (modelCache[position] == null || modelCache[position]?.isCacheable(partialState) == false) {
if (modelCache[position] == null || modelCache[position]?.isCacheable(partialState) == false || reactionListFactory.needsRebuild(event)) {
val prevEvent = currentSnapshot.prevOrNull(position)
val prevDisplayableEvent = currentSnapshot.subList(0, position).lastOrNull {
timelineEventVisibilityHelper.shouldShowEvent(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import im.vector.app.features.home.room.detail.timeline.item.E2EDecoration
import im.vector.app.features.home.room.detail.timeline.item.MessageInformationData
import im.vector.app.features.home.room.detail.timeline.item.PollResponseData
import im.vector.app.features.home.room.detail.timeline.item.PollVoteSummaryData
import im.vector.app.features.home.room.detail.timeline.item.ReactionInfoData
import im.vector.app.features.home.room.detail.timeline.item.ReferencesInfoData
import im.vector.app.features.home.room.detail.timeline.item.SendStateDecoration
import im.vector.app.features.home.room.detail.timeline.style.TimelineMessageLayoutFactory
Expand All @@ -50,7 +49,8 @@ import javax.inject.Inject
*/
class MessageInformationDataFactory @Inject constructor(private val session: Session,
private val dateFormatter: VectorDateFormatter,
private val messageLayoutFactory: TimelineMessageLayoutFactory) {
private val messageLayoutFactory: TimelineMessageLayoutFactory,
private val reactionsSummaryFactory: ReactionsSummaryFactory) {

fun create(params: TimelineItemFactoryParams): MessageInformationData {
val event = params.event
Expand Down Expand Up @@ -93,11 +93,7 @@ class MessageInformationDataFactory @Inject constructor(private val session: Ses
avatarUrl = event.senderInfo.avatarUrl,
memberName = event.senderInfo.disambiguatedDisplayName,
messageLayout = messageLayout,
orderedReactionList = event.annotations?.reactionsSummary
// ?.filter { isSingleEmoji(it.key) }
?.map {
ReactionInfoData(it.key, it.count, it.addedByMe, it.localEchoEvents.isEmpty())
},
reactionsSummary = reactionsSummaryFactory.create(event, params.callback),
pollResponseAggregatedSummary = event.annotations?.pollResponseSummary?.let {
PollResponseData(
myVote = it.aggregatedContent?.myVote,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* Copyright (c) 2021 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package im.vector.app.features.home.room.detail.timeline.helper

import dagger.hilt.android.scopes.ActivityScoped
import im.vector.app.features.home.room.detail.timeline.TimelineEventController
import im.vector.app.features.home.room.detail.timeline.item.ReactionInfoData
import im.vector.app.features.home.room.detail.timeline.item.ReactionsSummaryData
import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
import javax.inject.Inject

@ActivityScoped
class ReactionsSummaryFactory @Inject constructor() {

var onRequestBuild: (() -> Unit)? = null
private val showAllReactionsByEvent = HashSet<String>()
private val eventsRequestingBuild = HashSet<String>()

fun needsRebuild(event: TimelineEvent): Boolean {
return eventsRequestingBuild.remove(event.eventId)
}

fun create(event: TimelineEvent, callback: TimelineEventController.Callback?): ReactionsSummaryData {
val eventId = event.eventId
val showAllStates = showAllReactionsByEvent.contains(eventId)
val reactions = event.annotations?.reactionsSummary
?.map {
ReactionInfoData(it.key, it.count, it.addedByMe, it.localEchoEvents.isEmpty())
}
return ReactionsSummaryData(
reactions = reactions,
showAll = showAllStates,
onShowMoreClicked = {
showAllReactionsByEvent.add(eventId)
onRequestBuild(eventId)
},
onShowLessClicked = {
showAllReactionsByEvent.remove(eventId)
onRequestBuild(eventId)
},
onAddMoreClicked = {
callback?.onAddMoreReaction(event)
}
)
}

private fun onRequestBuild(eventId: String) {
eventsRequestingBuild.add(eventId)
onRequestBuild?.invoke()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,20 @@

package im.vector.app.features.home.room.detail.timeline.item

import android.annotation.SuppressLint
import android.view.Gravity
import android.view.View
import android.view.ViewGroup
import android.widget.ImageView
import android.widget.TextView
import androidx.annotation.IdRes
import androidx.core.content.ContextCompat.getDrawable
import androidx.core.view.isVisible
import im.vector.app.R
import im.vector.app.core.epoxy.ClickListener
import im.vector.app.core.epoxy.onClick
import im.vector.app.core.ui.views.ShieldImageView
import im.vector.app.core.utils.DimensionConverter
import im.vector.app.features.home.AvatarRenderer
import im.vector.app.features.home.room.detail.timeline.MessageColorProvider
import im.vector.app.features.home.room.detail.timeline.TimelineEventController
Expand All @@ -34,6 +38,8 @@ import im.vector.app.features.reactions.widget.ReactionButton
import org.matrix.android.sdk.api.crypto.RoomEncryptionTrustLevel
import org.matrix.android.sdk.api.session.room.send.SendState

private const val MAX_REACTIONS_TO_SHOW = 8

/**
* Base timeline item with reactions and read receipts.
* Manages associated click listeners and send status.
Expand Down Expand Up @@ -65,15 +71,39 @@ abstract class AbsBaseMessageItem<H : AbsBaseMessageItem.Holder> : BaseEventItem
return listOf(baseAttributes.informationData.eventId)
}

@SuppressLint("SetTextI18n")
override fun bind(holder: H) {
super.bind(holder)
val reactions = baseAttributes.informationData.orderedReactionList
renderReactions(holder, baseAttributes.informationData.reactionsSummary)
when (baseAttributes.informationData.e2eDecoration) {
E2EDecoration.NONE -> {
holder.e2EDecorationView.render(null)
}
E2EDecoration.WARN_IN_CLEAR,
E2EDecoration.WARN_SENT_BY_UNVERIFIED,
E2EDecoration.WARN_SENT_BY_UNKNOWN -> {
holder.e2EDecorationView.render(RoomEncryptionTrustLevel.Warning)
}
}

holder.view.onClick(baseAttributes.itemClickListener)
holder.view.setOnLongClickListener(baseAttributes.itemLongClickListener)
(holder.view as? TimelineMessageLayoutRenderer)?.renderMessageLayout(baseAttributes.informationData.messageLayout)
}

private fun renderReactions(holder: H, reactionsSummary: ReactionsSummaryData) {
val reactions = reactionsSummary.reactions
if (!shouldShowReactionAtBottom() || reactions.isNullOrEmpty()) {
holder.reactionsContainer.isVisible = false
} else {
holder.reactionsContainer.isVisible = true
holder.reactionsContainer.removeAllViews()
reactions.take(8).forEach { reaction ->
val reactionsToShow = if (reactionsSummary.showAll) {
reactions
} else {
reactions.take(MAX_REACTIONS_TO_SHOW)
}
reactionsToShow.forEach { reaction ->
val reactionButton = ReactionButton(holder.view.context)
reactionButton.reactedListener = reactionClickListener
reactionButton.setTag(R.id.reactionsContainer, reaction.key)
Expand All @@ -83,23 +113,36 @@ abstract class AbsBaseMessageItem<H : AbsBaseMessageItem.Holder> : BaseEventItem
reactionButton.isEnabled = reaction.synced
holder.reactionsContainer.addView(reactionButton)
}
if (reactions.count() > MAX_REACTIONS_TO_SHOW) {
val showReactionsTextView = createReactionTextView(holder, 6)
Copy link
Contributor

@ouchadam ouchadam Feb 11, 2022

Choose a reason for hiding this comment

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

nit: could be handy to have some named arguments for the hardcoded values for some extra context

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, I'm changing this to a Style as it's shared with ReactionButton

if (reactionsSummary.showAll) {
showReactionsTextView.setText(R.string.message_reaction_show_less)
showReactionsTextView.onClick { reactionsSummary.onShowLessClicked() }
} else {
val moreCount = reactions.count() - MAX_REACTIONS_TO_SHOW
showReactionsTextView.text = holder.view.resources.getString(R.string.message_reaction_show_more, moreCount)
showReactionsTextView.onClick { reactionsSummary.onShowMoreClicked() }
}
holder.reactionsContainer.addView(showReactionsTextView)
}
val addMoreReactionsTextView = createReactionTextView(holder, 14)
addMoreReactionsTextView.setCompoundDrawablesWithIntrinsicBounds(R.drawable.ic_add_reaction_small, 0, 0, 0)
addMoreReactionsTextView.onClick { reactionsSummary.onAddMoreClicked() }
holder.reactionsContainer.addView(addMoreReactionsTextView)
holder.reactionsContainer.setOnLongClickListener(baseAttributes.itemLongClickListener)
}
}

when (baseAttributes.informationData.e2eDecoration) {
E2EDecoration.NONE -> {
holder.e2EDecorationView.render(null)
}
E2EDecoration.WARN_IN_CLEAR,
E2EDecoration.WARN_SENT_BY_UNVERIFIED,
E2EDecoration.WARN_SENT_BY_UNKNOWN -> {
holder.e2EDecorationView.render(RoomEncryptionTrustLevel.Warning)
}
private fun createReactionTextView(holder: H, horizontalPaddingDp: Int): TextView {
return TextView(holder.view.context).apply {
textSize = 10f
gravity = Gravity.CENTER
minimumHeight = resources.getDimensionPixelSize(R.dimen.chat_reaction_min_height)
minimumWidth = resources.getDimensionPixelSize(R.dimen.chat_reaction_min_width)
background = getDrawable(context, R.drawable.reaction_rounded_rect_shape_off)
val padding = holder.dimensionConverter.dpToPx(horizontalPaddingDp)
setPadding(padding, 0, padding, 0)
}

holder.view.onClick(baseAttributes.itemClickListener)
holder.view.setOnLongClickListener(baseAttributes.itemLongClickListener)
(holder.view as? TimelineMessageLayoutRenderer)?.renderMessageLayout(baseAttributes.informationData.messageLayout)
}

override fun unbind(holder: H) {
Expand All @@ -115,6 +158,9 @@ abstract class AbsBaseMessageItem<H : AbsBaseMessageItem.Holder> : BaseEventItem
}

abstract class Holder(@IdRes stubId: Int) : BaseEventItem.BaseHolder(stubId) {
val dimensionConverter by lazy {
DimensionConverter(view.resources)
}
val reactionsContainer by bind<ViewGroup>(R.id.reactionsContainer)
val e2EDecorationView by bind<ShieldImageView>(R.id.messageE2EDecoration)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ data class MessageInformationData(
val avatarUrl: String?,
val memberName: CharSequence? = null,
val messageLayout: TimelineMessageLayout,
/*List of reactions (emoji,count,isSelected)*/
val orderedReactionList: List<ReactionInfoData>? = null,
val reactionsSummary: ReactionsSummaryData,
val pollResponseAggregatedSummary: PollResponseData? = null,
val hasBeenEdited: Boolean = false,
val hasPendingEdits: Boolean = false,
Expand All @@ -55,6 +54,16 @@ data class ReferencesInfoData(
val verificationStatus: VerificationState
) : Parcelable

@Parcelize
data class ReactionsSummaryData(
/*List of reactions (emoji,count,isSelected)*/
val reactions: List<ReactionInfoData>? = null,
val showAll: Boolean = false,
val onShowMoreClicked: () -> Unit,
val onShowLessClicked: () -> Unit,
val onAddMoreClicked: () -> Unit
Copy link
Member

Choose a reason for hiding this comment

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

Weird to see some lambda in data class

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure what will happen when we attempt to parcelize 🤔

) : Parcelable

@Parcelize
data class ReactionInfoData(
val key: String,
Expand Down
Loading