From 4952b4470d2b95722abdb48fa934aae4cab28217 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Mon, 14 Jan 2019 15:40:38 -0800 Subject: [PATCH] Fix bug related to gallery selection state. TreeSets are annoying. contains() is calculated with the comparator, which can lead to some weird bugs. Made sure the comparator didn't think two items with the same date were identical. Also fixed stableId generation to avoid any potential weirdness there. --- .../mediapreview/MediaRailAdapter.java | 26 +++++++++++----- .../mediasend/MediaPickerItemAdapter.java | 30 ++++++++++-------- .../securesms/util/StableIdGenerator.java | 31 +++++++++++++++++++ 3 files changed, 66 insertions(+), 21 deletions(-) create mode 100644 src/org/thoughtcrime/securesms/util/StableIdGenerator.java diff --git a/src/org/thoughtcrime/securesms/mediapreview/MediaRailAdapter.java b/src/org/thoughtcrime/securesms/mediapreview/MediaRailAdapter.java index 3bd63f8410..11c6610883 100644 --- a/src/org/thoughtcrime/securesms/mediapreview/MediaRailAdapter.java +++ b/src/org/thoughtcrime/securesms/mediapreview/MediaRailAdapter.java @@ -10,24 +10,29 @@ import org.thoughtcrime.securesms.R; import org.thoughtcrime.securesms.components.ThumbnailView; import org.thoughtcrime.securesms.mediasend.Media; import org.thoughtcrime.securesms.mms.GlideRequests; +import org.thoughtcrime.securesms.util.StableIdGenerator; import java.util.ArrayList; import java.util.List; public class MediaRailAdapter extends RecyclerView.Adapter { - private final GlideRequests glideRequests; - private final List media; - private final RailItemListener listener; - private final boolean deleteEnabled; + private final GlideRequests glideRequests; + private final List media; + private final RailItemListener listener; + private final boolean deleteEnabled; + private final StableIdGenerator stableIdGenerator; private int activePosition; public MediaRailAdapter(@NonNull GlideRequests glideRequests, @NonNull RailItemListener listener, boolean deleteEnabled) { - this.glideRequests = glideRequests; - this.media = new ArrayList<>(); - this.listener = listener; - this.deleteEnabled = deleteEnabled; + this.glideRequests = glideRequests; + this.media = new ArrayList<>(); + this.listener = listener; + this.deleteEnabled = deleteEnabled; + this.stableIdGenerator = new StableIdGenerator<>(); + + setHasStableIds(true); } @NonNull @@ -51,6 +56,11 @@ public class MediaRailAdapter extends RecyclerView.Adapter media) { setMedia(media, activePosition); } diff --git a/src/org/thoughtcrime/securesms/mediasend/MediaPickerItemAdapter.java b/src/org/thoughtcrime/securesms/mediasend/MediaPickerItemAdapter.java index 717c6d517e..03a3d47407 100644 --- a/src/org/thoughtcrime/securesms/mediasend/MediaPickerItemAdapter.java +++ b/src/org/thoughtcrime/securesms/mediasend/MediaPickerItemAdapter.java @@ -13,6 +13,7 @@ import com.bumptech.glide.load.resource.drawable.DrawableTransitionOptions; import org.thoughtcrime.securesms.R; import org.thoughtcrime.securesms.mms.GlideRequests; import org.thoughtcrime.securesms.util.MediaUtil; +import org.thoughtcrime.securesms.util.StableIdGenerator; import java.util.ArrayList; import java.util.Collection; @@ -22,22 +23,25 @@ import java.util.TreeSet; public class MediaPickerItemAdapter extends RecyclerView.Adapter { - private final GlideRequests glideRequests; - private final EventListener eventListener; - private final List media; - private final Set selected; - private final int maxSelection; + private final GlideRequests glideRequests; + private final EventListener eventListener; + private final List media; + private final Set selected; + private final int maxSelection; + private final StableIdGenerator stableIdGenerator; private boolean forcedMultiSelect; public MediaPickerItemAdapter(@NonNull GlideRequests glideRequests, @NonNull EventListener eventListener, int maxSelection) { - this.glideRequests = glideRequests; - this.eventListener = eventListener; - this.media = new ArrayList<>(); - this.maxSelection = maxSelection; - this.selected = new TreeSet<>((m1, m2) -> { - if (m1.equals(m2)) return 0; - else return Long.compare(m2.getDate(), m1.getDate()); + this.glideRequests = glideRequests; + this.eventListener = eventListener; + this.media = new ArrayList<>(); + this.maxSelection = maxSelection; + this.stableIdGenerator = new StableIdGenerator<>(); + this.selected = new TreeSet<>((m1, m2) -> { + if (m1.equals(m2)) return 0; + else if (Long.compare(m2.getDate(), m1.getDate()) == 0) return m2.getUri().compareTo(m1.getUri()); + else return Long.compare(m2.getDate(), m1.getDate()); }); setHasStableIds(true); @@ -65,7 +69,7 @@ public class MediaPickerItemAdapter extends RecyclerView.Adapter media) { diff --git a/src/org/thoughtcrime/securesms/util/StableIdGenerator.java b/src/org/thoughtcrime/securesms/util/StableIdGenerator.java new file mode 100644 index 0000000000..7acb9cc2a5 --- /dev/null +++ b/src/org/thoughtcrime/securesms/util/StableIdGenerator.java @@ -0,0 +1,31 @@ +package org.thoughtcrime.securesms.util; + +import android.support.annotation.MainThread; +import android.support.annotation.NonNull; + +import java.util.HashMap; +import java.util.Map; + +/** + * Useful for generate ID's to be used with + * {@link android.support.v7.widget.RecyclerView.Adapter#getItemId(int)} when you otherwise don't + * have a good way to generate an ID. + */ +public class StableIdGenerator { + + private final Map keys = new HashMap<>(); + + private long index = 1; + + @MainThread + public long getId(@NonNull E item) { + if (keys.containsKey(item)) { + return keys.get(item); + } + + long key = index++; + keys.put(item, key); + + return key; + } +}