From 0426084194c17a01acdd0a74df5167e3ab1d5a88 Mon Sep 17 00:00:00 2001 From: sk Date: Fri, 13 Oct 2023 00:14:29 +0200 Subject: [PATCH] fix changes breaking off-screen bound holders i hope i didn't break everything lol --- .../fragments/BaseStatusListFragment.java | 110 +++++++++++++----- .../report/ReportAddPostsChoiceFragment.java | 4 +- .../displayitems/HeaderStatusDisplayItem.java | 31 +++-- 3 files changed, 106 insertions(+), 39 deletions(-) diff --git a/mastodon/src/main/java/org/joinmastodon/android/fragments/BaseStatusListFragment.java b/mastodon/src/main/java/org/joinmastodon/android/fragments/BaseStatusListFragment.java index 76a8fb993..deb001613 100644 --- a/mastodon/src/main/java/org/joinmastodon/android/fragments/BaseStatusListFragment.java +++ b/mastodon/src/main/java/org/joinmastodon/android/fragments/BaseStatusListFragment.java @@ -33,7 +33,6 @@ import org.joinmastodon.android.model.Translation; import org.joinmastodon.android.ui.BetterItemAnimator; import org.joinmastodon.android.ui.M3AlertDialogBuilder; import org.joinmastodon.android.ui.displayitems.AccountStatusDisplayItem; -import org.joinmastodon.android.ui.displayitems.EmojiReactionsStatusDisplayItem; import org.joinmastodon.android.ui.displayitems.ExtendedFooterStatusDisplayItem; import org.joinmastodon.android.ui.displayitems.FooterStatusDisplayItem; import org.joinmastodon.android.ui.displayitems.GapStatusDisplayItem; @@ -558,21 +557,21 @@ public abstract class BaseStatusListFragment exten public void onVisibilityIconClick(HeaderStatusDisplayItem.Holder holder) { Status status = holder.getItem().status; - MediaGridStatusDisplayItem.Holder mediaGrid = findHolderOfType(holder.getItemID(), MediaGridStatusDisplayItem.Holder.class); - if (mediaGrid != null) { - if (!status.sensitiveRevealed) mediaGrid.revealSensitive(); - else mediaGrid.hideSensitive(); - } else { - // media grid's methods normally change the status' state - we still want to be able - // to do this if the media grid is not bound, tho - so, doing it ourselves here - status.sensitiveRevealed = !status.sensitiveRevealed; - } if(holder.getItem().hasVisibilityToggle) holder.animateVisibilityToggle(false); + MediaGridStatusDisplayItem.Holder mediaGrid=findHolderOfType(holder.getItemID(), MediaGridStatusDisplayItem.Holder.class); + if(mediaGrid!=null){ + if(!status.sensitiveRevealed) mediaGrid.revealSensitive(); + else mediaGrid.hideSensitive(); + }else{ + status.sensitiveRevealed=false; + notifyItemChangedAfter(holder.getItem(), MediaGridStatusDisplayItem.class); + } } public void onSensitiveRevealed(MediaGridStatusDisplayItem.Holder holder) { - HeaderStatusDisplayItem.Holder header = findHolderOfType(holder.getItemID(), HeaderStatusDisplayItem.Holder.class); - if(header != null && header.getItem().hasVisibilityToggle) header.animateVisibilityToggle(true); + HeaderStatusDisplayItem.Holder header=findHolderOfType(holder.getItemID(), HeaderStatusDisplayItem.Holder.class); + if(header!=null && header.getItem().hasVisibilityToggle) header.animateVisibilityToggle(true); + else notifyItemChangedBefore(holder.getItem(), HeaderStatusDisplayItem.class); } protected void toggleSpoiler(Status status, String itemID){ @@ -581,8 +580,8 @@ public abstract class BaseStatusListFragment exten status.sensitiveRevealed = false; SpoilerStatusDisplayItem.Holder spoiler=findHolderOfType(itemID, SpoilerStatusDisplayItem.Holder.class); - if(spoiler!=null) - spoiler.rebind(); + if(spoiler!=null) spoiler.rebind(); + else notifyItemChanged(itemID, SpoilerStatusDisplayItem.class); SpoilerStatusDisplayItem spoilerItem=Objects.requireNonNull(findItemOfType(itemID, SpoilerStatusDisplayItem.class)); int index=displayItems.indexOf(spoilerItem); @@ -594,30 +593,29 @@ public abstract class BaseStatusListFragment exten adapter.notifyItemRangeRemoved(index+1, spoilerItem.contentItems.size()); } - TextStatusDisplayItem.Holder text=findHolderOfType(itemID, TextStatusDisplayItem.Holder.class); - if(text!=null) - adapter.notifyItemChanged(text.getAbsoluteAdapterPosition()-getMainAdapterOffset()); + notifyItemChanged(itemID, TextStatusDisplayItem.class); HeaderStatusDisplayItem.Holder header=findHolderOfType(itemID, HeaderStatusDisplayItem.Holder.class); - if(header!=null) - header.rebind(); + if(header!=null) header.rebind(); + else notifyItemChanged(itemID, HeaderStatusDisplayItem.class); list.invalidateItemDecorations(); } public void onEnableExpandable(TextStatusDisplayItem.Holder holder, boolean expandable) { - if (holder.getItem().status.textExpandable != expandable && list != null) { - holder.getItem().status.textExpandable = expandable; - HeaderStatusDisplayItem.Holder header = findHolderOfType(holder.getItemID(), HeaderStatusDisplayItem.Holder.class); - if (header != null) header.rebind(); + Status s=holder.getItem().status; + if(s.textExpandable!=expandable && list!=null) { + s.textExpandable=expandable; + HeaderStatusDisplayItem.Holder header=findHolderOfType(holder.getItemID(), HeaderStatusDisplayItem.Holder.class); + if(header!=null) header.bindCollapseButton(); } } public void onToggleExpanded(Status status, String itemID) { status.textExpanded = !status.textExpanded; - TextStatusDisplayItem.Holder text=findHolderOfType(itemID, TextStatusDisplayItem.Holder.class); + notifyItemChanged(itemID, TextStatusDisplayItem.class); HeaderStatusDisplayItem.Holder header=findHolderOfType(itemID, HeaderStatusDisplayItem.Holder.class); - if (text != null) text.rebind(); - if (header != null) header.rebind(); + if(header!=null) header.animateExpandToggle(); + else notifyItemChanged(itemID, HeaderStatusDisplayItem.class); } public void onGapClick(GapStatusDisplayItem.Holder item, boolean downwards){} @@ -676,9 +674,61 @@ public abstract class BaseStatusListFragment exten return null; } + /** + * Use this as a fallback if findHolderOfType fails to find the ViewHolder. + * It might still be bound but off-screen and therefore not a child of the RecyclerView - + * resulting in the ViewHolder displaying an outdated state once scrolled back into view. + */ + protected int notifyItemChanged(String id, Class type){ + boolean encounteredParent=false; + for(int i=0; i int notifyItemChangedAfter(StatusDisplayItem afterThis, Class type){ + int startIndex=displayItems.indexOf(afterThis); + if(startIndex == -1) throw new IllegalStateException("notifyItemChangedAfter didn't find the passed StatusDisplayItem"); + String parentID=afterThis.parentID; + for(int i=startIndex; i int notifyItemChangedBefore(StatusDisplayItem beforeThis, Class type){ + int startIndex=displayItems.indexOf(beforeThis); + if(startIndex == -1) throw new IllegalStateException("notifyItemChangedBefore didn't find the passed StatusDisplayItem"); + String parentID=beforeThis.parentID; + for(int i=startIndex; i>=0; i--){ + StatusDisplayItem item=displayItems.get(i); + if(!parentID.equals(item.parentID)) break; // didn't find anything + if(type.isInstance(item)){ + // found it + adapter.notifyItemChanged(i); + return i; + } + } + return -1; + } + @Nullable protected > H findHolderOfType(String id, Class type){ - for(int i=0;i itemHolder && itemHolder.getItemID().equals(id) && type.isInstance(holder)) return type.cast(holder); @@ -800,6 +850,8 @@ public abstract class BaseStatusListFragment exten if(text!=null){ text.updateTranslation(true); imgLoader.bindViewHolder((ImageLoaderRecyclerAdapter) list.getAdapter(), text, text.getAbsoluteAdapterPosition()); + }else{ + notifyItemChanged(itemID, TextStatusDisplayItem.class); } } @@ -811,6 +863,8 @@ public abstract class BaseStatusListFragment exten TextStatusDisplayItem.Holder text=findHolderOfType(itemID, TextStatusDisplayItem.Holder.class); if(text!=null){ text.updateTranslation(true); + }else{ + notifyItemChanged(itemID, TextStatusDisplayItem.class); } new M3AlertDialogBuilder(getActivity()) .setTitle(R.string.error) @@ -827,6 +881,8 @@ public abstract class BaseStatusListFragment exten if(text!=null){ text.updateTranslation(true); imgLoader.bindViewHolder((ImageLoaderRecyclerAdapter) list.getAdapter(), text, text.getAbsoluteAdapterPosition()); + }else{ + notifyItemChanged(itemID, TextStatusDisplayItem.class); } } diff --git a/mastodon/src/main/java/org/joinmastodon/android/fragments/report/ReportAddPostsChoiceFragment.java b/mastodon/src/main/java/org/joinmastodon/android/fragments/report/ReportAddPostsChoiceFragment.java index 34e65cdfa..dabbf898d 100644 --- a/mastodon/src/main/java/org/joinmastodon/android/fragments/report/ReportAddPostsChoiceFragment.java +++ b/mastodon/src/main/java/org/joinmastodon/android/fragments/report/ReportAddPostsChoiceFragment.java @@ -104,8 +104,8 @@ public class ReportAddPostsChoiceFragment extends StatusListFragment{ else selectedIDs.add(id); CheckableHeaderStatusDisplayItem.Holder holder=findHolderOfType(id, CheckableHeaderStatusDisplayItem.Holder.class); - if(holder!=null) - holder.rebind(); + if(holder!=null) holder.rebind(); + else notifyItemChanged(id, CheckableHeaderStatusDisplayItem.class); } @Override diff --git a/mastodon/src/main/java/org/joinmastodon/android/ui/displayitems/HeaderStatusDisplayItem.java b/mastodon/src/main/java/org/joinmastodon/android/ui/displayitems/HeaderStatusDisplayItem.java index afda3b178..7de40a6d9 100644 --- a/mastodon/src/main/java/org/joinmastodon/android/ui/displayitems/HeaderStatusDisplayItem.java +++ b/mastodon/src/main/java/org/joinmastodon/android/ui/displayitems/HeaderStatusDisplayItem.java @@ -377,21 +377,32 @@ public class HeaderStatusDisplayItem extends StatusDisplayItem{ markAsRead.setVisibility(View.GONE); } - if (item.status == null || !item.status.textExpandable) { - collapseBtn.setVisibility(View.GONE); - } else { - String collapseText = item.parentFragment.getString(item.status.textExpanded ? R.string.sk_collapse : R.string.sk_expand); - collapseBtn.setVisibility(item.status.textExpandable ? View.VISIBLE : View.GONE); - collapseBtn.setContentDescription(collapseText); - if (GlobalUserPreferences.reduceMotion) collapseBtnIcon.setScaleY(item.status.textExpanded ? -1 : 1); - else collapseBtnIcon.animate().scaleY(item.status.textExpanded ? -1 : 1).start(); - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) collapseBtn.setTooltipText(collapseText); - } + bindCollapseButton(); itemView.setPaddingRelative(itemView.getPaddingStart(), itemView.getPaddingTop(), item.inset ? V.dp(10) : V.dp(4), itemView.getPaddingBottom()); } + public void bindCollapseButton(){ + boolean expandable=item.status!=null && item.status.textExpandable; + collapseBtn.setVisibility(expandable ? View.VISIBLE : View.GONE); + if(expandable) { + bindCollapseButtonText(); + collapseBtnIcon.setScaleY(item.status.textExpanded ? -1 : 1); + } + } + + private void bindCollapseButtonText(){ + String collapseText = item.parentFragment.getString(item.status.textExpanded ? R.string.sk_collapse : R.string.sk_expand); + collapseBtn.setContentDescription(collapseText); + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) collapseBtn.setTooltipText(collapseText); + } + + public void animateExpandToggle(){ + bindCollapseButtonText(); + collapseBtnIcon.animate().scaleY(item.status.textExpanded ? -1 : 1).start(); + } + public void animateVisibilityToggle(boolean visible){ visibility.animate() .alpha(visible ? 1 : 0)