Code Review

This commit is contained in:
Valere 2021-05-11 19:11:15 +02:00
parent eb9fadaebf
commit 0de56e2993
4 changed files with 79 additions and 65 deletions

View file

@ -20,6 +20,7 @@ import android.view.Gravity
import android.widget.ImageView import android.widget.ImageView
import android.widget.TextView import android.widget.TextView
import androidx.annotation.DrawableRes import androidx.annotation.DrawableRes
import androidx.core.view.isVisible
import androidx.core.widget.ImageViewCompat import androidx.core.widget.ImageViewCompat
import com.airbnb.epoxy.EpoxyAttribute import com.airbnb.epoxy.EpoxyAttribute
import com.airbnb.epoxy.EpoxyModelClass import com.airbnb.epoxy.EpoxyModelClass
@ -30,10 +31,7 @@ import im.vector.app.core.extensions.setTextOrHide
import im.vector.app.features.themes.ThemeUtils import im.vector.app.features.themes.ThemeUtils
/** /**
* A generic list item. * A generic list item with a rounded corner background and an optional icon
* Displays an item with a title, and optional description.
* Can display an accessory on the right, that can be an image or an indeterminate progress.
* If provided with an action, will display a button at the bottom of the list item.
*/ */
@EpoxyModelClass(layout = R.layout.item_generic_pill_footer) @EpoxyModelClass(layout = R.layout.item_generic_pill_footer)
abstract class GenericPillItem : VectorEpoxyModel<GenericPillItem.Holder>() { abstract class GenericPillItem : VectorEpoxyModel<GenericPillItem.Holder>() {
@ -65,7 +63,12 @@ abstract class GenericPillItem : VectorEpoxyModel<GenericPillItem.Holder>() {
holder.textView.textSize = style.toTextSize() holder.textView.textSize = style.toTextSize()
holder.textView.gravity = if (centered) Gravity.CENTER_HORIZONTAL else Gravity.START holder.textView.gravity = if (centered) Gravity.CENTER_HORIZONTAL else Gravity.START
imageRes?.let { holder.imageView.setImageResource(it) } if (imageRes != null) {
holder.imageView.setImageResource(imageRes!!)
holder.imageView.isVisible = true
} else {
holder.imageView.isVisible = false
}
if (tintIcon) { if (tintIcon) {
val iconTintColor = ThemeUtils.getColor(holder.view.context, R.attr.riotx_text_secondary) val iconTintColor = ThemeUtils.getColor(holder.view.context, R.attr.riotx_text_secondary)
ImageViewCompat.setImageTintList(holder.imageView, ColorStateList.valueOf(iconTintColor)) ImageViewCompat.setImageTintList(holder.imageView, ColorStateList.valueOf(iconTintColor))

View file

@ -93,8 +93,11 @@ class SpaceRoomListSectionBuilder(
activeSpaceUpdaters = activeSpaceAwareQueries, activeSpaceUpdaters = activeSpaceAwareQueries,
nameRes = R.string.invitations_header, nameRes = R.string.invitations_header,
notifyOfLocalEcho = true, notifyOfLocalEcho = true,
spaceFilterStrategy = RoomListViewModel.SpaceFilterStrategy.ORPHANS_IF_SPACE_NULL.takeIf { onlyOrphansInHome } spaceFilterStrategy = if (onlyOrphansInHome) {
?: RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL, RoomListViewModel.SpaceFilterStrategy.ORPHANS_IF_SPACE_NULL
} else {
RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL
},
countRoomAsNotif = true countRoomAsNotif = true
) { ) {
it.memberships = listOf(Membership.INVITE) it.memberships = listOf(Membership.INVITE)
@ -102,12 +105,15 @@ class SpaceRoomListSectionBuilder(
} }
addSection( addSection(
sections, sections = sections,
activeSpaceAwareQueries, activeSpaceUpdaters = activeSpaceAwareQueries,
R.string.bottom_action_rooms, nameRes = R.string.bottom_action_rooms,
false, notifyOfLocalEcho = false,
RoomListViewModel.SpaceFilterStrategy.ORPHANS_IF_SPACE_NULL.takeIf { onlyOrphansInHome } spaceFilterStrategy = if (onlyOrphansInHome) {
?: RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL RoomListViewModel.SpaceFilterStrategy.ORPHANS_IF_SPACE_NULL
} else {
RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL
}
) { ) {
it.memberships = listOf(Membership.JOIN) it.memberships = listOf(Membership.JOIN)
it.roomCategoryFilter = RoomCategoryFilter.ONLY_WITH_NOTIFICATIONS it.roomCategoryFilter = RoomCategoryFilter.ONLY_WITH_NOTIFICATIONS
@ -155,12 +161,15 @@ class SpaceRoomListSectionBuilder(
} }
addSection( addSection(
sections, sections = sections,
activeSpaceAwareQueries, activeSpaceUpdaters = activeSpaceAwareQueries,
R.string.bottom_action_rooms, nameRes = R.string.bottom_action_rooms,
false, notifyOfLocalEcho = false,
RoomListViewModel.SpaceFilterStrategy.ORPHANS_IF_SPACE_NULL.takeIf { onlyOrphansInHome } spaceFilterStrategy = if (onlyOrphansInHome) {
?: RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL RoomListViewModel.SpaceFilterStrategy.ORPHANS_IF_SPACE_NULL
} else {
RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL
}
) { ) {
it.memberships = listOf(Membership.JOIN) it.memberships = listOf(Membership.JOIN)
it.roomCategoryFilter = RoomCategoryFilter.ONLY_ROOMS it.roomCategoryFilter = RoomCategoryFilter.ONLY_ROOMS
@ -168,12 +177,15 @@ class SpaceRoomListSectionBuilder(
} }
addSection( addSection(
sections, sections = sections,
activeSpaceAwareQueries, activeSpaceUpdaters = activeSpaceAwareQueries,
R.string.low_priority_header, nameRes = R.string.low_priority_header,
false, notifyOfLocalEcho = false,
RoomListViewModel.SpaceFilterStrategy.ORPHANS_IF_SPACE_NULL.takeIf { onlyOrphansInHome } spaceFilterStrategy = if (onlyOrphansInHome) {
?: RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL RoomListViewModel.SpaceFilterStrategy.ORPHANS_IF_SPACE_NULL
} else {
RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL
}
) { ) {
it.memberships = listOf(Membership.JOIN) it.memberships = listOf(Membership.JOIN)
it.roomCategoryFilter = RoomCategoryFilter.ONLY_ROOMS it.roomCategoryFilter = RoomCategoryFilter.ONLY_ROOMS
@ -181,12 +193,15 @@ class SpaceRoomListSectionBuilder(
} }
addSection( addSection(
sections, sections = sections,
activeSpaceAwareQueries, activeSpaceUpdaters = activeSpaceAwareQueries,
R.string.system_alerts_header, nameRes = R.string.system_alerts_header,
false, notifyOfLocalEcho = false,
RoomListViewModel.SpaceFilterStrategy.ORPHANS_IF_SPACE_NULL.takeIf { onlyOrphansInHome } spaceFilterStrategy = if (onlyOrphansInHome) {
?: RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL RoomListViewModel.SpaceFilterStrategy.ORPHANS_IF_SPACE_NULL
} else {
RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL
}
) { ) {
it.memberships = listOf(Membership.JOIN) it.memberships = listOf(Membership.JOIN)
it.roomCategoryFilter = RoomCategoryFilter.ONLY_ROOMS it.roomCategoryFilter = RoomCategoryFilter.ONLY_ROOMS
@ -372,7 +387,7 @@ class SpaceRoomListSectionBuilder(
activeSpaceFilter = ActiveSpaceFilter.ActiveSpace(currentSpace) activeSpaceFilter = ActiveSpaceFilter.ActiveSpace(currentSpace)
) )
} }
RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL -> { RoomListViewModel.SpaceFilterStrategy.ALL_IF_SPACE_NULL -> {
if (currentSpace == null) { if (currentSpace == null) {
copy( copy(
activeSpaceFilter = ActiveSpaceFilter.None activeSpaceFilter = ActiveSpaceFilter.None
@ -383,7 +398,7 @@ class SpaceRoomListSectionBuilder(
) )
} }
} }
RoomListViewModel.SpaceFilterStrategy.NONE -> this RoomListViewModel.SpaceFilterStrategy.NONE -> this
} }
} }
} }

View file

@ -179,31 +179,31 @@ class SpaceSummaryController @Inject constructor(
private fun buildSubSpace(summaries: List<RoomSummary>?, private fun buildSubSpace(summaries: List<RoomSummary>?,
expandedStates: Map<String, Boolean>, expandedStates: Map<String, Boolean>,
selected: RoomGroupingMethod, selected: RoomGroupingMethod,
childSum: SpaceChildInfo, currentDepth: Int, maxDepth: Int) { info: SpaceChildInfo, currentDepth: Int, maxDepth: Int) {
if (currentDepth >= maxDepth) return if (currentDepth >= maxDepth) return
val childSum = summaries?.firstOrNull { it.roomId == childSum.childRoomId } ?: return val childSummary = summaries?.firstOrNull { it.roomId == info.childRoomId } ?: return
// does it have children? // does it have children?
val subSpaces = childSum.spaceChildren?.filter { childInfo -> val subSpaces = childSummary.spaceChildren?.filter { childInfo ->
summaries.indexOfFirst { it.roomId == childInfo.childRoomId } != -1 summaries.indexOfFirst { it.roomId == childInfo.childRoomId } != -1
}?.sortedWith(subSpaceComparator) }?.sortedWith(subSpaceComparator)
val expanded = expandedStates[childSum.roomId] == true val expanded = expandedStates[childSummary.roomId] == true
val isSelected = selected is RoomGroupingMethod.BySpace && childSum.roomId == selected.space()?.roomId val isSelected = selected is RoomGroupingMethod.BySpace && childSummary.roomId == selected.space()?.roomId
subSpaceSummaryItem { subSpaceSummaryItem {
avatarRenderer(avatarRenderer) avatarRenderer(avatarRenderer)
id(childSum.roomId) id(childSummary.roomId)
hasChildren(!subSpaces.isNullOrEmpty()) hasChildren(!subSpaces.isNullOrEmpty())
selected(isSelected) selected(isSelected)
expanded(expanded) expanded(expanded)
onMore { callback?.onSpaceSettings(childSum) } onMore { callback?.onSpaceSettings(childSummary) }
matrixItem(childSum.toMatrixItem()) matrixItem(childSummary.toMatrixItem())
listener { callback?.onSpaceSelected(childSum) } listener { callback?.onSpaceSelected(childSummary) }
toggleExpand { callback?.onToggleExpand(childSum) } toggleExpand { callback?.onToggleExpand(childSummary) }
indent(currentDepth) indent(currentDepth)
countState( countState(
UnreadCounterBadgeView.State( UnreadCounterBadgeView.State(
childSum.notificationCount, childSummary.notificationCount,
childSum.highlightCount > 0 childSummary.highlightCount > 0
) )
) )
} }

View file

@ -20,7 +20,6 @@ import android.widget.ImageView
import android.widget.Space import android.widget.Space
import android.widget.TextView import android.widget.TextView
import androidx.core.content.ContextCompat import androidx.core.content.ContextCompat
import androidx.core.view.isGone
import androidx.core.view.isVisible import androidx.core.view.isVisible
import androidx.core.view.updateLayoutParams import androidx.core.view.updateLayoutParams
import com.airbnb.epoxy.EpoxyAttribute import com.airbnb.epoxy.EpoxyAttribute
@ -37,16 +36,16 @@ import org.matrix.android.sdk.api.util.MatrixItem
@EpoxyModelClass(layout = R.layout.item_sub_space) @EpoxyModelClass(layout = R.layout.item_sub_space)
abstract class SubSpaceSummaryItem : VectorEpoxyModel<SubSpaceSummaryItem.Holder>() { abstract class SubSpaceSummaryItem : VectorEpoxyModel<SubSpaceSummaryItem.Holder>() {
@EpoxyAttribute(EpoxyAttribute.Option.DoNotHash) lateinit var avatarRenderer: AvatarRenderer @EpoxyAttribute(EpoxyAttribute.Option.DoNotHash) lateinit var avatarRenderer: AvatarRenderer
@EpoxyAttribute lateinit var matrixItem: MatrixItem @EpoxyAttribute lateinit var matrixItem: MatrixItem
@EpoxyAttribute var selected: Boolean = false @EpoxyAttribute var selected: Boolean = false
@EpoxyAttribute(EpoxyAttribute.Option.DoNotHash) var listener: (() -> Unit)? = null @EpoxyAttribute(EpoxyAttribute.Option.DoNotHash) var listener: (() -> Unit)? = null
@EpoxyAttribute(EpoxyAttribute.Option.DoNotHash) var onMore: (() -> Unit)? = null @EpoxyAttribute(EpoxyAttribute.Option.DoNotHash) var onMore: (() -> Unit)? = null
@EpoxyAttribute(EpoxyAttribute.Option.DoNotHash) var toggleExpand: (() -> Unit)? = null @EpoxyAttribute(EpoxyAttribute.Option.DoNotHash) var toggleExpand: (() -> Unit)? = null
@EpoxyAttribute var expanded: Boolean = false @EpoxyAttribute var expanded: Boolean = false
@EpoxyAttribute var hasChildren: Boolean = false @EpoxyAttribute var hasChildren: Boolean = false
@EpoxyAttribute var indent: Int = 0 @EpoxyAttribute var indent: Int = 0
@EpoxyAttribute var countState : UnreadCounterBadgeView.State = UnreadCounterBadgeView.State(0, false) @EpoxyAttribute var countState: UnreadCounterBadgeView.State = UnreadCounterBadgeView.State(0, false)
override fun bind(holder: Holder) { override fun bind(holder: Holder) {
super.bind(holder) super.bind(holder)
@ -64,21 +63,18 @@ abstract class SubSpaceSummaryItem : VectorEpoxyModel<SubSpaceSummaryItem.Holder
holder.moreView.isVisible = false holder.moreView.isVisible = false
} }
if (hasChildren) { holder.collapseIndicator.setImageDrawable(
holder.collapseIndicator.isVisible = true ContextCompat.getDrawable(holder.view.context,
holder.collapseIndicator.setImageDrawable( if (expanded) R.drawable.ic_expand_less else R.drawable.ic_expand_more
ContextCompat.getDrawable(holder.view.context, )
if (expanded) R.drawable.ic_expand_less else R.drawable.ic_expand_more )
) holder.collapseIndicator.setOnClickListener(
) DebouncedClickListener({ _ ->
holder.collapseIndicator.setOnClickListener( toggleExpand?.invoke()
DebouncedClickListener({ _ -> })
toggleExpand?.invoke() )
})
) holder.collapseIndicator.isVisible = hasChildren
} else {
holder.collapseIndicator.isGone = true
}
holder.indentSpace.isVisible = indent > 0 holder.indentSpace.isVisible = indent > 0
holder.indentSpace.updateLayoutParams { holder.indentSpace.updateLayoutParams {