fix diffing statuses (#4909)

Several cases of payloads not being forwarded correctly, leading to
unnecessary re-binding of the whole status view. I simplified the logic
by removing the additional list level, making it easier to understand
what is going on.
This commit is contained in:
Konrad Pozniak 2025-02-06 11:38:05 +01:00 committed by GitHub
parent 78152f0449
commit b24427c6df
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 84 additions and 65 deletions

View File

@ -40,7 +40,11 @@ class FilteredStatusViewHolder(
viewData: NotificationViewData.Concrete,
payloads: List<*>,
statusDisplayOptions: StatusDisplayOptions
) = bind(viewData.statusViewData!!)
) {
if (payloads.isEmpty()) {
bind(viewData.statusViewData!!)
}
}
fun bind(viewData: StatusViewData.Concrete) {
val matchedFilterResult: FilterResult? = viewData.actionable.filtered.orEmpty().find { filterResult ->

View File

@ -49,6 +49,9 @@ class FollowRequestViewHolder(
payloads: List<*>,
statusDisplayOptions: StatusDisplayOptions
) {
if (payloads.isNotEmpty()) {
return
}
setupWithAccount(
viewData.account,
statusDisplayOptions.animateAvatars,

View File

@ -779,9 +779,9 @@ public abstract class StatusBaseViewHolder extends RecyclerView.ViewHolder {
public void setupWithStatus(@NonNull StatusViewData.Concrete status,
@NonNull final StatusActionListener listener,
@NonNull StatusDisplayOptions statusDisplayOptions,
@Nullable Object payloads,
@NonNull List<Object> payloads,
final boolean showStatusInfo) {
if (payloads == null) {
if (payloads.isEmpty()) {
Status actionable = status.getActionable();
setDisplayName(actionable.getAccount().getName(), actionable.getAccount().getEmojis(), statusDisplayOptions);
setUsername(actionable.getAccount().getUsername());
@ -832,16 +832,16 @@ public abstract class StatusBaseViewHolder extends RecyclerView.ViewHolder {
// and let RecyclerView ask for a new delegate.
itemView.setAccessibilityDelegate(null);
} else {
if (payloads instanceof List)
for (Object item : (List<?>) payloads) {
if (Key.KEY_CREATED.equals(item)) {
setMetaData(status, statusDisplayOptions, listener);
if (status.getStatus().getCard() != null && status.getStatus().getCard().getPublishedAt() != null) {
// there is a preview card showing the published time, we need to refresh it as well
setupCard(status, status.isExpanded(), statusDisplayOptions.cardViewMode(), statusDisplayOptions, listener);
}
for (Object item : payloads) {
if (Key.KEY_CREATED.equals(item)) {
setMetaData(status, statusDisplayOptions, listener);
if (status.getStatus().getCard() != null && status.getStatus().getCard().getPublishedAt() != null) {
// there is a preview card showing the published time, we need to refresh it as well
setupCard(status, status.isExpanded(), statusDisplayOptions.cardViewMode(), statusDisplayOptions, listener);
}
break;
}
}
}
}
@ -1239,7 +1239,7 @@ public abstract class StatusBaseViewHolder extends RecyclerView.ViewHolder {
builder = builder.placeholder(decodeBlurHash(card.getBlurhash()));
}
builder.centerInside()
.into(cardImage);
.into(cardImage);
} else if (statusDisplayOptions.useBlurhash() && !TextUtils.isEmpty(card.getBlurhash())) {
int radius = cardImage.getContext().getResources()
.getDimensionPixelSize(R.dimen.inner_card_radius);

View File

@ -27,6 +27,7 @@ import com.keylesspalace.tusky.viewdata.StatusViewData;
import java.text.DateFormat;
import java.util.Date;
import java.util.List;
public class StatusDetailedViewHolder extends StatusBaseViewHolder {
private final TextView reblogs;
@ -140,7 +141,7 @@ public class StatusDetailedViewHolder extends StatusBaseViewHolder {
public void setupWithStatus(@NonNull final StatusViewData.Concrete status,
@NonNull final StatusActionListener listener,
@NonNull StatusDisplayOptions statusDisplayOptions,
@Nullable Object payloads,
@NonNull List<Object> payloads,
final boolean showStatusInfo) {
// We never collapse statuses in the detail view
StatusViewData.Concrete uncollapsedStatus = (status.isCollapsible() && status.isCollapsed()) ?
@ -149,7 +150,7 @@ public class StatusDetailedViewHolder extends StatusBaseViewHolder {
super.setupWithStatus(uncollapsedStatus, listener, statusDisplayOptions, payloads, showStatusInfo);
setupCard(uncollapsedStatus, status.isExpanded(), CardViewMode.FULL_WIDTH, statusDisplayOptions, listener); // Always show card for detailed status
if (payloads == null) {
if (payloads.isEmpty()) {
Status actionable = uncollapsedStatus.getActionable();
if (!statusDisplayOptions.hideStats()) {

View File

@ -39,6 +39,7 @@ import com.keylesspalace.tusky.util.StringUtils;
import com.keylesspalace.tusky.viewdata.StatusViewData;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import at.connyduck.sparkbutton.helpers.Utils;
@ -64,9 +65,9 @@ public class StatusViewHolder extends StatusBaseViewHolder {
public void setupWithStatus(@NonNull StatusViewData.Concrete status,
@NonNull final StatusActionListener listener,
@NonNull StatusDisplayOptions statusDisplayOptions,
@Nullable Object payloads,
@NonNull List<Object> payloads,
final boolean showStatusInfo) {
if (payloads == null) {
if (payloads.isEmpty()) {
boolean sensitive = !TextUtils.isEmpty(status.getActionable().getSpoilerText());
boolean expanded = status.isExpanded();

View File

@ -48,13 +48,9 @@ class ConversationAdapter(
onBindViewHolder(holder, position, emptyList())
}
override fun onBindViewHolder(
holder: ConversationViewHolder,
position: Int,
payloads: List<Any>
) {
override fun onBindViewHolder(holder: ConversationViewHolder, position: Int, payloads: List<Any>) {
getItem(position)?.let { conversationViewData ->
holder.setupWithConversation(conversationViewData, payloads.firstOrNull())
holder.setupWithConversation(conversationViewData, payloads)
}
}
@ -80,7 +76,7 @@ class ConversationAdapter(
): Any? {
return if (oldItem == newItem) {
// If items are equal - update timestamp only
listOf(StatusBaseViewHolder.Key.KEY_CREATED)
StatusBaseViewHolder.Key.KEY_CREATED
} else {
// If items are different - update the whole view holder
null

View File

@ -69,7 +69,7 @@ public class ConversationViewHolder extends StatusBaseViewHolder {
void setupWithConversation(
@NonNull ConversationViewData conversation,
@Nullable Object payloads
@Nullable List<Object> payloads
) {
StatusViewData.Concrete statusViewData = conversation.getLastStatus();
@ -118,11 +118,9 @@ public class ConversationViewHolder extends StatusBaseViewHolder {
setAvatars(conversation.getAccounts());
} else {
if (payloads instanceof List) {
for (Object item : (List<?>) payloads) {
if (Key.KEY_CREATED.equals(item)) {
setMetaData(statusViewData, statusDisplayOptions, listener);
}
for (Object item : payloads) {
if (Key.KEY_CREATED.equals(item)) {
setMetaData(statusViewData, statusDisplayOptions, listener);
}
}
}

View File

@ -184,7 +184,7 @@ class ConversationsFragment :
adapter.notifyItemRangeChanged(
0,
adapter.itemCount,
listOf(StatusBaseViewHolder.Key.KEY_CREATED)
StatusBaseViewHolder.Key.KEY_CREATED
)
delay(1.toDuration(DurationUnit.MINUTES))
}

View File

@ -36,6 +36,9 @@ class FollowViewHolder(
payloads: List<*>,
statusDisplayOptions: StatusDisplayOptions
) {
if (payloads.isNotEmpty()) {
return
}
val context = itemView.context
val account = viewData.account
val messageTemplate =

View File

@ -273,7 +273,7 @@ class NotificationsFragment :
adapter.notifyItemRangeChanged(
0,
adapter.itemCount,
listOf(StatusBaseViewHolder.Key.KEY_CREATED)
StatusBaseViewHolder.Key.KEY_CREATED
)
delay(1.toDuration(DurationUnit.MINUTES))
}

View File

@ -143,18 +143,10 @@ class NotificationsPagingAdapter(
}
override fun onBindViewHolder(viewHolder: RecyclerView.ViewHolder, position: Int) {
bindViewHolder(viewHolder, position, emptyList())
onBindViewHolder(viewHolder, position, emptyList())
}
override fun onBindViewHolder(
viewHolder: RecyclerView.ViewHolder,
position: Int,
payloads: List<Any>
) {
bindViewHolder(viewHolder, position, payloads)
}
private fun bindViewHolder(viewHolder: RecyclerView.ViewHolder, position: Int, payloads: List<Any>) {
override fun onBindViewHolder(viewHolder: RecyclerView.ViewHolder, position: Int, payloads: List<Any>) {
getItem(position)?.let { notification ->
when (notification) {
is NotificationViewData.Concrete ->
@ -197,7 +189,7 @@ class NotificationsPagingAdapter(
): Any? {
return if (oldItem == newItem) {
// If items are equal - update timestamp only
listOf(StatusBaseViewHolder.Key.KEY_CREATED)
StatusBaseViewHolder.Key.KEY_CREATED
} else {
// If items are different - update the whole view holder
null

View File

@ -39,6 +39,9 @@ class ReportNotificationViewHolder(
payloads: List<*>,
statusDisplayOptions: StatusDisplayOptions
) {
if (payloads.isNotEmpty()) {
return
}
val report = viewData.report!!
val reporter = viewData.account

View File

@ -47,7 +47,7 @@ internal class StatusViewHolder(
statusViewData,
statusActionListener,
statusDisplayOptions,
payloads.firstOrNull(),
payloads,
false
)
}

View File

@ -20,6 +20,7 @@ import android.view.ViewGroup
import androidx.paging.PagingDataAdapter
import androidx.recyclerview.widget.DiffUtil
import com.keylesspalace.tusky.R
import com.keylesspalace.tusky.adapter.StatusBaseViewHolder
import com.keylesspalace.tusky.adapter.StatusViewHolder
import com.keylesspalace.tusky.interfaces.StatusActionListener
import com.keylesspalace.tusky.util.StatusDisplayOptions
@ -37,23 +38,44 @@ class SearchStatusesAdapter(
}
override fun onBindViewHolder(holder: StatusViewHolder, position: Int) {
onBindViewHolder(holder, position, emptyList())
}
override fun onBindViewHolder(holder: StatusViewHolder, position: Int, payloads: List<Any>) {
getItem(position)?.let { item ->
holder.setupWithStatus(item, statusListener, statusDisplayOptions, null, true)
holder.setupWithStatus(item, statusListener, statusDisplayOptions, payloads, true)
}
}
companion object {
val STATUS_COMPARATOR = object : DiffUtil.ItemCallback<StatusViewData.Concrete>() {
override fun areContentsTheSame(
oldItem: StatusViewData.Concrete,
newItem: StatusViewData.Concrete
): Boolean = oldItem == newItem
override fun areItemsTheSame(
oldItem: StatusViewData.Concrete,
newItem: StatusViewData.Concrete
): Boolean = oldItem.id == newItem.id
): Boolean {
return oldItem.id == newItem.id
}
override fun areContentsTheSame(
oldItem: StatusViewData.Concrete,
newItem: StatusViewData.Concrete
): Boolean {
return false // Items are different always. It allows to refresh timestamp on every view holder update
}
override fun getChangePayload(
oldItem: StatusViewData.Concrete,
newItem: StatusViewData.Concrete
): Any? {
return if (oldItem == newItem) {
// If items are equal - update timestamp only
StatusBaseViewHolder.Key.KEY_CREATED
} else {
// If items are different - update the whole view holder
null
}
}
}
}
}

View File

@ -300,7 +300,7 @@ class TimelineFragment :
adapter.notifyItemRangeChanged(
0,
adapter.itemCount,
listOf(StatusBaseViewHolder.Key.KEY_CREATED)
StatusBaseViewHolder.Key.KEY_CREATED
)
}
}

View File

@ -71,21 +71,13 @@ class TimelinePagingAdapter(
}
override fun onBindViewHolder(viewHolder: RecyclerView.ViewHolder, position: Int) {
bindViewHolder(viewHolder, position, null)
onBindViewHolder(viewHolder, position, emptyList())
}
override fun onBindViewHolder(
viewHolder: RecyclerView.ViewHolder,
position: Int,
payloads: List<*>
) {
bindViewHolder(viewHolder, position, payloads)
}
private fun bindViewHolder(
viewHolder: RecyclerView.ViewHolder,
position: Int,
payloads: List<*>?
payloads: List<Any>
) {
val viewData = getItem(position)
if (viewData is StatusViewData.Placeholder) {
@ -101,7 +93,7 @@ class TimelinePagingAdapter(
viewData,
statusListener,
statusDisplayOptions,
if (payloads != null && payloads.isNotEmpty()) payloads[0] else null,
payloads,
true
)
}
@ -142,7 +134,7 @@ class TimelinePagingAdapter(
override fun getChangePayload(oldItem: StatusViewData, newItem: StatusViewData): Any? {
return if (oldItem == newItem) {
// If items are equal - update timestamp only
listOf(StatusBaseViewHolder.Key.KEY_CREATED)
StatusBaseViewHolder.Key.KEY_CREATED
} else {
// If items are different - update the whole view holder
null

View File

@ -55,11 +55,15 @@ class ThreadAdapter(
}
override fun onBindViewHolder(viewHolder: RecyclerView.ViewHolder, position: Int) {
onBindViewHolder(viewHolder, position, emptyList())
}
override fun onBindViewHolder(viewHolder: RecyclerView.ViewHolder, position: Int, payloads: List<Any>) {
val status = getItem(position)
if (viewHolder is FilteredStatusViewHolder) {
viewHolder.bind(status)
} else if (viewHolder is StatusBaseViewHolder) {
viewHolder.setupWithStatus(status, statusActionListener, statusDisplayOptions, null, false)
viewHolder.setupWithStatus(status, statusActionListener, statusDisplayOptions, payloads, false)
}
}
@ -100,7 +104,7 @@ class ThreadAdapter(
): Any? {
return if (oldItem == newItem) {
// If items are equal - update timestamp only
listOf(StatusBaseViewHolder.Key.KEY_CREATED)
StatusBaseViewHolder.Key.KEY_CREATED
} else {
// If items are different - update the whole view holder
null