From 1c227e81e4fb53323633ee8da75674760b86e39a Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 4 Mar 2019 14:40:46 -0300 Subject: [PATCH 1/4] [android] update unread counts with a ScrollListener in threaded conversations --- .../threaded/BaseThreadItemViewHolder.java | 1 - .../android/threaded/ThreadItemAdapter.java | 40 +-------- .../android/threaded/ThreadListActivity.java | 59 ++----------- .../threaded/ThreadScrollListener.java | 85 +++++++++++++++++++ .../android/util/ItemReturningAdapter.java | 9 ++ .../view/BriarRecyclerViewScrollListener.java | 80 +++++++++++++++++ 6 files changed, 184 insertions(+), 90 deletions(-) create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadScrollListener.java create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/util/ItemReturningAdapter.java create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/view/BriarRecyclerViewScrollListener.java diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/BaseThreadItemViewHolder.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/BaseThreadItemViewHolder.java index 529fa7d00..a0f54bad3 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/BaseThreadItemViewHolder.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/BaseThreadItemViewHolder.java @@ -51,7 +51,6 @@ public abstract class BaseThreadItemViewHolder } else if (!item.isRead()) { layout.setActivated(true); animateFadeOut(); - listener.onUnreadItemVisible(item); } else { layout.setActivated(false); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemAdapter.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemAdapter.java index 15bd410e6..c9af77016 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemAdapter.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItemAdapter.java @@ -10,6 +10,7 @@ import android.view.ViewGroup; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.briar.R; +import org.briarproject.briar.android.util.ItemReturningAdapter; import org.briarproject.briar.android.util.VersionedAdapter; import java.util.Collection; @@ -21,7 +22,7 @@ import static android.support.v7.widget.RecyclerView.NO_POSITION; @UiThread public class ThreadItemAdapter extends RecyclerView.Adapter> - implements VersionedAdapter { + implements VersionedAdapter, ItemReturningAdapter { static final int UNDEFINED = -1; @@ -136,30 +137,6 @@ public class ThreadItemAdapter return null; } - /** - * Gets the number of unread items above and below the current view port. - * - * Attention: Do not call this when the list is still scrolling, - * because then the view port is unknown. - */ - public UnreadCount getUnreadCount() { - int positionTop = layoutManager.findFirstVisibleItemPosition(); - int positionBottom = layoutManager.findLastVisibleItemPosition(); - if (positionTop == NO_POSITION && positionBottom == NO_POSITION) - return new UnreadCount(0, 0); - - int unreadCounterTop = 0, unreadCounterBottom = 0; - for (int i = 0; i < items.size(); i++) { - I item = items.get(i); - if (i < positionTop && !item.isRead()) { - unreadCounterTop++; - } else if (i > positionBottom && !item.isRead()) { - unreadCounterBottom++; - } - } - return new UnreadCount(unreadCounterTop, unreadCounterBottom); - } - /** * Returns the position of the first unread item below the current viewport */ @@ -188,20 +165,7 @@ public class ThreadItemAdapter return NO_POSITION; } - static class UnreadCount { - - final int top, bottom; - - private UnreadCount(int top, int bottom) { - this.top = top; - this.bottom = bottom; - } - } - public interface ThreadItemListener { - - void onUnreadItemVisible(I item); - void onReplyClick(I item); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListActivity.java index ed0ac5b94..c679ce2ad 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadListActivity.java @@ -9,7 +9,6 @@ import android.support.annotation.UiThread; import android.support.design.widget.Snackbar; import android.support.v7.app.ActionBar; import android.support.v7.widget.LinearLayoutManager; -import android.support.v7.widget.RecyclerView; import android.view.MenuItem; import org.briarproject.bramble.api.contact.ContactId; @@ -43,10 +42,7 @@ import javax.inject.Inject; import static android.support.design.widget.Snackbar.make; import static android.support.v7.widget.RecyclerView.NO_POSITION; -import static android.support.v7.widget.RecyclerView.SCROLL_STATE_IDLE; -import static java.util.logging.Level.INFO; import static org.briarproject.bramble.util.StringUtils.isNullOrEmpty; -import static org.briarproject.briar.android.threaded.ThreadItemAdapter.UnreadCount; @MethodsNotNullByDefault @ParametersNotNullByDefault @@ -61,11 +57,11 @@ public abstract class ThreadListActivity scrollListener; protected BriarRecyclerView list; private LinearLayoutManager layoutManager; protected TextInputView textInput; protected GroupId groupId; - private UnreadMessageButton upButton, downButton; @Nullable private MessageId replyId; @@ -76,7 +72,6 @@ public abstract class ThreadListActivity(adapter, getController(), + upButton, downButton); + list.getRecyclerView().addOnScrollListener(scrollListener); - list.getRecyclerView().addOnScrollListener( - new RecyclerView.OnScrollListener() { - @Override - public void onScrolled(RecyclerView recyclerView, int dx, - int dy) { - super.onScrolled(recyclerView, dx, dy); - if (dx == 0 && dy == 0) { - // scrollToPosition has been called and finished - updateUnreadCount(); - } - } - - @Override - public void onScrollStateChanged(RecyclerView recyclerView, - int newState) { - super.onScrollStateChanged(recyclerView, newState); - if (newState == SCROLL_STATE_IDLE) { - updateUnreadCount(); - } - } - }); - upButton = findViewById(R.id.upButton); - downButton = findViewById(R.id.downButton); upButton.setOnClickListener(v -> { int position = adapter.getVisibleUnreadPosTop(); if (position != NO_POSITION) { @@ -211,7 +187,6 @@ public abstract class ThreadListActivity { + extends BriarAdapter + implements ItemReturningAdapter { private ConversationListener listener; private final RecycledViewPool imageViewPool; @@ -69,7 +71,6 @@ class ConversationAdapter public void onBindViewHolder(ConversationItemViewHolder ui, int position) { ConversationItem item = items.get(position); ui.bind(item); - listener.onItemVisible(item); } @Override diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationItem.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationItem.java index 5ab0ee33c..5fb4c5b4b 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationItem.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationItem.java @@ -69,6 +69,10 @@ abstract class ConversationItem { return read; } + void markRead() { + read = true; + } + /** * Only useful for outgoing messages. */ diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationListener.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationListener.java index 9c8db9538..d7c445fae 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationListener.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationListener.java @@ -9,8 +9,6 @@ import org.briarproject.bramble.api.nullsafety.NotNullByDefault; @NotNullByDefault interface ConversationListener { - void onItemVisible(ConversationItem item); - void respondToRequest(ConversationRequestItem item, boolean accept); void openRequestedShareable(ConversationRequestItem item); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationScrollListener.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationScrollListener.java new file mode 100644 index 000000000..300a8ed3b --- /dev/null +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationScrollListener.java @@ -0,0 +1,26 @@ +package org.briarproject.briar.android.conversation; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.briar.android.view.BriarRecyclerViewScrollListener; + +@NotNullByDefault +class ConversationScrollListener extends + BriarRecyclerViewScrollListener { + + private final ConversationViewModel viewModel; + + protected ConversationScrollListener(ConversationAdapter adapter, + ConversationViewModel viewModel) { + super(adapter); + this.viewModel = viewModel; + } + + @Override + protected void onNewItemVisible(ConversationItem item) { + if (!item.isRead()) { + viewModel.markMessageRead(item.getGroupId(), item.getId()); + item.markRead(); + } + } + +} diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java index 5f93fcd14..c9b3a1cd7 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java @@ -26,6 +26,7 @@ import org.briarproject.bramble.api.settings.Settings; import org.briarproject.bramble.api.settings.SettingsManager; import org.briarproject.bramble.api.sync.GroupId; import org.briarproject.bramble.api.sync.Message; +import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.briar.android.util.UiUtils; import org.briarproject.briar.api.messaging.Attachment; import org.briarproject.briar.api.messaging.AttachmentHeader; @@ -150,6 +151,18 @@ public class ConversationViewModel extends AndroidViewModel { }); } + void markMessageRead(GroupId g, MessageId m) { + dbExecutor.execute(() -> { + try { + long start = now(); + messagingManager.setReadFlag(g, m, true); + logDuration(LOG, "Marking read", start); + } catch (DbException e) { + logException(LOG, WARNING, e); + } + }); + } + void setContactAlias(String alias) { dbExecutor.execute(() -> { try { From ae09b4c607c36cbf6fb409ad59307cecc5a8271c Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 19 Mar 2019 12:35:15 -0300 Subject: [PATCH 3/4] [android] remove complicated logic for detecting new visible items notify after every scroll for all visible items instead --- .../ConversationScrollListener.java | 2 +- .../threaded/ThreadScrollListener.java | 6 ++-- .../view/BriarRecyclerViewScrollListener.java | 33 ++++--------------- 3 files changed, 11 insertions(+), 30 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationScrollListener.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationScrollListener.java index 300a8ed3b..da90aa0cf 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationScrollListener.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationScrollListener.java @@ -16,7 +16,7 @@ class ConversationScrollListener extends } @Override - protected void onNewItemVisible(ConversationItem item) { + protected void onItemVisible(ConversationItem item) { if (!item.isRead()) { viewModel.markMessageRead(item.getGroupId(), item.getId()); item.markRead(); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadScrollListener.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadScrollListener.java index d9f915048..1091d507c 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadScrollListener.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadScrollListener.java @@ -34,14 +34,14 @@ class ThreadScrollListener } @Override - protected void onNewItemVisible(int firstVisible, int lastVisible, + protected void onItemsVisible(int firstVisible, int lastVisible, int itemCount) { - super.onNewItemVisible(firstVisible, lastVisible, itemCount); + super.onItemsVisible(firstVisible, lastVisible, itemCount); updateUnreadButtons(firstVisible, lastVisible, itemCount); } @Override - protected void onNewItemVisible(I item) { + protected void onItemVisible(I item) { if (!item.isRead()) { item.setRead(true); controller.markItemRead(item); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/BriarRecyclerViewScrollListener.java b/briar-android/src/main/java/org/briarproject/briar/android/view/BriarRecyclerViewScrollListener.java index fc7cf1214..73686d601 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/BriarRecyclerViewScrollListener.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/BriarRecyclerViewScrollListener.java @@ -36,7 +36,7 @@ public abstract class BriarRecyclerViewScrollListener visibleItems; - - if (init || jump) { - // If the list has loaded, or we jump scrolled, - // all visible items are newly visible - for (int i = firstVisible; i <= lastVisible; i++) - onNewItemVisible(i); - return; - } - - boolean up = firstVisible > prevFirstVisible || - lastVisible > prevLastVisible; - if (up) { - for (int i = prevLastVisible + 1; i <= lastVisible; i++) - onNewItemVisible(i); - } else { - for (int i = firstVisible; i < prevFirstVisible; i++) - onNewItemVisible(i); + for (int i = firstVisible; i <= lastVisible; i++) { + onItemVisible(i); } } - private void onNewItemVisible(int position) { + private void onItemVisible(int position) { I item = requireNonNull(adapter.getItemAt(position)); - onNewItemVisible(item); + onItemVisible(item); } - protected abstract void onNewItemVisible(I item); + protected abstract void onItemVisible(I item); } From 4c6f68c255ac47edcc3952e281c34e088f790cc7 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 21 Mar 2019 09:59:33 -0300 Subject: [PATCH 4/4] [android] optimize method to update unread counts --- .../android/threaded/ThreadScrollListener.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadScrollListener.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadScrollListener.java index 1091d507c..d8af8a354 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadScrollListener.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadScrollListener.java @@ -61,14 +61,15 @@ class ThreadScrollListener setUnreadButtons(0, 0); return; } - int unreadCounterFirst = 0, unreadCounterLast = 0; - for (int i = 0; i < count; i++) { + int unreadCounterFirst = 0; + for (int i = 0; i < firstVisible; i++) { I item = requireNonNull(adapter.getItemAt(i)); - if (i < firstVisible && !item.isRead()) { - unreadCounterFirst++; - } else if (i > lastVisible && !item.isRead()) { - unreadCounterLast++; - } + if (!item.isRead()) unreadCounterFirst++; + } + int unreadCounterLast = 0; + for (int i = lastVisible + 1; i < count; i++) { + I item = requireNonNull(adapter.getItemAt(i)); + if (!item.isRead()) unreadCounterLast++; } setUnreadButtons(unreadCounterFirst, unreadCounterLast); }