From 4f8d905e7ba6aa1a3634a1bfc267b7139d93765c Mon Sep 17 00:00:00 2001 From: Konrad Pozniak Date: Wed, 22 Aug 2018 21:18:56 +0200 Subject: [PATCH] fix notification tab loading (#777) * fix progressbars of footer and fragment overlapping * add progressbar to bottom of notification list again * fix bottom loading getting stuck sometimes --- .../tusky/adapter/NotificationsAdapter.java | 22 ++---- .../tusky/adapter/PlaceholderViewHolder.java | 19 ++--- .../tusky/adapter/TimelineAdapter.java | 3 +- .../tusky/fragment/NotificationsFragment.java | 75 +++++++++---------- 4 files changed, 49 insertions(+), 70 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/adapter/NotificationsAdapter.java b/app/src/main/java/com/keylesspalace/tusky/adapter/NotificationsAdapter.java index 136805fa..fa71d00e 100644 --- a/app/src/main/java/com/keylesspalace/tusky/adapter/NotificationsAdapter.java +++ b/app/src/main/java/com/keylesspalace/tusky/adapter/NotificationsAdapter.java @@ -64,7 +64,6 @@ public class NotificationsAdapter extends RecyclerView.Adapter { private List notifications; private StatusActionListener statusListener; private NotificationActionListener notificationActionListener; - private FooterViewHolder.State footerState; private boolean mediaPreviewEnabled; private BidiFormatter bidiFormatter; @@ -74,7 +73,6 @@ public class NotificationsAdapter extends RecyclerView.Adapter { notifications = new ArrayList<>(); this.statusListener = statusListener; this.notificationActionListener = notificationActionListener; - footerState = FooterViewHolder.State.END; mediaPreviewEnabled = true; bidiFormatter = BidiFormatter.getInstance(); } @@ -119,7 +117,7 @@ public class NotificationsAdapter extends RecyclerView.Adapter { if (notification instanceof NotificationViewData.Placeholder) { NotificationViewData.Placeholder placeholder = ((NotificationViewData.Placeholder) notification); PlaceholderViewHolder holder = (PlaceholderViewHolder) viewHolder; - holder.setup(!placeholder.isLoading(), statusListener); + holder.setup(statusListener, placeholder.isLoading()); return; } NotificationViewData.Concrete concreteNotificaton = @@ -164,15 +162,12 @@ public class NotificationsAdapter extends RecyclerView.Adapter { break; } } - } else { - FooterViewHolder holder = (FooterViewHolder) viewHolder; - holder.setState(footerState); } } @Override public int getItemCount() { - return notifications.size() + 1; + return notifications.size(); } @Override @@ -224,19 +219,16 @@ public class NotificationsAdapter extends RecyclerView.Adapter { notifyItemRangeInserted(notifications.size(), newNotifications.size()); } + public void removeItemAndNotify(int position) { + notifications.remove(position); + notifyItemRemoved(position); + } + public void clear() { notifications.clear(); notifyDataSetChanged(); } - public void setFooterState(FooterViewHolder.State newFooterState) { - FooterViewHolder.State oldValue = footerState; - footerState = newFooterState; - if (footerState != oldValue) { - notifyItemChanged(notifications.size()); - } - } - public void setMediaPreviewEnabled(boolean enabled) { mediaPreviewEnabled = enabled; } diff --git a/app/src/main/java/com/keylesspalace/tusky/adapter/PlaceholderViewHolder.java b/app/src/main/java/com/keylesspalace/tusky/adapter/PlaceholderViewHolder.java index 2c65ddf5..ed4940df 100644 --- a/app/src/main/java/com/keylesspalace/tusky/adapter/PlaceholderViewHolder.java +++ b/app/src/main/java/com/keylesspalace/tusky/adapter/PlaceholderViewHolder.java @@ -34,21 +34,16 @@ public final class PlaceholderViewHolder extends RecyclerView.ViewHolder { progressBar = itemView.findViewById(R.id.progress_bar); } - public void setup(boolean enabled, final StatusActionListener listener) { - this.setup(enabled, listener, false); - } - - public void setup(boolean enabled, final StatusActionListener listener, boolean progress) { + public void setup(final StatusActionListener listener, boolean progress) { loadMoreButton.setVisibility(progress ? View.GONE : View.VISIBLE); progressBar.setVisibility(progress ? View.VISIBLE : View.GONE); - loadMoreButton.setEnabled(enabled); - if (enabled) { - loadMoreButton.setOnClickListener(v -> { - loadMoreButton.setEnabled(false); - listener.onLoadMore(getAdapterPosition()); - }); - } + loadMoreButton.setEnabled(true); + loadMoreButton.setOnClickListener(v -> { + loadMoreButton.setEnabled(false); + listener.onLoadMore(getAdapterPosition()); + }); + } } \ No newline at end of file diff --git a/app/src/main/java/com/keylesspalace/tusky/adapter/TimelineAdapter.java b/app/src/main/java/com/keylesspalace/tusky/adapter/TimelineAdapter.java index 9c6ef9d4..db7a519c 100644 --- a/app/src/main/java/com/keylesspalace/tusky/adapter/TimelineAdapter.java +++ b/app/src/main/java/com/keylesspalace/tusky/adapter/TimelineAdapter.java @@ -71,8 +71,7 @@ public final class TimelineAdapter extends RecyclerView.Adapter { StatusViewData status = dataSource.getItemAt(position); if (status instanceof StatusViewData.Placeholder) { PlaceholderViewHolder holder = (PlaceholderViewHolder) viewHolder; - holder.setup(!((StatusViewData.Placeholder) status).isLoading(), - statusListener, ((StatusViewData.Placeholder) status).isLoading()); + holder.setup(statusListener, ((StatusViewData.Placeholder) status).isLoading()); } else { StatusViewHolder holder = (StatusViewHolder) viewHolder; holder.setupWithStatus((StatusViewData.Concrete) status, diff --git a/app/src/main/java/com/keylesspalace/tusky/fragment/NotificationsFragment.java b/app/src/main/java/com/keylesspalace/tusky/fragment/NotificationsFragment.java index a2c3c900..1a76a227 100644 --- a/app/src/main/java/com/keylesspalace/tusky/fragment/NotificationsFragment.java +++ b/app/src/main/java/com/keylesspalace/tusky/fragment/NotificationsFragment.java @@ -43,10 +43,9 @@ import android.widget.TextView; import com.keylesspalace.tusky.MainActivity; import com.keylesspalace.tusky.R; -import com.keylesspalace.tusky.adapter.FooterViewHolder; import com.keylesspalace.tusky.adapter.NotificationsAdapter; -import com.keylesspalace.tusky.appstore.EventHub; import com.keylesspalace.tusky.appstore.BlockEvent; +import com.keylesspalace.tusky.appstore.EventHub; import com.keylesspalace.tusky.appstore.FavoriteEvent; import com.keylesspalace.tusky.appstore.ReblogEvent; import com.keylesspalace.tusky.db.AccountEntity; @@ -69,6 +68,7 @@ import com.keylesspalace.tusky.viewdata.NotificationViewData; import com.keylesspalace.tusky.viewdata.StatusViewData; import java.math.BigInteger; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Objects; @@ -132,9 +132,7 @@ public class NotificationsFragment extends SFragment implements private TabLayout.OnTabSelectedListener onTabSelectedListener; private boolean hideFab; private boolean topLoading; - private int topFetches; private boolean bottomLoading; - private int bottomFetches; private String bottomId; private String topId; private boolean alwaysShowSensitiveMedia; @@ -202,15 +200,15 @@ public class NotificationsFragment extends SFragment implements notifications.clear(); topLoading = false; - topFetches = 0; bottomLoading = false; - bottomFetches = 0; bottomId = null; topId = null; ((SimpleItemAnimator) recyclerView.getItemAnimator()).setSupportsChangeAnimations(false); setupNothingView(); + sendFetchNotificationsRequest(null, topId, FetchEnd.TOP, -1); + return rootView; } @@ -551,6 +549,14 @@ public class NotificationsFragment extends SFragment implements } private void onLoadMore() { + Either last = notifications.get(notifications.size() - 1); + if (last.isRight()) { + notifications.add(Either.left(Placeholder.getInstance())); + NotificationViewData viewData = new NotificationViewData.Placeholder(true); + notifications.setPairedItem(notifications.size() - 1, viewData); + recyclerView.post(() -> adapter.addItems(Collections.singletonList(viewData))); + } + sendFetchNotificationsRequest(bottomId, null, FetchEnd.BOTTOM, -1); } @@ -564,19 +570,16 @@ public class NotificationsFragment extends SFragment implements /* If there is a fetch already ongoing, record however many fetches are requested and * fulfill them after it's complete. */ if (fetchEnd == FetchEnd.TOP && topLoading) { - topFetches++; return; } if (fetchEnd == FetchEnd.BOTTOM && bottomLoading) { - bottomFetches++; return; } - - if (fromId != null || adapter.getItemCount() <= 1) { - /* When this is called by the EndlessScrollListener it cannot refresh the footer state - * using adapter.notifyItemChanged. So its necessary to postpone doing so until a - * convenient time for the UI thread using a Runnable. */ - recyclerView.post(() -> adapter.setFooterState(FooterViewHolder.State.LOADING)); + if(fetchEnd == FetchEnd.TOP) { + topLoading = true; + } + if(fetchEnd == FetchEnd.BOTTOM) { + bottomLoading = true; } Call> call = mastodonApi.notifications(fromId, uptoId, LOAD_AT_ONCE); @@ -624,6 +627,13 @@ public class NotificationsFragment extends SFragment implements if (next != null) { fromId = next.uri.getQueryParameter("max_id"); } + + if (!this.notifications.isEmpty() + && !this.notifications.get(this.notifications.size() - 1).isRight()) { + this.notifications.remove(this.notifications.size() - 1); + adapter.removeItemAndNotify(this.notifications.size()); + } + if (adapter.getItemCount() > 1) { addItems(notifications, fromId); } else { @@ -644,11 +654,17 @@ public class NotificationsFragment extends SFragment implements saveNewestNotificationId(notifications); - fulfillAnyQueuedFetches(fetchEnd); - if (notifications.size() == 0 && adapter.getItemCount() == 1) { - adapter.setFooterState(FooterViewHolder.State.EMPTY); + if(fetchEnd == FetchEnd.TOP) { + topLoading = false; + } + if(fetchEnd == FetchEnd.BOTTOM) { + bottomLoading = false; + } + + if (notifications.size() == 0 && adapter.getItemCount() == 0) { + nothingMessageView.setVisibility(View.VISIBLE); } else { - adapter.setFooterState(FooterViewHolder.State.END); + nothingMessageView.setVisibility(View.GONE); } swipeRefreshLayout.setRefreshing(false); progressBar.setVisibility(View.GONE); @@ -663,7 +679,6 @@ public class NotificationsFragment extends SFragment implements adapter.updateItemWithNotify(position, placeholderVD, true); } Log.e(TAG, "Fetch failure: " + exception.getMessage()); - fulfillAnyQueuedFetches(fetchEnd); progressBar.setVisibility(View.GONE); } @@ -710,7 +725,6 @@ public class NotificationsFragment extends SFragment implements notifications.remove(0); } - int newIndex = liftedNew.indexOf(notifications.get(0)); if (newIndex == -1) { if (index == -1 && liftedNew.size() >= LOAD_AT_ONCE) { @@ -743,27 +757,6 @@ public class NotificationsFragment extends SFragment implements } } - private void fulfillAnyQueuedFetches(FetchEnd fetchEnd) { - switch (fetchEnd) { - case BOTTOM: { - bottomLoading = false; - if (bottomFetches > 0) { - bottomFetches--; - onLoadMore(); - } - break; - } - case TOP: { - topLoading = false; - if (topFetches > 0) { - topFetches--; - onRefresh(); - } - break; - } - } - } - private void replacePlaceholderWithNotifications(List newNotifications, int pos) { // Remove placeholder notifications.remove(pos);