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.
This commit is contained in:
Greyson Parrelli 2019-01-14 15:40:38 -08:00
parent 5450967d00
commit 4952b4470d
3 changed files with 66 additions and 21 deletions

View File

@ -10,24 +10,29 @@ import org.thoughtcrime.securesms.R;
import org.thoughtcrime.securesms.components.ThumbnailView; import org.thoughtcrime.securesms.components.ThumbnailView;
import org.thoughtcrime.securesms.mediasend.Media; import org.thoughtcrime.securesms.mediasend.Media;
import org.thoughtcrime.securesms.mms.GlideRequests; import org.thoughtcrime.securesms.mms.GlideRequests;
import org.thoughtcrime.securesms.util.StableIdGenerator;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
public class MediaRailAdapter extends RecyclerView.Adapter<MediaRailAdapter.MediaRailViewHolder> { public class MediaRailAdapter extends RecyclerView.Adapter<MediaRailAdapter.MediaRailViewHolder> {
private final GlideRequests glideRequests; private final GlideRequests glideRequests;
private final List<Media> media; private final List<Media> media;
private final RailItemListener listener; private final RailItemListener listener;
private final boolean deleteEnabled; private final boolean deleteEnabled;
private final StableIdGenerator<Media> stableIdGenerator;
private int activePosition; private int activePosition;
public MediaRailAdapter(@NonNull GlideRequests glideRequests, @NonNull RailItemListener listener, boolean deleteEnabled) { public MediaRailAdapter(@NonNull GlideRequests glideRequests, @NonNull RailItemListener listener, boolean deleteEnabled) {
this.glideRequests = glideRequests; this.glideRequests = glideRequests;
this.media = new ArrayList<>(); this.media = new ArrayList<>();
this.listener = listener; this.listener = listener;
this.deleteEnabled = deleteEnabled; this.deleteEnabled = deleteEnabled;
this.stableIdGenerator = new StableIdGenerator<>();
setHasStableIds(true);
} }
@NonNull @NonNull
@ -51,6 +56,11 @@ public class MediaRailAdapter extends RecyclerView.Adapter<MediaRailAdapter.Medi
return media.size(); return media.size();
} }
@Override
public long getItemId(int position) {
return stableIdGenerator.getId(media.get(position));
}
public void setMedia(@NonNull List<Media> media) { public void setMedia(@NonNull List<Media> media) {
setMedia(media, activePosition); setMedia(media, activePosition);
} }

View File

@ -13,6 +13,7 @@ import com.bumptech.glide.load.resource.drawable.DrawableTransitionOptions;
import org.thoughtcrime.securesms.R; import org.thoughtcrime.securesms.R;
import org.thoughtcrime.securesms.mms.GlideRequests; import org.thoughtcrime.securesms.mms.GlideRequests;
import org.thoughtcrime.securesms.util.MediaUtil; import org.thoughtcrime.securesms.util.MediaUtil;
import org.thoughtcrime.securesms.util.StableIdGenerator;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
@ -22,22 +23,25 @@ import java.util.TreeSet;
public class MediaPickerItemAdapter extends RecyclerView.Adapter<MediaPickerItemAdapter.ItemViewHolder> { public class MediaPickerItemAdapter extends RecyclerView.Adapter<MediaPickerItemAdapter.ItemViewHolder> {
private final GlideRequests glideRequests; private final GlideRequests glideRequests;
private final EventListener eventListener; private final EventListener eventListener;
private final List<Media> media; private final List<Media> media;
private final Set<Media> selected; private final Set<Media> selected;
private final int maxSelection; private final int maxSelection;
private final StableIdGenerator<Media> stableIdGenerator;
private boolean forcedMultiSelect; private boolean forcedMultiSelect;
public MediaPickerItemAdapter(@NonNull GlideRequests glideRequests, @NonNull EventListener eventListener, int maxSelection) { public MediaPickerItemAdapter(@NonNull GlideRequests glideRequests, @NonNull EventListener eventListener, int maxSelection) {
this.glideRequests = glideRequests; this.glideRequests = glideRequests;
this.eventListener = eventListener; this.eventListener = eventListener;
this.media = new ArrayList<>(); this.media = new ArrayList<>();
this.maxSelection = maxSelection; this.maxSelection = maxSelection;
this.selected = new TreeSet<>((m1, m2) -> { this.stableIdGenerator = new StableIdGenerator<>();
if (m1.equals(m2)) return 0; this.selected = new TreeSet<>((m1, m2) -> {
else return Long.compare(m2.getDate(), m1.getDate()); 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); setHasStableIds(true);
@ -65,7 +69,7 @@ public class MediaPickerItemAdapter extends RecyclerView.Adapter<MediaPickerItem
@Override @Override
public long getItemId(int position) { public long getItemId(int position) {
return media.get(position).getDate(); return stableIdGenerator.getId(media.get(position));
} }
void setMedia(@NonNull List<Media> media) { void setMedia(@NonNull List<Media> media) {

View File

@ -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<E> {
private final Map<E, Long> 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;
}
}