From b3d3230549a38a5ca35d4fd31f56c934bab67f1e Mon Sep 17 00:00:00 2001 From: Torsten Grote <t@grobox.de> Date: Tue, 3 Jan 2017 16:53:34 -0200 Subject: [PATCH] Remove code for collapsing threads and for reply count --- .../briar/android/forum/ForumActivity.java | 2 +- .../conversation/GroupActivity.java | 2 +- .../conversation/GroupMessageAdapter.java | 7 +- .../conversation/GroupMessageItem.java | 6 +- .../conversation/JoinMessageItem.java | 5 - .../JoinMessageItemViewHolder.java | 8 +- .../threaded/BaseThreadItemViewHolder.java | 26 +- .../briar/android/threaded/ThreadItem.java | 22 +- .../android/threaded/ThreadItemAdapter.java | 327 +++--------------- .../android/threaded/ThreadListActivity.java | 54 ++- .../threaded/ThreadPostViewHolder.java | 52 +-- .../src/main/res/layout/list_item_forum.xml | 2 +- .../src/main/res/layout/list_item_group.xml | 2 +- .../layout/list_item_group_join_notice.xml | 30 +- .../main/res/layout/list_item_invitations.xml | 2 +- .../main/res/layout/list_item_rss_feed.xml | 2 +- .../src/main/res/layout/list_item_thread.xml | 54 +-- briar-android/src/main/res/values/styles.xml | 4 +- .../android/forum/ForumActivityTest.java | 23 +- .../briar/api/client/MessageTree.java | 2 - .../briar/client/MessageTreeImpl.java | 1 - 21 files changed, 157 insertions(+), 476 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/forum/ForumActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/forum/ForumActivity.java index 1967e2cd00..ee82e165b7 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/forum/ForumActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/forum/ForumActivity.java @@ -100,7 +100,7 @@ public class ForumActivity extends super.onActivityResult(request, result, data); if (request == REQUEST_SHARE_FORUM && result == RESULT_OK) { - displaySnackbarShort(R.string.forum_shared_snackbar); + displaySnackbar(R.string.forum_shared_snackbar); } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupActivity.java index 8abb679a7b..feaddbec9c 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupActivity.java @@ -183,7 +183,7 @@ public class GroupActivity extends @Override protected void onActivityResult(int request, int result, Intent data) { if (request == REQUEST_GROUP_INVITE && result == RESULT_OK) { - displaySnackbarShort(R.string.groups_invitation_sent); + displaySnackbar(R.string.groups_invitation_sent); } else super.onActivityResult(request, result, data); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupMessageAdapter.java b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupMessageAdapter.java index dbf8c7b96f..b511804f3c 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupMessageAdapter.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupMessageAdapter.java @@ -31,9 +31,8 @@ class GroupMessageAdapter extends ThreadItemAdapter<GroupMessageItem> { @LayoutRes @Override public int getItemViewType(int position) { - GroupMessageItem item = getVisibleItem(position); - if (item != null) return item.getLayout(); - return R.layout.list_item_thread; + GroupMessageItem item = items.get(position); + return item.getLayout(); } @Override @@ -58,7 +57,7 @@ class GroupMessageAdapter extends ThreadItemAdapter<GroupMessageItem> { GroupMessageItem item = items.get(position); if (item instanceof JoinMessageItem) { ((JoinMessageItem) item).setVisibility(v); - notifyItemChanged(getVisiblePos(item), item); + notifyItemChanged(findItemPosition(item), item); } } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupMessageItem.java b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupMessageItem.java index 79ca011d28..a64769fdff 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupMessageItem.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/GroupMessageItem.java @@ -1,6 +1,7 @@ package org.briarproject.briar.android.privategroup.conversation; import android.support.annotation.LayoutRes; +import android.support.annotation.Nullable; import android.support.annotation.UiThread; import org.briarproject.bramble.api.identity.Author; @@ -20,9 +21,8 @@ class GroupMessageItem extends ThreadItem { private final GroupId groupId; private GroupMessageItem(MessageId messageId, GroupId groupId, - MessageId parentId, - String text, long timestamp, Author author, Status status, - boolean isRead) { + @Nullable MessageId parentId, String text, long timestamp, + Author author, Status status, boolean isRead) { super(messageId, parentId, text, timestamp, author, status, isRead); this.groupId = groupId; } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/JoinMessageItem.java b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/JoinMessageItem.java index dfdd920223..405a862d08 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/JoinMessageItem.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/JoinMessageItem.java @@ -27,11 +27,6 @@ class JoinMessageItem extends GroupMessageItem { return 0; } - @Override - public boolean hasDescendants() { - return false; - } - @Override @LayoutRes public int getLayout() { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/JoinMessageItemViewHolder.java b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/JoinMessageItemViewHolder.java index 5fe8cf3996..6d6a1f216d 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/JoinMessageItemViewHolder.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/conversation/JoinMessageItemViewHolder.java @@ -12,7 +12,6 @@ import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.R; import org.briarproject.briar.android.privategroup.reveal.RevealContactsActivity; import org.briarproject.briar.android.threaded.BaseThreadItemViewHolder; -import org.briarproject.briar.android.threaded.ThreadItemAdapter; import org.briarproject.briar.android.threaded.ThreadItemAdapter.ThreadItemListener; import static org.briarproject.bramble.api.identity.Author.Status.OURSELVES; @@ -41,10 +40,9 @@ class JoinMessageItemViewHolder } @Override - public void bind(ThreadItemAdapter<GroupMessageItem> adapter, - ThreadItemListener<GroupMessageItem> listener, - GroupMessageItem item, int pos) { - super.bind(adapter, listener, item, pos); + public void bind(GroupMessageItem item, + ThreadItemListener<GroupMessageItem> listener) { + super.bind(item, listener); if (isCreator) bindForCreator((JoinMessageItem) item); else bind((JoinMessageItem) item); 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 a018652374..b7fdeff1af 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 @@ -20,9 +20,6 @@ import org.briarproject.briar.R; import org.briarproject.briar.android.threaded.ThreadItemAdapter.ThreadItemListener; import org.briarproject.briar.android.view.AuthorView; -import static android.view.View.INVISIBLE; -import static android.view.View.VISIBLE; - @UiThread @NotNullByDefault public abstract class BaseThreadItemViewHolder<I extends ThreadItem> @@ -33,7 +30,6 @@ public abstract class BaseThreadItemViewHolder<I extends ThreadItem> protected final TextView textView; private final ViewGroup layout; private final AuthorView author; - private final View topDivider; public BaseThreadItemViewHolder(View v) { super(v); @@ -41,43 +37,30 @@ public abstract class BaseThreadItemViewHolder<I extends ThreadItem> layout = (ViewGroup) v.findViewById(R.id.layout); textView = (TextView) v.findViewById(R.id.text); author = (AuthorView) v.findViewById(R.id.author); - topDivider = v.findViewById(R.id.top_divider); } - // TODO improve encapsulation, so we don't need to pass the adapter here @CallSuper - public void bind(final ThreadItemAdapter<I> adapter, - final ThreadItemListener<I> listener, final I item, int pos) { - + public void bind(final I item, final ThreadItemListener<I> listener) { textView.setText(StringUtils.trim(item.getText())); - if (pos == 0) { - topDivider.setVisibility(INVISIBLE); - } else { - topDivider.setVisibility(VISIBLE); - } - author.setAuthor(item.getAuthor()); author.setDate(item.getTimestamp()); author.setAuthorStatus(item.getStatus()); - if (item.equals(adapter.getReplyItem())) { + if (item.isHighlighted()) { layout.setActivated(true); } else if (!item.isRead()) { layout.setActivated(true); - animateFadeOut(adapter, item); + animateFadeOut(); listener.onUnreadItemVisible(item); } else { layout.setActivated(false); } } - private void animateFadeOut(final ThreadItemAdapter<I> adapter, - final I addedItem) { - + private void animateFadeOut() { setIsRecyclable(false); ValueAnimator anim = new ValueAnimator(); - adapter.addAnimatingItem(addedItem, anim); ColorDrawable viewColor = new ColorDrawable(ContextCompat .getColor(getContext(), R.color.forum_cell_highlight)); anim.setIntValues(viewColor.getColor(), ContextCompat @@ -94,7 +77,6 @@ public abstract class BaseThreadItemViewHolder<I extends ThreadItem> R.drawable.list_item_thread_background); layout.setActivated(false); setIsRecyclable(true); - adapter.removeAnimatingItem(addedItem); } @Override public void onAnimationCancel(Animator animation) { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItem.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItem.java index 39d00f98f2..62431da9a1 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItem.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadItem.java @@ -23,9 +23,7 @@ public abstract class ThreadItem implements MessageNode { private final Author author; private final Status status; private int level = UNDEFINED; - private boolean isShowingDescendants = true; - private int descendantCount = 0; - private boolean isRead; + private boolean isRead, highlighted; public ThreadItem(MessageId messageId, @Nullable MessageId parentId, String text, long timestamp, Author author, Status status, @@ -37,6 +35,7 @@ public abstract class ThreadItem implements MessageNode { this.author = author; this.status = status; this.isRead = isRead; + this.highlighted = false; } public String getText() { @@ -71,19 +70,11 @@ public abstract class ThreadItem implements MessageNode { return status; } - public boolean isShowingDescendants() { - return isShowingDescendants; - } - @Override public void setLevel(int level) { this.level = level; } - public void setShowingDescendants(boolean showingDescendants) { - this.isShowingDescendants = showingDescendants; - } - public boolean isRead() { return isRead; } @@ -92,13 +83,12 @@ public abstract class ThreadItem implements MessageNode { isRead = read; } - public boolean hasDescendants() { - return descendantCount > 0; + public void setHighlighted(boolean highlighted) { + this.highlighted = highlighted; } - @Override - public void setDescendantCount(int descendantCount) { - this.descendantCount = descendantCount; + public boolean isHighlighted() { + return highlighted; } } 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 f663a219c2..72b03a03fc 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 @@ -1,6 +1,5 @@ package org.briarproject.briar.android.threaded; -import android.animation.ValueAnimator; import android.support.annotation.Nullable; import android.support.annotation.UiThread; import android.support.v7.widget.LinearLayoutManager; @@ -13,14 +12,11 @@ import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.briar.R; import org.briarproject.briar.android.util.VersionedAdapter; -import java.util.ArrayList; import java.util.Collection; -import java.util.HashMap; -import java.util.List; -import java.util.Map; import static android.support.v7.widget.RecyclerView.NO_POSITION; +@UiThread public class ThreadItemAdapter<I extends ThreadItem> extends RecyclerView.Adapter<BaseThreadItemViewHolder<I>> implements VersionedAdapter { @@ -28,13 +24,9 @@ public class ThreadItemAdapter<I extends ThreadItem> static final int UNDEFINED = -1; protected final NestedTreeList<I> items = new NestedTreeList<>(); - private final Map<I, ValueAnimator> animatingItems = new HashMap<>(); private final ThreadItemListener<I> listener; private final LinearLayoutManager layoutManager; - @Nullable - private I replyItem; - private volatile int revision = 0; public ThreadItemAdapter(ThreadItemListener<I> listener, @@ -53,24 +45,23 @@ public class ThreadItemAdapter<I extends ThreadItem> @Override public void onBindViewHolder(BaseThreadItemViewHolder<I> ui, int position) { - I item = getVisibleItem(position); - if (item == null) return; - ui.bind(this, listener, item, position); + I item = items.get(position); + ui.bind(item, listener); } - /** - * Contrary to the super class adapter, - * this returns the number of <b>visible</b> items, - * not all items in the dataset. - */ @Override public int getItemCount() { - return getVisiblePos(null); + return items.size(); } - @Nullable - I getReplyItem() { - return replyItem; + @Override + public int getRevision() { + return revision; + } + + @Override + public void incrementRevision() { + revision++; } public void setItems(Collection<I> items) { @@ -81,233 +72,51 @@ public class ThreadItemAdapter<I extends ThreadItem> public void add(I item) { items.add(item); - if (item.getParentId() == null) { - notifyItemInserted(getVisiblePos(item)); - } else { - // Try to find the item's parent and perform the proper ui update if - // it's present and visible. - for (int i = items.indexOf(item) - 1; i >= 0; i--) { - I higherItem = items.get(i); - if (higherItem.getLevel() < item.getLevel()) { - // parent found - if (higherItem.isShowingDescendants()) { - int parentVisiblePos = getVisiblePos(higherItem); - if (parentVisiblePos != NO_POSITION) { - // parent is visible, we need to update its ui - notifyItemChanged(parentVisiblePos); - // new item insert ui - int visiblePos = getVisiblePos(item); - notifyItemInserted(visiblePos); - break; - } - } else { - // do not show the new item if its parent is not showing - // descendants (this can be overridden by the user by - // pressing the snack bar) - break; - } - } - } - } - } - - void scrollTo(I item) { - int visiblePos = getVisiblePos(item); - MessageId parentId = item.getParentId(); - if (visiblePos == NO_POSITION && parentId != null) { - // The item is not visible due to being hidden by its parent item. - // Find the parent and make it visible and traverse up the parent - // chain if necessary to make the item visible - for (int i = items.indexOf(item) - 1; i >= 0; i--) { - I higherItem = items.get(i); - if (higherItem.getId().equals(parentId)) { - // parent found - showDescendants(higherItem); - int parentPos = getVisiblePos(higherItem); - if (parentPos != NO_POSITION) { - // parent or ancestor is visible, item's visibility - // is ensured - notifyItemChanged(parentPos); - visiblePos = parentPos; - break; - } - // parent or ancestor is hidden, we need to continue up the - // dependency chain - parentId = higherItem.getParentId(); - if (parentId == null) throw new AssertionError(); - } - } - } - if (visiblePos != NO_POSITION) - layoutManager.scrollToPositionWithOffset(visiblePos, 0); + notifyItemInserted(findItemPosition(item)); } - int getReplyCount(I item) { - int counter = 0; - int pos = items.indexOf(item); - if (pos >= 0) { - int ancestorLvl = item.getLevel(); - for (int i = pos + 1; i < items.size(); i++) { - int descendantLvl = items.get(i).getLevel(); - if (descendantLvl <= ancestorLvl) - break; - if (descendantLvl == ancestorLvl + 1) - counter++; - } - } - return counter; - } - - void setReplyItem(@Nullable I item) { - if (replyItem != null) { - notifyItemChanged(getVisiblePos(replyItem)); - } - replyItem = item; - if (replyItem != null) { - notifyItemChanged(getVisiblePos(replyItem)); - } - } - - void setReplyItemById(MessageId id) { - for (I item : items) { - if (item.getId().equals(id)) { - setReplyItem(item); - break; - } - } - } - - private List<Integer> getSubTreeIndexes(int pos, int levelLimit) { - List<Integer> indexList = new ArrayList<>(); - - for (int i = pos + 1; i < getItemCount(); i++) { - I item = getVisibleItem(i); - if (item != null && item.getLevel() > levelLimit) { - indexList.add(i); - } else { - break; - } - } - return indexList; - } - - public void showDescendants(I item) { - item.setShowingDescendants(true); - int visiblePos = getVisiblePos(item); - List<Integer> indexList = - getSubTreeIndexes(visiblePos, item.getLevel()); - if (!indexList.isEmpty()) { - if (indexList.size() == 1) { - notifyItemInserted(indexList.get(0)); - } else { - notifyItemRangeInserted(indexList.get(0), - indexList.size()); - } + @Nullable + public I getItemAt(int position) { + if (position == NO_POSITION || position >= items.size()) { + return null; } + return items.get(position); } - public void hideDescendants(I item) { - int visiblePos = getVisiblePos(item); - List<Integer> indexList = - getSubTreeIndexes(visiblePos, item.getLevel()); - if (!indexList.isEmpty()) { - // stop animating children - for (int index : indexList) { - ValueAnimator anim = animatingItems.get(items.get(index)); - if (anim != null && anim.isRunning()) { - anim.cancel(); - } - } - if (indexList.size() == 1) { - notifyItemRemoved(indexList.get(0)); - } else { - notifyItemRangeRemoved(indexList.get(0), - indexList.size()); - } + protected int findItemPosition(@Nullable I item) { + for (int i = 0; i < items.size(); i++) { + if (items.get(i).equals(item)) return i; } - item.setShowingDescendants(false); + return NO_POSITION; // Not found } - /** - * Returns the visible item at the given position + * Highlights the item with the given {@link MessageId} + * and disables the highlight for a previously highlighted item, if any. * - * @param position is visible item index - * @return the visible item at index 'position' from an ordered list of - * visible items, or null if 'position' is larger than the number of - * visible items. + * Only one item can be highlighted at a time. */ - @Nullable - public I getVisibleItem(int position) { - int levelLimit = UNDEFINED; - for (I item : items) { - if (levelLimit >= 0) { - // skip hidden items that their parent is hiding - if (item.getLevel() > levelLimit) { - continue; - } - levelLimit = UNDEFINED; - } - if (!item.isShowingDescendants()) { - levelLimit = item.getLevel(); - } - if (position-- == 0) { - return item; + void setHighlightedItem(@Nullable MessageId id) { + for (int i = 0; i < items.size(); i++) { + I item = items.get(i); + if (id != null && item.getId().equals(id)) { + item.setHighlighted(true); + notifyItemChanged(i, item); + } else if (item.isHighlighted()) { + item.setHighlighted(false); + notifyItemChanged(i, item); } } - return null; } - /** - * Returns the visible position of the given item. - * - * @param item the item to find the visible position of, or null to - * return the total count of visible items. - * @return the visible position of 'item', or the total number of visible - * items if 'item' is null. If 'item' is not visible, NO_POSITION is - * returned. - */ - protected int getVisiblePos(@Nullable I item) { - int visibleCounter = 0; - int levelLimit = UNDEFINED; + @Nullable + I getHighlightedItem() { for (I i : items) { - if (levelLimit >= 0) { - if (i.getLevel() > levelLimit) { - // skip all the items below a non visible branch - continue; - } - levelLimit = UNDEFINED; - } - if (item != null && item.equals(i)) { - return visibleCounter; - } else if (!i.isShowingDescendants()) { - levelLimit = i.getLevel(); - } - visibleCounter++; + if (i.isHighlighted()) return i; } - return item == null ? visibleCounter : NO_POSITION; - } - - void addAnimatingItem(I item, ValueAnimator anim) { - animatingItems.put(item, anim); - } - - void removeAnimatingItem(I item) { - animatingItems.remove(item); - } - - @Override - public int getRevision() { - return revision; - } - - @UiThread - @Override - public void incrementRevision() { - revision++; + return null; } - /** * Gets the number of unread items above and below the current view port. * @@ -321,24 +130,13 @@ public class ThreadItemAdapter<I extends ThreadItem> return new UnreadCount(0, 0); int unreadCounterTop = 0, unreadCounterBottom = 0; - int visibleCounter = 0; - int levelLimit = UNDEFINED; - for (I i : items) { - if (levelLimit >= 0) { - if (i.getLevel() > levelLimit) { - // skip all the items below a non visible branch - continue; - } - levelLimit = UNDEFINED; - } - if (visibleCounter > positionBottom && !i.isRead()) { - unreadCounterBottom++; - } else if (visibleCounter < positionTop && !i.isRead()) { + for (int i = 0; i < items.size(); i++) { + I item = items.get(i); + if (i < positionTop && !item.isRead()) { unreadCounterTop++; - } else if (!i.isShowingDescendants()) { - levelLimit = i.getLevel(); + } else if (i > positionBottom && !item.isRead()) { + unreadCounterBottom++; } - visibleCounter++; } return new UnreadCount(unreadCounterTop, unreadCounterBottom); } @@ -348,22 +146,9 @@ public class ThreadItemAdapter<I extends ThreadItem> */ public int getVisibleUnreadPosBottom() { final int positionBottom = layoutManager.findLastVisibleItemPosition(); - int visibleCounter = 0; - int levelLimit = UNDEFINED; - for (I i : items) { - if (levelLimit >= 0) { - if (i.getLevel() > levelLimit) { - // skip all the items below a non visible branch - continue; - } - levelLimit = UNDEFINED; - } - if (visibleCounter > positionBottom && !i.isRead()) { - return visibleCounter; - } else if (!i.isShowingDescendants()) { - levelLimit = i.getLevel(); - } - visibleCounter++; + if (positionBottom == NO_POSITION) return NO_POSITION; + for (int i = positionBottom + 1; i < items.size(); i++) { + if (!items.get(i).isRead()) return i; } return NO_POSITION; } @@ -374,24 +159,12 @@ public class ThreadItemAdapter<I extends ThreadItem> public int getVisibleUnreadPosTop() { final int positionTop = layoutManager.findFirstVisibleItemPosition(); int position = NO_POSITION; - int visibleCounter = 0; - int levelLimit = UNDEFINED; - for (I i : items) { - if (levelLimit >= 0) { - if (i.getLevel() > levelLimit) { - // skip all the items below a non visible branch - continue; - } - levelLimit = UNDEFINED; - } - if (visibleCounter < positionTop && !i.isRead()) { - position = visibleCounter; - } if (visibleCounter >= positionTop) { + for (int i = 0; i < items.size(); i++) { + if (i < positionTop && !items.get(i).isRead()) { + position = i; + } else if (i >= positionTop) { return position; - } else if (!i.isShowingDescendants()) { - levelLimit = i.getLevel(); } - visibleCounter++; } return NO_POSITION; } 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 c43ce66e02..77ef469716 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 @@ -3,6 +3,7 @@ package org.briarproject.briar.android.threaded; import android.content.Intent; import android.os.Bundle; import android.support.annotation.CallSuper; +import android.support.annotation.Nullable; import android.support.annotation.StringRes; import android.support.annotation.UiThread; import android.support.design.widget.Snackbar; @@ -32,11 +33,11 @@ import org.briarproject.briar.android.view.TextInputView.TextInputListener; import org.briarproject.briar.android.view.UnreadMessageButton; import org.briarproject.briar.api.client.NamedGroup; import org.briarproject.briar.api.client.PostHeader; +import org.thoughtcrime.securesms.components.KeyboardAwareLinearLayout; import java.util.Collection; import java.util.logging.Logger; -import javax.annotation.Nullable; import javax.inject.Inject; import static android.support.design.widget.Snackbar.make; @@ -62,9 +63,11 @@ public abstract class ThreadListActivity<G extends NamedGroup, A extends ThreadI protected A adapter; protected BriarRecyclerView list; + private LinearLayoutManager layoutManager; protected TextInputView textInput; protected GroupId groupId; private UnreadMessageButton upButton, downButton; + @Nullable private MessageId replyId; protected abstract ThreadListController<G, I, H> getController(); @@ -89,9 +92,9 @@ public abstract class ThreadListActivity<G extends NamedGroup, A extends ThreadI textInput.setVisibility(GONE); textInput.setListener(this); list = (BriarRecyclerView) findViewById(R.id.list); - LinearLayoutManager linearLayoutManager = new LinearLayoutManager(this); - list.setLayoutManager(linearLayoutManager); - adapter = createAdapter(linearLayoutManager); + layoutManager = new LinearLayoutManager(this); + list.setLayoutManager(layoutManager); + adapter = createAdapter(layoutManager); list.setAdapter(adapter); list.getRecyclerView().addOnScrollListener( @@ -179,7 +182,7 @@ public abstract class ThreadListActivity<G extends NamedGroup, A extends ThreadI adapter.setItems(items); list.showData(); if (replyId != null) - adapter.setReplyItemById(replyId); + adapter.setHighlightedItem(replyId); } } else { LOG.info("Concurrent update, reloading"); @@ -240,7 +243,7 @@ public abstract class ThreadListActivity<G extends NamedGroup, A extends ThreadI super.onSaveInstanceState(outState); boolean visible = textInput.getVisibility() == VISIBLE; outState.putBoolean(KEY_INPUT_VISIBILITY, visible); - ThreadItem replyItem = adapter.getReplyItem(); + ThreadItem replyItem = adapter.getHighlightedItem(); if (replyItem != null) { outState.putByteArray(KEY_REPLY_ID, replyItem.getId().getBytes()); } @@ -261,7 +264,7 @@ public abstract class ThreadListActivity<G extends NamedGroup, A extends ThreadI public void onBackPressed() { if (textInput.getVisibility() == VISIBLE) { textInput.setVisibility(GONE); - adapter.setReplyItem(null); + adapter.setHighlightedItem(null); } else { super.onBackPressed(); } @@ -276,8 +279,21 @@ public abstract class ThreadListActivity<G extends NamedGroup, A extends ThreadI } @Override - public void onReplyClick(I item) { + public void onReplyClick(final I item) { showTextInput(item); + if (textInput.isKeyboardOpen()) { + scrollToItemAtTop(item); + } else { + // wait with scrolling until keyboard opened + textInput.addOnKeyboardShownListener( + new KeyboardAwareLinearLayout.OnKeyboardShownListener() { + @Override + public void onKeyboardShown() { + scrollToItemAtTop(item); + textInput.removeOnKeyboardShownListener(this); + } + }); + } } @Override @@ -300,7 +316,15 @@ public abstract class ThreadListActivity<G extends NamedGroup, A extends ThreadI } } - protected void displaySnackbarShort(@StringRes int stringId) { + private void scrollToItemAtTop(I item) { + int position = adapter.findItemPosition(item); + if (position != NO_POSITION) { + layoutManager + .scrollToPositionWithOffset(position, 0); + } + } + + protected void displaySnackbar(@StringRes int stringId) { Snackbar snackbar = make(list, stringId, Snackbar.LENGTH_SHORT); snackbar.getView().setBackgroundResource(R.color.briar_primary); snackbar.show(); @@ -318,7 +342,8 @@ public abstract class ThreadListActivity<G extends NamedGroup, A extends ThreadI textInput.showSoftKeyboard(); textInput.setHint(replyItem == null ? R.string.forum_new_message_hint : R.string.forum_message_reply_hint); - adapter.setReplyItem(replyItem); + adapter.setHighlightedItem( + replyItem == null ? null : replyItem.getId()); } @Override @@ -326,10 +351,10 @@ public abstract class ThreadListActivity<G extends NamedGroup, A extends ThreadI if (text.trim().length() == 0) return; if (StringUtils.utf8IsTooLong(text, getMaxBodyLength())) { - displaySnackbarShort(R.string.text_too_long); + displaySnackbar(R.string.text_too_long); return; } - I replyItem = adapter.getReplyItem(); + I replyItem = adapter.getHighlightedItem(); UiResultExceptionHandler<I, DbException> handler = new UiResultExceptionHandler<I, DbException>(this) { @Override @@ -346,7 +371,7 @@ public abstract class ThreadListActivity<G extends NamedGroup, A extends ThreadI textInput.hideSoftKeyboard(); textInput.setVisibility(GONE); textInput.setText(""); - adapter.setReplyItem(null); + adapter.setHighlightedItem(null); } protected abstract int getMaxBodyLength(); @@ -377,7 +402,8 @@ public abstract class ThreadListActivity<G extends NamedGroup, A extends ThreadI adapter.add(item); if (isLocal) { - displaySnackbarShort(getItemPostedString()); + displaySnackbar(getItemPostedString()); + scrollToItemAtTop(item); } else { updateUnreadCount(); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadPostViewHolder.java b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadPostViewHolder.java index 5bdca7ba2f..5719f9a0e0 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadPostViewHolder.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/threaded/ThreadPostViewHolder.java @@ -16,32 +16,29 @@ import static android.view.View.VISIBLE; public class ThreadPostViewHolder<I extends ThreadItem> extends BaseThreadItemViewHolder<I> { - private final TextView lvlText, repliesText; + private final TextView lvlText; private final View[] lvls; - private final View chevron, replyButton; + private final View replyButton; + + private final static int[] nestedLineIds = { + R.id.nested_line_1, R.id.nested_line_2, R.id.nested_line_3, + R.id.nested_line_4, R.id.nested_line_5 + }; public ThreadPostViewHolder(View v) { super(v); lvlText = (TextView) v.findViewById(R.id.nested_line_text); - repliesText = (TextView) v.findViewById(R.id.replies); - int[] nestedLineIds = { - R.id.nested_line_1, R.id.nested_line_2, R.id.nested_line_3, - R.id.nested_line_4, R.id.nested_line_5 - }; lvls = new View[nestedLineIds.length]; for (int i = 0; i < lvls.length; i++) { lvls[i] = v.findViewById(nestedLineIds[i]); } - chevron = v.findViewById(R.id.chevron); replyButton = v.findViewById(R.id.btn_reply); } - // TODO improve encapsulation, so we don't need to pass the adapter here @Override - public void bind(final ThreadItemAdapter<I> adapter, - final ThreadItemListener<I> listener, final I item, int pos) { - super.bind(adapter, listener, item, pos); + public void bind(final I item, final ThreadItemListener<I> listener) { + super.bind(item, listener); for (int i = 0; i < lvls.length; i++) { lvls[i].setVisibility(i < item.getLevel() ? VISIBLE : GONE); @@ -53,41 +50,10 @@ public class ThreadPostViewHolder<I extends ThreadItem> lvlText.setVisibility(GONE); } - int replies = adapter.getReplyCount(item); - if (replies == 0) { - repliesText.setText(""); - } else { - repliesText.setText(getContext().getResources() - .getQuantityString(R.plurals.message_replies, replies, - replies)); - } - - if (item.hasDescendants()) { -// chevron.setVisibility(VISIBLE); - if (item.isShowingDescendants()) { - chevron.setSelected(false); - } else { - chevron.setSelected(true); - } - chevron.setOnClickListener(new View.OnClickListener() { - @Override - public void onClick(View v) { - chevron.setSelected(!chevron.isSelected()); - if (chevron.isSelected()) { - adapter.hideDescendants(item); - } else { - adapter.showDescendants(item); - } - } - }); - } else { -// chevron.setVisibility(INVISIBLE); - } replyButton.setOnClickListener(new View.OnClickListener() { @Override public void onClick(View v) { listener.onReplyClick(item); - adapter.scrollTo(item); } }); } diff --git a/briar-android/src/main/res/layout/list_item_forum.xml b/briar-android/src/main/res/layout/list_item_forum.xml index 50bd1a18af..a3881e9e74 100644 --- a/briar-android/src/main/res/layout/list_item_forum.xml +++ b/briar-android/src/main/res/layout/list_item_forum.xml @@ -60,7 +60,7 @@ tools:text="Dec 24"/> <View - style="@style/Divider.ForumList" + style="@style/Divider.ThreadItem" android:layout_alignParentLeft="true" android:layout_alignParentStart="true" android:layout_below="@+id/postCountView"/> diff --git a/briar-android/src/main/res/layout/list_item_group.xml b/briar-android/src/main/res/layout/list_item_group.xml index d0b1122648..19ea41eb87 100644 --- a/briar-android/src/main/res/layout/list_item_group.xml +++ b/briar-android/src/main/res/layout/list_item_group.xml @@ -97,7 +97,7 @@ <View android:id="@+id/divider" - style="@style/Divider.ForumList" + style="@style/Divider.ThreadItem" android:layout_alignParentLeft="true" android:layout_alignParentStart="true" android:layout_below="@+id/statusView" diff --git a/briar-android/src/main/res/layout/list_item_group_join_notice.xml b/briar-android/src/main/res/layout/list_item_group_join_notice.xml index 23b7fd283c..02e0b4b4b2 100644 --- a/briar-android/src/main/res/layout/list_item_group_join_notice.xml +++ b/briar-android/src/main/res/layout/list_item_group_join_notice.xml @@ -6,23 +6,14 @@ xmlns:tools="http://schemas.android.com/tools" android:layout_width="match_parent" android:layout_height="wrap_content" - android:layout_marginLeft="@dimen/margin_medium" android:baselineAligned="false" android:orientation="vertical"> - <View - android:id="@+id/top_divider" - style="@style/Divider.ForumList" - android:layout_alignParentTop="true"/> - <org.thoughtcrime.securesms.components.emoji.EmojiTextView android:id="@+id/text" android:layout_width="match_parent" android:layout_height="wrap_content" - android:layout_below="@+id/top_divider" - android:layout_marginBottom="@dimen/margin_small" - android:layout_marginRight="@dimen/margin_medium" - android:layout_marginTop="@dimen/margin_medium" + android:layout_margin="@dimen/margin_medium" android:textColor="@color/briar_text_secondary" android:textSize="@dimen/text_size_medium" android:textStyle="italic" @@ -32,9 +23,12 @@ android:id="@+id/icon" android:layout_width="wrap_content" android:layout_height="wrap_content" + android:layout_alignBottom="@+id/info" android:layout_alignLeft="@+id/text" + android:layout_alignTop="@+id/info" android:layout_below="@+id/text" - android:layout_marginRight="@dimen/margin_small" + android:layout_marginRight="@dimen/margin_medium" + android:scaleType="center" tools:ignore="ContentDescription" tools:src="@drawable/ic_visibility"/> @@ -45,6 +39,7 @@ android:layout_alignEnd="@+id/text" android:layout_alignRight="@+id/text" android:layout_below="@+id/text" + android:layout_marginBottom="@dimen/margin_medium" android:layout_toRightOf="@+id/icon" android:gravity="center_vertical" android:minHeight="24dp" @@ -57,10 +52,10 @@ <org.briarproject.briar.android.view.AuthorView android:id="@+id/author" android:layout_width="wrap_content" - android:layout_height="@dimen/button_size" + android:layout_height="wrap_content" android:layout_alignLeft="@+id/text" android:layout_below="@+id/info" - android:gravity="center" + android:layout_toLeftOf="@+id/optionsButton" app:persona="commenter"/> <Button @@ -68,11 +63,16 @@ style="@style/BriarButtonFlat.Positive.Tiny" android:layout_width="wrap_content" android:layout_height="wrap_content" + android:layout_alignBottom="@+id/author" android:layout_alignEnd="@+id/text" android:layout_alignRight="@+id/text" - android:layout_alignTop="@+id/author" - android:layout_toRightOf="@+id/author" + android:layout_below="@+id/info" android:gravity="right|center_vertical" android:text="@string/options"/> + <View + style="@style/Divider.ThreadItem" + android:layout_below="@+id/author" + android:layout_marginTop="@dimen/margin_medium"/> + </RelativeLayout> diff --git a/briar-android/src/main/res/layout/list_item_invitations.xml b/briar-android/src/main/res/layout/list_item_invitations.xml index ce7afa1477..5aab1e5919 100644 --- a/briar-android/src/main/res/layout/list_item_invitations.xml +++ b/briar-android/src/main/res/layout/list_item_invitations.xml @@ -81,7 +81,7 @@ android:text="@string/decline"/> <View - style="@style/Divider.ForumList" + style="@style/Divider.ThreadItem" android:layout_alignParentLeft="true" android:layout_alignParentStart="true" android:layout_below="@+id/acceptButton"/> diff --git a/briar-android/src/main/res/layout/list_item_rss_feed.xml b/briar-android/src/main/res/layout/list_item_rss_feed.xml index 51061a3314..76e0978a77 100644 --- a/briar-android/src/main/res/layout/list_item_rss_feed.xml +++ b/briar-android/src/main/res/layout/list_item_rss_feed.xml @@ -113,7 +113,7 @@ tools:text="This is a description of the RSS feed. It can be several lines long, but it can also not exist at all if it is not present in the feed itself."/> <View - style="@style/Divider.ForumList" + style="@style/Divider.ThreadItem" android:layout_alignParentLeft="true" android:layout_alignParentStart="true" android:layout_below="@+id/descriptionView" diff --git a/briar-android/src/main/res/layout/list_item_thread.xml b/briar-android/src/main/res/layout/list_item_thread.xml index b6ec4a14b4..c161088397 100644 --- a/briar-android/src/main/res/layout/list_item_thread.xml +++ b/briar-android/src/main/res/layout/list_item_thread.xml @@ -62,23 +62,20 @@ android:background="@drawable/level_indicator_circle" android:gravity="center" android:textSize="@dimen/text_size_small" - android:visibility="gone" - /> + android:visibility="gone"/> </RelativeLayout> <RelativeLayout android:layout_width="0dp" android:layout_height="wrap_content" - android:layout_marginLeft="@dimen/margin_medium" android:layout_weight="1"> <org.thoughtcrime.securesms.components.emoji.EmojiTextView android:id="@+id/text" android:layout_width="match_parent" android:layout_height="wrap_content" - android:paddingRight="@dimen/margin_medium" - android:paddingTop="@dimen/margin_medium" + android:padding="@dimen/margin_medium" android:textColor="@color/briar_text_primary" android:textIsSelectable="true" android:textSize="@dimen/text_size_medium" @@ -87,26 +84,12 @@ <org.briarproject.briar.android.view.AuthorView android:id="@+id/author" android:layout_width="wrap_content" - android:layout_height="@dimen/button_size" + android:layout_height="wrap_content" android:layout_alignLeft="@id/text" android:layout_below="@id/text" - android:gravity="center" - app:persona="commenter"/> - - <TextView - android:id="@+id/replies" - android:layout_width="wrap_content" - android:layout_height="wrap_content" - android:layout_alignBottom="@+id/author" - android:layout_alignTop="@+id/author" + android:layout_marginLeft="@dimen/margin_medium" android:layout_toLeftOf="@+id/btn_reply" - android:layout_toRightOf="@+id/author" - android:ellipsize="end" - android:gravity="right|end|center_vertical" - android:maxLines="1" - android:padding="@dimen/margin_medium" - android:textSize="@dimen/text_size_tiny" - tools:text="2 replies"/> + app:persona="commenter"/> <TextView android:id="@+id/btn_reply" @@ -115,32 +98,17 @@ android:layout_height="wrap_content" android:layout_alignBottom="@+id/author" android:layout_alignParentRight="true" - android:layout_toLeftOf="@+id/chevron" + android:layout_below="@+id/text" + android:layout_marginRight="@dimen/margin_medium" android:text="@string/btn_reply" android:textSize="@dimen/text_size_tiny"/> - <ImageView - android:id="@+id/chevron" - android:layout_width="@dimen/button_size" - android:layout_height="@dimen/button_size" - android:layout_alignBottom="@+id/author" - android:layout_alignParentRight="true" - android:background="?attr/selectableItemBackground" - android:clickable="true" - android:padding="@dimen/margin_medium" - android:scaleType="center" - android:src="@drawable/selector_chevron" - android:visibility="gone"/> - <View - android:id="@+id/top_divider" - style="@style/Divider.ForumList" - android:layout_width="match_parent" - android:layout_height="@dimen/margin_separator" + style="@style/Divider.ThreadItem" android:layout_alignLeft="@id/text" - android:layout_alignParentTop="true"/> + android:layout_below="@+id/author" + android:layout_marginTop="@dimen/margin_medium"/> </RelativeLayout> - -</LinearLayout> \ No newline at end of file +</LinearLayout> diff --git a/briar-android/src/main/res/values/styles.xml b/briar-android/src/main/res/values/styles.xml index 99105713df..f599ac1899 100644 --- a/briar-android/src/main/res/values/styles.xml +++ b/briar-android/src/main/res/values/styles.xml @@ -79,7 +79,7 @@ <item name="android:layout_marginLeft">@dimen/margin_large</item> </style> - <style name="Divider.ForumList" parent="Divider"> + <style name="Divider.ThreadItem" parent="Divider"> <item name="android:layout_width">match_parent</item> <item name="android:layout_height">1dp</item> </style> @@ -102,7 +102,7 @@ <style name="DiscussionLevelIndicator"> <item name="android:layout_marginLeft">4dp</item> - <item name="android:background">?android:attr/listDivider</item> + <item name="android:background">@color/divider</item> </style> <style name="BriarTabLayout" parent="Widget.Design.TabLayout"> diff --git a/briar-android/src/test/java/org/briarproject/briar/android/forum/ForumActivityTest.java b/briar-android/src/test/java/org/briarproject/briar/android/forum/ForumActivityTest.java index 5773f0d2aa..7986764099 100644 --- a/briar-android/src/test/java/org/briarproject/briar/android/forum/ForumActivityTest.java +++ b/briar-android/src/test/java/org/briarproject/briar/android/forum/ForumActivityTest.java @@ -114,28 +114,15 @@ public class ForumActivityTest { rc.getValue().onResult(dummyData); ThreadItemAdapter<ForumItem> adapter = forumActivity.getAdapter(); Assert.assertNotNull(adapter); - // Cascade close assertEquals(6, adapter.getItemCount()); - adapter.hideDescendants(dummyData.get(2)); - assertEquals(5, adapter.getItemCount()); - adapter.hideDescendants(dummyData.get(1)); - assertEquals(4, adapter.getItemCount()); - adapter.hideDescendants(dummyData.get(0)); - assertEquals(2, adapter.getItemCount()); assertTrue(dummyData.get(0).getText() - .equals(adapter.getVisibleItem(0).getText())); + .equals(adapter.getItemAt(0).getText())); assertTrue(dummyData.get(5).getText() - .equals(adapter.getVisibleItem(1).getText())); - // Cascade re-open - adapter.showDescendants(dummyData.get(0)); - assertEquals(4, adapter.getItemCount()); - adapter.showDescendants(dummyData.get(1)); - assertEquals(5, adapter.getItemCount()); - adapter.showDescendants(dummyData.get(2)); - assertEquals(6, adapter.getItemCount()); + .equals(adapter.getItemAt(1).getText())); assertTrue(dummyData.get(2).getText() - .equals(adapter.getVisibleItem(2).getText())); + .equals(adapter.getItemAt(2).getText())); assertTrue(dummyData.get(4).getText() - .equals(adapter.getVisibleItem(4).getText())); + .equals(adapter.getItemAt(4).getText())); } + } diff --git a/briar-api/src/main/java/org/briarproject/briar/api/client/MessageTree.java b/briar-api/src/main/java/org/briarproject/briar/api/client/MessageTree.java index cca70884e3..8d2ed6cd9a 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/client/MessageTree.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/client/MessageTree.java @@ -31,8 +31,6 @@ public interface MessageTree<T extends MessageTree.MessageNode> { void setLevel(int level); - void setDescendantCount(int descendantCount); - long getTimestamp(); } diff --git a/briar-core/src/main/java/org/briarproject/briar/client/MessageTreeImpl.java b/briar-core/src/main/java/org/briarproject/briar/client/MessageTreeImpl.java index 982bcd2e76..5636c5f751 100644 --- a/briar-core/src/main/java/org/briarproject/briar/client/MessageTreeImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/client/MessageTreeImpl.java @@ -83,7 +83,6 @@ public class MessageTreeImpl<T extends MessageTree.MessageNode> list.add(node); List<T> children = nodeMap.get(node.getId()); node.setLevel(level); - node.setDescendantCount(children.size()); for (T child : children) { traverse(list, child, level + 1); } -- GitLab