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.

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

Replaces subtitle in Search Rooms with room context rather than last event #5860

Merged
merged 24 commits into from
May 30, 2022
Merged

Replaces subtitle in Search Rooms with room context rather than last event #5860

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0250f61
Replaces izPublic with isPublic
ericdecanini Apr 26, 2022
a3367d4
Replaces else cases in when branches to RoomListDisplayMode.FILTERED
ericdecanini Apr 26, 2022
9e53e6c
Adds space name to rooms in filtered search
ericdecanini Apr 28, 2022
70cded2
Adds user id and canonical alias to search result subtitles
ericdecanini Apr 28, 2022
87ad35d
Disables typing indicator in filtered search
ericdecanini Apr 28, 2022
3347560
Adds canonical named argument to RoomSummaryUpdater
ericdecanini Apr 28, 2022
b280358
Adds more named arguments to RoomSummaryUpdater
ericdecanini Apr 28, 2022
4784717
Fixes lint error
ericdecanini Apr 28, 2022
962e9ab
Fixes lint error
ericdecanini Apr 28, 2022
7e415e8
Fixes lint error
ericdecanini Apr 28, 2022
7cc79fe
Refactors RoomSummaryItem
ericdecanini Apr 29, 2022
a355b62
Adds displayMode to RoomSummaryListController
ericdecanini Apr 29, 2022
f70a24d
Changes IncomingShareController display mode to FILTERED
ericdecanini Apr 29, 2022
47493fc
Replaces method for getting the space parents of rooms
ericdecanini May 2, 2022
c9b32fe
Changes ordering of room subtitles used
ericdecanini May 2, 2022
b46794d
Adds changelog file
ericdecanini May 2, 2022
52c404a
Merge remote-tracking branch 'origin/develop' into feature/eric/repla…
ericdecanini May 12, 2022
21fe5a2
Adds vertical centering of title when no subtitle is present
ericdecanini May 13, 2022
50839c2
Adds flattenParents field to RoomSummary and corresponding mapping
ericdecanini May 13, 2022
7c1d1c3
Adds centering of items with no subtitles
ericdecanini May 16, 2022
83bd9bc
Fixes lint error
ericdecanini May 16, 2022
03acf45
Uses second layout to center room summary item title
ericdecanini May 17, 2022
d12ab17
Fixes lint errors
ericdecanini May 18, 2022
a5dc8ec
Only gets flattenParents if specifically requested
ericdecanini May 27, 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
16 changes: 3 additions & 13 deletions ...n/java/org/matrix/android/sdk/internal/session/room/relationship/RoomChildRelationInfo.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ internal class RoomChildRelationInfo(
data class SpaceChildInfo(
val roomId: String,
val order: String?,
// val autoJoin: Boolean,
val viaServers: List<String>
)

Expand All @@ -60,18 +59,13 @@ internal class RoomChildRelationInfo(
fun getDirectChildrenDescriptions(): List<SpaceChildInfo> {
return CurrentStateEventEntity.whereType(realm, roomId, EventType.STATE_SPACE_CHILD)
.findAll()
// .also {
// Timber.v("## Space: Found ${it.count()} m.space.child state events for $roomId")
// }
.mapNotNull {
ContentMapper.map(it.root?.content).toModel<SpaceChildContent>()?.let { scc ->
// Timber.v("## Space child desc state event $scc")
// Children where via is not present are ignored.
scc.via?.let { via ->
SpaceChildInfo(
roomId = it.stateKey,
order = scc.validOrder(),
// autoJoin = scc.autoJoin ?: false,
viaServers = via
)
}
Expand All @@ -83,17 +77,13 @@ internal class RoomChildRelationInfo(
fun getParentDescriptions(): List<SpaceParentInfo> {
return CurrentStateEventEntity.whereType(realm, roomId, EventType.STATE_SPACE_PARENT)
.findAll()
// .also {
// Timber.v("## Space: Found ${it.count()} m.space.parent state events for $roomId")
// }
.mapNotNull {
ContentMapper.map(it.root?.content).toModel<SpaceParentContent>()?.let { scc ->
// Timber.v("## Space parent desc state event $scc")
ContentMapper.map(it.root?.content).toModel<SpaceParentContent>()?.let { spaceParentContent ->
// Parent where via is not present are ignored.
scc.via?.let { via ->
spaceParentContent.via?.let { via ->
SpaceParentInfo(
roomId = it.stateKey,
canonical = scc.canonical ?: false,
canonical = spaceParentContent.canonical ?: false,
viaServers = via,
stateEventSender = it.root?.sender ?: ""
)
Expand Down
29 changes: 12 additions & 17 deletions .../src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.matrix.android.sdk.internal.session.room.summary

import io.realm.Realm
import io.realm.RealmList
import io.realm.kotlin.createObject
import kotlinx.coroutines.runBlocking
import org.matrix.android.sdk.api.extensions.orFalse
Expand Down Expand Up @@ -349,39 +350,34 @@ internal class RoomSummaryUpdater @Inject constructor(
}

val acyclicGraph = graph.withoutEdges(backEdges)
// Timber.v("## SPACES: acyclicGraph $acyclicGraph")
val flattenSpaceParents = acyclicGraph.flattenDestination().map {
it.key.name to it.value.map { it.name }
}.toMap()
// Timber.v("## SPACES: flattenSpaceParents ${flattenSpaceParents.map { it.key.name to it.value.map { it.name } }.joinToString("\n") {
// it.first + ": [" + it.second.joinToString(",") + "]"
// }}")

// Timber.v("## SPACES: lookup map ${lookupMap.map { it.key.name to it.value.map { it.name } }.toMap()}")

lookupMap.entries
.filter { it.key.roomType == RoomType.SPACE && it.key.membership == Membership.JOIN }
.forEach { entry ->
val parent = RoomSummaryEntity.where(realm, entry.key.roomId).findFirst()
if (parent != null) {
// Timber.v("## SPACES: check hierarchy of ${parent.name} id ${parent.roomId}")
// Timber.v("## SPACES: flat known parents of ${parent.name} are ${flattenSpaceParents[parent.roomId]}")
val flattenParentsIds = (flattenSpaceParents[parent.roomId] ?: emptyList()) + listOf(parent.roomId)
// Timber.v("## SPACES: flatten known parents of children of ${parent.name} are ${flattenParentsIds}")
entry.value.forEach { child ->
RoomSummaryEntity.where(realm, child.roomId).findFirst()?.let { childSum ->

// Timber.w("## SPACES: ${childSum.name} is ${childSum.roomId} fc: ${childSum.flattenParentIds}")
// var allParents = childSum.flattenParentIds ?: ""
// TODO: Revisit
childSum.parents.add(SpaceParentSummaryEntity(
canonical = true,
parentRoomId = parent.roomId,
parentSummaryEntity = parent,
viaServers = RealmList()
))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BillCarsonFr @bmarty @ganfra can you guys give me more context surrounding this please.

I added this block to add the parents of a room (spaces, etc) as they currently weren't being added, but the canonical = true and viaServers = RealmList() are currently placeholders as I don't know what they are or what they should be.

Copy link
Member

Choose a reason for hiding this comment

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

You should remove this part. Parent is only for m.space.parent state event.
We are not adding a parent link because you are a child.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does a child have no knowledge of its parent then, other than through flattenParentIds? This is confusing because each roomSummary has the field spaceParents which is currently often empty.

Ultimately as well, we would like to be able to know a room's parent in its summary object so we can achieve the same as what web does here (the space of the room is displayed in search)

image

Copy link
Member

Choose a reason for hiding this comment

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

Yes parent is really when there is a m.space.parent state event in the room (and a valid one, added by an admin in both rooms).
It's not really used right now, it's more to help for discovery, when you join a room from a link the UI could prompt a tip to propose you to join the space.

I think you can use flattenParentsIds for the use case you mentioned.

if (childSum.flattenParentIds == null) childSum.flattenParentIds = ""
flattenParentsIds.forEach {
if (childSum.flattenParentIds?.contains(it) != true) {
childSum.flattenParentIds += "|$it"
if (childSum.flattenParentIds?.isEmpty() == false) {
childSum.flattenParentIds += "|"
}
childSum.flattenParentIds += it
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes a bug where the first element of flattenParentIds would always be ""

Copy link
Member

Choose a reason for hiding this comment

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

Why is it a bug? What was not working?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

E.g. if a room had one parent, its roomSummary.flattenParentIds would be {"", "parentRoomId"}

Idk if it has any user-facing impact but I believe this isn't expected behaviour in the code

Copy link
Member

Choose a reason for hiding this comment

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

I am surprised we didn't get report on this, maybe the RoomSummaryMapper is doing some cleaning? None the less I would submit this as a separate PR.

}
}
// childSum.flattenParentIds = "$allParents|"

// Timber.v("## SPACES: flatten of ${childSum.name} is ${childSum.flattenParentIds}")
}
}
}
Expand Down Expand Up @@ -411,7 +407,6 @@ internal class RoomSummaryUpdater @Inject constructor(
// we keep real m.child/m.parent relations and add the one for common memberships
dmRoom.flattenParentIds += "|${flattenRelated.joinToString("|")}|"
}
// Timber.v("## SPACES: flatten of ${dmRoom.otherMemberIds.joinToString(",")} is ${dmRoom.flattenParentIds}")
}

// Maybe a good place to count the number of notifications for spaces?
Expand Down
2 changes: 1 addition & 1 deletion ...or/src/main/java/im/vector/app/features/home/room/list/CollapsableTypedEpoxyController.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ abstract class CollapsableTypedEpoxyController<T> :
}

override fun buildModels() {
check(isBuildingModels()) {
check(isBuildingModels) {
("You cannot call `buildModels` directly. Call `setData` instead to trigger a model " +
"refresh with new data.")
}
Expand Down
21 changes: 11 additions & 10 deletions vector/src/main/java/im/vector/app/features/home/room/list/RoomListFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ class RoomListFragment @Inject constructor(
RoomListDisplayMode.NOTIFICATIONS -> views.createChatFabMenu.isVisible = true
RoomListDisplayMode.PEOPLE -> views.createChatRoomButton.isVisible = true
RoomListDisplayMode.ROOMS -> views.createGroupRoomButton.isVisible = true
else -> Unit // No button in this mode
RoomListDisplayMode.FILTERED -> Unit // No button in this mode
}

views.createChatRoomButton.debouncedClicks {
Expand All @@ -230,7 +230,7 @@ class RoomListFragment @Inject constructor(
RoomListDisplayMode.NOTIFICATIONS -> views.createChatFabMenu.hide()
RoomListDisplayMode.PEOPLE -> views.createChatRoomButton.hide()
RoomListDisplayMode.ROOMS -> views.createGroupRoomButton.hide()
else -> Unit
RoomListDisplayMode.FILTERED -> Unit
}
}
}
Expand Down Expand Up @@ -287,7 +287,7 @@ class RoomListFragment @Inject constructor(
val contentAdapter =
when {
section.livePages != null -> {
pagedControllerFactory.createRoomSummaryPagedController()
pagedControllerFactory.createRoomSummaryPagedController(roomListParams.displayMode)
.also { controller ->
section.livePages.observe(viewLifecycleOwner) { pl ->
controller.submitList(pl)
Expand All @@ -309,7 +309,7 @@ class RoomListFragment @Inject constructor(
)
}
}
section.isExpanded.observe(viewLifecycleOwner) { _ ->
section.isExpanded.observe(viewLifecycleOwner) {
refreshCollapseStates()
}
controller.listener = this
Expand All @@ -330,14 +330,14 @@ class RoomListFragment @Inject constructor(
checkEmptyState()
}
observeItemCount(section, sectionAdapter)
section.isExpanded.observe(viewLifecycleOwner) { _ ->
section.isExpanded.observe(viewLifecycleOwner) {
refreshCollapseStates()
}
controller.listener = this
}
}
else -> {
pagedControllerFactory.createRoomSummaryListController()
pagedControllerFactory.createRoomSummaryListController(roomListParams.displayMode)
.also { controller ->
section.liveList?.observe(viewLifecycleOwner) { list ->
controller.setData(list)
Expand All @@ -359,7 +359,7 @@ class RoomListFragment @Inject constructor(
)
}
}
section.isExpanded.observe(viewLifecycleOwner) { _ ->
section.isExpanded.observe(viewLifecycleOwner) {
refreshCollapseStates()
}
controller.listener = this
Expand Down Expand Up @@ -395,7 +395,7 @@ class RoomListFragment @Inject constructor(
RoomListDisplayMode.NOTIFICATIONS -> views.createChatFabMenu.show()
RoomListDisplayMode.PEOPLE -> views.createChatRoomButton.show()
RoomListDisplayMode.ROOMS -> views.createGroupRoomButton.show()
else -> Unit
RoomListDisplayMode.FILTERED -> Unit
}
}
}
Expand Down Expand Up @@ -474,7 +474,8 @@ class RoomListFragment @Inject constructor(
StateView.State.Empty(
title = getString(R.string.room_list_catchup_empty_title),
image = ContextCompat.getDrawable(requireContext(), R.drawable.ic_noun_party_popper),
message = getString(R.string.room_list_catchup_empty_body))
message = getString(R.string.room_list_catchup_empty_body)
)
}
RoomListDisplayMode.PEOPLE ->
StateView.State.Empty(
Expand All @@ -490,7 +491,7 @@ class RoomListFragment @Inject constructor(
isBigImage = true,
message = getString(R.string.room_list_rooms_empty_body)
)
else ->
RoomListDisplayMode.FILTERED ->
// Always display the content in this mode, because if the footer
StateView.State.Content
}
Expand Down
103 changes: 79 additions & 24 deletions vector/src/main/java/im/vector/app/features/home/room/list/RoomSummaryItem.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import im.vector.app.core.ui.views.PresenceStateImageView
import im.vector.app.core.ui.views.ShieldImageView
import im.vector.app.features.displayname.getBestName
import im.vector.app.features.home.AvatarRenderer
import im.vector.app.features.home.RoomListDisplayMode
import im.vector.app.features.themes.ThemeUtils
import im.vector.lib.core.utils.epoxy.charsequence.EpoxyCharSequence
import org.matrix.android.sdk.api.session.crypto.model.RoomEncryptionTrustLevel
Expand All @@ -45,48 +46,102 @@ import org.matrix.android.sdk.api.util.MatrixItem
@EpoxyModelClass(layout = R.layout.item_room)
abstract class RoomSummaryItem : VectorEpoxyModel<RoomSummaryItem.Holder>() {

@EpoxyAttribute lateinit var typingMessage: String
@EpoxyAttribute lateinit var avatarRenderer: AvatarRenderer
@EpoxyAttribute lateinit var matrixItem: MatrixItem

@EpoxyAttribute lateinit var lastFormattedEvent: EpoxyCharSequence
@EpoxyAttribute lateinit var lastEventTime: String
@EpoxyAttribute var encryptionTrustLevel: RoomEncryptionTrustLevel? = null
@EpoxyAttribute var userPresence: UserPresence? = null
@EpoxyAttribute var showPresence: Boolean = false
@EpoxyAttribute var izPublic: Boolean = false
@EpoxyAttribute var unreadNotificationCount: Int = 0
@EpoxyAttribute var hasUnreadMessage: Boolean = false
@EpoxyAttribute var hasDraft: Boolean = false
@EpoxyAttribute var showHighlighted: Boolean = false
@EpoxyAttribute var hasFailedSending: Boolean = false
@EpoxyAttribute(EpoxyAttribute.Option.DoNotHash) var itemLongClickListener: View.OnLongClickListener? = null
@EpoxyAttribute(EpoxyAttribute.Option.DoNotHash) var itemClickListener: ClickListener? = null
@EpoxyAttribute var showSelected: Boolean = false
@EpoxyAttribute
lateinit var typingMessage: String

@EpoxyAttribute
lateinit var avatarRenderer: AvatarRenderer

@EpoxyAttribute
lateinit var matrixItem: MatrixItem

@EpoxyAttribute
var displayMode: RoomListDisplayMode = RoomListDisplayMode.PEOPLE

@EpoxyAttribute
lateinit var subtitle: String

@EpoxyAttribute
lateinit var lastFormattedEvent: EpoxyCharSequence

@EpoxyAttribute
lateinit var lastEventTime: String

@EpoxyAttribute
var encryptionTrustLevel: RoomEncryptionTrustLevel? = null

@EpoxyAttribute
var userPresence: UserPresence? = null

@EpoxyAttribute
var showPresence: Boolean = false

@EpoxyAttribute @JvmField
var isPublic: Boolean = false

@EpoxyAttribute
var unreadNotificationCount: Int = 0

@EpoxyAttribute
var hasUnreadMessage: Boolean = false

@EpoxyAttribute
var hasDraft: Boolean = false

@EpoxyAttribute
var showHighlighted: Boolean = false

@EpoxyAttribute
var hasFailedSending: Boolean = false

@EpoxyAttribute(EpoxyAttribute.Option.DoNotHash)
var itemLongClickListener: View.OnLongClickListener? = null

@EpoxyAttribute(EpoxyAttribute.Option.DoNotHash)
var itemClickListener: ClickListener? = null

@EpoxyAttribute
var showSelected: Boolean = false

override fun bind(holder: Holder) {
super.bind(holder)

renderDisplayMode(holder)
holder.rootView.onClick(itemClickListener)
holder.rootView.setOnLongClickListener {
it.performHapticFeedback(HapticFeedbackConstants.LONG_PRESS)
itemLongClickListener?.onLongClick(it) ?: false
}
holder.titleView.text = matrixItem.getBestName()
holder.lastEventTimeView.text = lastEventTime
holder.lastEventView.text = lastFormattedEvent.charSequence
holder.unreadCounterBadgeView.render(UnreadCounterBadgeView.State(unreadNotificationCount, showHighlighted))
holder.unreadIndentIndicator.isVisible = hasUnreadMessage
holder.draftView.isVisible = hasDraft
avatarRenderer.render(matrixItem, holder.avatarImageView)
holder.roomAvatarDecorationImageView.render(encryptionTrustLevel)
holder.roomAvatarPublicDecorationImageView.isVisible = izPublic
holder.roomAvatarPublicDecorationImageView.isVisible = isPublic
holder.roomAvatarFailSendingImageView.isVisible = hasFailedSending
renderSelection(holder, showSelected)
holder.typingView.setTextOrHide(typingMessage)
holder.lastEventView.isInvisible = holder.typingView.isVisible
holder.roomAvatarPresenceImageView.render(showPresence, userPresence)
}

private fun renderDisplayMode(holder: Holder) = when (displayMode) {
RoomListDisplayMode.ROOMS,
RoomListDisplayMode.PEOPLE,
RoomListDisplayMode.NOTIFICATIONS -> renderForDefaultDisplayMode(holder)
RoomListDisplayMode.FILTERED -> renderForFilteredDisplayMode(holder)
}

private fun renderForDefaultDisplayMode(holder: Holder) {
holder.subtitleView.text = lastFormattedEvent.charSequence
holder.lastEventTimeView.text = lastEventTime
}

private fun renderForFilteredDisplayMode(holder: Holder) {
holder.subtitleView.text = subtitle
holder.typingView.setTextOrHide(typingMessage)
holder.subtitleView.isInvisible = holder.typingView.isVisible
}

override fun unbind(holder: Holder) {
holder.rootView.setOnClickListener(null)
holder.rootView.setOnLongClickListener(null)
Expand All @@ -110,7 +165,7 @@ abstract class RoomSummaryItem : VectorEpoxyModel<RoomSummaryItem.Holder>() {
val titleView by bind<TextView>(R.id.roomNameView)
val unreadCounterBadgeView by bind<UnreadCounterBadgeView>(R.id.roomUnreadCounterBadgeView)
val unreadIndentIndicator by bind<View>(R.id.roomUnreadIndicator)
val lastEventView by bind<TextView>(R.id.roomLastEventView)
val subtitleView by bind<TextView>(R.id.subtitleView)
val typingView by bind<TextView>(R.id.roomTypingView)
val draftView by bind<ImageView>(R.id.roomDraftBadge)
val lastEventTimeView by bind<TextView>(R.id.roomLastEventTimeView)
Expand Down
Loading