Skip to content

Fixes timeline crash due to additionalTopSpace #5534

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
ericdecanini merged 4 commits into from
Mar 15, 2022
Merged

Fixes timeline crash due to additionalTopSpace #5534

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2176129
Moves additional top space usage to bubble view
ericdecanini Mar 14, 2022
d35c1e8
Refactors MessageBubbleView
ericdecanini Mar 14, 2022
3981e72
Adds changelog file
ericdecanini Mar 14, 2022
184b35a
Removes changelog file
ericdecanini Mar 14, 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/5534.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes crash upon opening timeline due to additionalTopSpace
Copy link
Contributor

@mnaturel mnaturel Mar 14, 2022

Choose a reason for hiding this comment

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

I am wondering if this fix needs an entry. Is the crash on the current release?

Copy link
Contributor Author

@ericdecanini ericdecanini Mar 14, 2022

Choose a reason for hiding this comment

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

After several discussions, updated the PR and removed the changelog 👍️

3 changes: 0 additions & 3 deletions 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 @@ -106,8 +106,6 @@ abstract class AbsMessageItem<H : AbsMessageItem.Holder> : AbsBaseMessageItem<H>
holder.timeView.isVisible = false
}

holder.additionalTopSpace.isVisible = attributes.informationData.messageLayout.addTopMargin

// Render send state indicator
holder.sendStateImageView.render(attributes.informationData.sendStateDecoration)
holder.eventSendingIndicator.isVisible = attributes.informationData.sendStateDecoration == SendStateDecoration.SENDING_MEDIA
Expand Down Expand Up @@ -157,7 +155,6 @@ abstract class AbsMessageItem<H : AbsMessageItem.Holder> : AbsBaseMessageItem<H>

abstract class Holder(@IdRes stubId: Int) : AbsBaseMessageItem.Holder(stubId) {

val additionalTopSpace by bind<View>(R.id.additionalTopSpace)
val avatarImageView by bind<ImageView>(R.id.messageAvatarImageView)
val memberNameView by bind<TextView>(R.id.messageMemberNameView)
val timeView by bind<TextView>(R.id.messageTimeView)
Expand Down
49 changes: 25 additions & 24 deletions ...main/java/im/vector/app/features/home/room/detail/timeline/style/TimelineMessageLayout.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,43 +21,44 @@ import im.vector.app.R
import kotlinx.parcelize.Parcelize

sealed interface TimelineMessageLayout : Parcelable {

val layoutRes: Int
val showAvatar: Boolean
val showDisplayName: Boolean
val addTopMargin: Boolean
val showTimestamp: Boolean

@Parcelize
data class Default(override val showAvatar: Boolean,
override val showDisplayName: Boolean,
override val showTimestamp: Boolean,
override val addTopMargin: Boolean = false,
// Keep defaultLayout generated on epoxy items
override val layoutRes: Int = 0) : TimelineMessageLayout
data class Default(
override val showAvatar: Boolean,
override val showDisplayName: Boolean,
override val showTimestamp: Boolean,
// Keep defaultLayout generated on epoxy items
override val layoutRes: Int = 0,
) : TimelineMessageLayout

@Parcelize
data class Bubble(
override val showAvatar: Boolean,
override val showDisplayName: Boolean,
override val showTimestamp: Boolean = true,
override val addTopMargin: Boolean = false,
val isIncoming: Boolean,
val isPseudoBubble: Boolean,
val cornersRadius: CornersRadius,
val timestampAsOverlay: Boolean,
override val layoutRes: Int = if (isIncoming) {
R.layout.item_timeline_event_bubble_incoming_base
} else {
R.layout.item_timeline_event_bubble_outgoing_base
}
override val showAvatar: Boolean,
override val showDisplayName: Boolean,
override val showTimestamp: Boolean = true,
val addTopMargin: Boolean = false,
val isIncoming: Boolean,
val isPseudoBubble: Boolean,
val cornersRadius: CornersRadius,
val timestampAsOverlay: Boolean,
override val layoutRes: Int = if (isIncoming) {
R.layout.item_timeline_event_bubble_incoming_base
} else {
R.layout.item_timeline_event_bubble_outgoing_base
},
) : TimelineMessageLayout {

@Parcelize
data class CornersRadius(
val topStartRadius: Float,
val topEndRadius: Float,
val bottomStartRadius: Float,
val bottomEndRadius: Float
val topStartRadius: Float,
val topEndRadius: Float,
val bottomStartRadius: Float,
val bottomEndRadius: Float,
) : Parcelable
}
}
79 changes: 50 additions & 29 deletions .../src/main/java/im/vector/app/features/home/room/detail/timeline/view/MessageBubbleView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,11 @@ import im.vector.app.features.home.room.detail.timeline.style.shapeAppearanceMod
import im.vector.app.features.themes.ThemeUtils
import timber.log.Timber

class MessageBubbleView @JvmOverloads constructor(context: Context, attrs: AttributeSet? = null,
defStyleAttr: Int = 0) :
RelativeLayout(context, attrs, defStyleAttr), TimelineMessageLayoutRenderer {
class MessageBubbleView @JvmOverloads constructor(
context: Context,
attrs: AttributeSet? = null,
defStyleAttr: Int = 0,
) : RelativeLayout(context, attrs, defStyleAttr), TimelineMessageLayoutRenderer {

private var isIncoming: Boolean = false

Expand Down Expand Up @@ -87,22 +89,43 @@ class MessageBubbleView @JvmOverloads constructor(context: Context, attrs: Attri
outlineProvider = ViewOutlineProvider.BACKGROUND
clipToOutline = true
background = RippleDrawable(
ContextCompat.getColorStateList(context, R.color.mtrl_btn_ripple_color) ?: ColorStateList.valueOf(Color.TRANSPARENT),
bubbleDrawable,
rippleMaskDrawable)
ContextCompat.getColorStateList(context, R.color.mtrl_btn_ripple_color) ?: ColorStateList.valueOf(Color.TRANSPARENT),
bubbleDrawable,
rippleMaskDrawable)
}
}

override fun renderMessageLayout(messageLayout: TimelineMessageLayout) {
if (messageLayout !is TimelineMessageLayout.Bubble) {
Timber.v("Can't render messageLayout $messageLayout")
return
(messageLayout as? TimelineMessageLayout.Bubble)
?.updateDrawables()
?.setConstraintsAndColor()
?.toggleMessageOverlay()
?.setPadding()
?.setMargins()
?.setAdditionalTopSpace()
?: Timber.v("Can't render messageLayout $messageLayout")
}

private fun TimelineMessageLayout.Bubble.updateDrawables() = apply {
val shapeAppearanceModel = cornersRadius.shapeAppearanceModel()
bubbleDrawable.apply {
this.shapeAppearanceModel = shapeAppearanceModel
this.fillColor = if (isPseudoBubble) {
ColorStateList.valueOf(Color.TRANSPARENT)
} else {
val backgroundColorAttr = if (isIncoming) R.attr.vctr_message_bubble_inbound else R.attr.vctr_message_bubble_outbound
val backgroundColor = ThemeUtils.getColor(context, backgroundColorAttr)
ColorStateList.valueOf(backgroundColor)
}
}
updateDrawables(messageLayout)
rippleMaskDrawable.shapeAppearanceModel = shapeAppearanceModel
}

private fun TimelineMessageLayout.Bubble.setConstraintsAndColor() = apply {
ConstraintSet().apply {
clone(views.bubbleView)
clear(R.id.viewStubContainer, ConstraintSet.END)
if (messageLayout.timestampAsOverlay) {
if (timestampAsOverlay) {
val timeColor = ContextCompat.getColor(context, R.color.palette_white)
views.messageTimeView.setTextColor(timeColor)
connect(R.id.viewStubContainer, ConstraintSet.END, R.id.parent, ConstraintSet.END, 0)
Expand All @@ -113,17 +136,26 @@ class MessageBubbleView @JvmOverloads constructor(context: Context, attrs: Attri
}
applyTo(views.bubbleView)
}
if (messageLayout.timestampAsOverlay) {
}

private fun TimelineMessageLayout.Bubble.toggleMessageOverlay() = apply {
if (timestampAsOverlay) {
views.messageOverlayView.isVisible = true
(views.messageOverlayView.background as? GradientDrawable)?.cornerRadii = messageLayout.cornersRadius.toFloatArray()
(views.messageOverlayView.background as? GradientDrawable)?.cornerRadii = cornersRadius.toFloatArray()
} else {
views.messageOverlayView.isVisible = false
}
if (messageLayout.isPseudoBubble && messageLayout.timestampAsOverlay) {
}

private fun TimelineMessageLayout.Bubble.setPadding() = apply {
if (isPseudoBubble && timestampAsOverlay) {
views.viewStubContainer.root.setPadding(0, 0, 0, 0)
} else {
views.viewStubContainer.root.setPadding(horizontalStubPadding, verticalStubPadding, horizontalStubPadding, verticalStubPadding)
}
}

private fun TimelineMessageLayout.Bubble.setMargins() = apply {
if (isIncoming) {
views.messageEndGuideline.updateLayoutParams<LayoutParams> {
marginEnd = resources.getDimensionPixelSize(R.dimen.chat_bubble_margin_end)
Expand All @@ -141,22 +173,11 @@ class MessageBubbleView @JvmOverloads constructor(context: Context, attrs: Attri
}
}

private fun TimelineMessageLayout.Bubble.CornersRadius.toFloatArray(): FloatArray {
return floatArrayOf(topStartRadius, topStartRadius, topEndRadius, topEndRadius, bottomEndRadius, bottomEndRadius, bottomStartRadius, bottomStartRadius)
private fun TimelineMessageLayout.Bubble.setAdditionalTopSpace() = apply {
views.additionalTopSpace.isVisible = addTopMargin
}

private fun updateDrawables(messageLayout: TimelineMessageLayout.Bubble) {
val shapeAppearanceModel = messageLayout.cornersRadius.shapeAppearanceModel()
bubbleDrawable.apply {
this.shapeAppearanceModel = shapeAppearanceModel
this.fillColor = if (messageLayout.isPseudoBubble) {
ColorStateList.valueOf(Color.TRANSPARENT)
} else {
val backgroundColorAttr = if (isIncoming) R.attr.vctr_message_bubble_inbound else R.attr.vctr_message_bubble_outbound
val backgroundColor = ThemeUtils.getColor(context, backgroundColorAttr)
ColorStateList.valueOf(backgroundColor)
}
}
rippleMaskDrawable.shapeAppearanceModel = shapeAppearanceModel
private fun TimelineMessageLayout.Bubble.CornersRadius.toFloatArray(): FloatArray {
return floatArrayOf(topStartRadius, topStartRadius, topEndRadius, topEndRadius, bottomEndRadius, bottomEndRadius, bottomStartRadius, bottomStartRadius)
}
}