From 2332a58681422745cb370c171ec1988519d7a13f Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 12 Dec 2018 16:55:09 -0200 Subject: [PATCH] [android] address review comments for displaying multiple images --- .../ConversationMessageViewHolder.java | 3 +- .../android/conversation/ImageAdapter.java | 11 ++++---- .../conversation/ImageItemDecoration.java | 28 ++++++------------- .../android/conversation/ImageViewHolder.java | 9 ++---- .../android/conversation/ImageViewModel.java | 4 +-- .../briar/android/util/UiUtils.java | 7 +++++ 6 files changed, 26 insertions(+), 36 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationMessageViewHolder.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationMessageViewHolder.java index 7893071aa..94c80639a 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationMessageViewHolder.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationMessageViewHolder.java @@ -33,8 +33,7 @@ class ConversationMessageViewHolder extends ConversationItemViewHolder { // image list RecyclerView list = v.findViewById(R.id.imageList); list.setRecycledViewPool(imageViewPool); - adapter = - new ImageAdapter(v.getContext(), imageItemDecoration, listener); + adapter = new ImageAdapter(v.getContext(), listener); list.setAdapter(adapter); list.addItemDecoration(imageItemDecoration); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageAdapter.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageAdapter.java index 134f19975..8c9e15d30 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageAdapter.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageAdapter.java @@ -19,36 +19,35 @@ import java.util.List; import static android.content.Context.WINDOW_SERVICE; import static java.util.Objects.requireNonNull; +import static org.briarproject.briar.android.util.UiUtils.isRtl; @NotNullByDefault class ImageAdapter extends Adapter { private final List items = new ArrayList<>(); private final ConversationListener listener; - private final int imageSize, borderSize; + private final int imageSize; private final int radiusBig, radiusSmall; private final boolean isRtl; @Nullable private ConversationMessageItem conversationItem; - public ImageAdapter(Context ctx, ImageItemDecoration imageItemDecoration, - ConversationListener listener) { + ImageAdapter(Context ctx, ConversationListener listener) { this.listener = listener; - borderSize = imageItemDecoration.getBorderSize(); imageSize = getImageSize(ctx); Resources res = ctx.getResources(); radiusBig = res.getDimensionPixelSize(R.dimen.message_bubble_radius_big); radiusSmall = res.getDimensionPixelSize(R.dimen.message_bubble_radius_small); - isRtl = imageItemDecoration.isRtl(); + isRtl = isRtl(ctx); } @Override public ImageViewHolder onCreateViewHolder(ViewGroup viewGroup, int type) { View v = LayoutInflater.from(viewGroup.getContext()).inflate( R.layout.list_item_image, viewGroup, false); - return new ImageViewHolder(v, imageSize, borderSize); + return new ImageViewHolder(v, imageSize); } @Override diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageItemDecoration.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageItemDecoration.java index c01f962c8..92df42e40 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageItemDecoration.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageItemDecoration.java @@ -1,7 +1,6 @@ package org.briarproject.briar.android.conversation; import android.content.Context; -import android.content.res.Configuration; import android.content.res.Resources; import android.graphics.Rect; import android.support.v7.widget.RecyclerView; @@ -11,9 +10,8 @@ import android.view.View; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.briar.R; +import org.briarproject.briar.android.util.UiUtils; -import static android.os.Build.VERSION.SDK_INT; -import static android.support.v4.view.ViewCompat.LAYOUT_DIRECTION_RTL; import static org.briarproject.briar.android.conversation.ImageAdapter.isBottomRow; import static org.briarproject.briar.android.conversation.ImageAdapter.isLeft; import static org.briarproject.briar.android.conversation.ImageAdapter.isTopRow; @@ -22,24 +20,22 @@ import static org.briarproject.briar.android.conversation.ImageAdapter.singleInR @NotNullByDefault class ImageItemDecoration extends ItemDecoration { - private final int realBorderSize, border; + private final int border; private final boolean isRtl; - public ImageItemDecoration(Context ctx) { + ImageItemDecoration(Context ctx) { Resources res = ctx.getResources(); // for pixel perfection, add a pixel to the border if it has an odd size int b = res.getDimensionPixelSize(R.dimen.message_bubble_border); - realBorderSize = b % 2 == 0 ? b : b + 1;; + int realBorderSize = b % 2 == 0 ? b : b + 1; // we are applying half the border around the insides of each image // to prevent differently sized images looking slightly broken border = realBorderSize / 2; // find out if we are showing a RTL language - Configuration config = res.getConfiguration(); - isRtl = SDK_INT >= 17 && - config.getLayoutDirection() == LAYOUT_DIRECTION_RTL; + isRtl = UiUtils.isRtl(ctx); } @Override @@ -48,19 +44,11 @@ class ImageItemDecoration extends ItemDecoration { if (state.getItemCount() == 1) return; int pos = parent.getChildAdapterPosition(view); int num = state.getItemCount(); - boolean left = isLeft(pos) ^ isRtl; + boolean start = isLeft(pos) ^ isRtl; outRect.top = isTopRow(pos) ? 0 : border; - outRect.left = left ? 0 : border; - outRect.right = left && !singleInRow(pos, num) ? border : 0; + outRect.left = start ? 0 : border; + outRect.right = start && !singleInRow(pos, num) ? border : 0; outRect.bottom = isBottomRow(pos, num) ? 0 : border; } - public int getBorderSize() { - return realBorderSize; - } - - public boolean isRtl() { - return isRtl; - } - } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewHolder.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewHolder.java index 271fa52ea..301fa3c13 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewHolder.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewHolder.java @@ -25,13 +25,12 @@ class ImageViewHolder extends ViewHolder { private static final int ERROR_RES = R.drawable.ic_image_broken; protected final ImageView imageView; - private final int imageSize, borderSize; + private final int imageSize; - public ImageViewHolder(View v, int imageSize, int borderSize) { + ImageViewHolder(View v, int imageSize) { super(v); imageView = v.findViewById(R.id.imageView); this.imageSize = imageSize; - this.borderSize = borderSize; } void bind(AttachmentItem attachment, Radii r, boolean single, @@ -49,9 +48,7 @@ class ImageViewHolder extends ViewHolder { private void setImageViewDimensions(AttachmentItem a, boolean single, boolean needsStretch) { LayoutParams params = (LayoutParams) imageView.getLayoutParams(); - // actual image size will shrink half the border - int stretchSize = (imageSize - borderSize / 2) * 2 + borderSize; - int width = needsStretch ? stretchSize : imageSize; + int width = needsStretch ? imageSize * 2 : imageSize; params.width = single ? a.getThumbnailWidth() : width; params.height = single ? a.getThumbnailHeight() : imageSize; params.setFullSpan(!single && needsStretch); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java index 694887c91..d3cc62019 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewModel.java @@ -48,10 +48,10 @@ public class ImageViewModel extends AndroidViewModel { @IoExecutor private final Executor ioExecutor; - private MutableLiveData saveState = new MutableLiveData<>(); + private final MutableLiveData saveState = new MutableLiveData<>(); @Inject - public ImageViewModel(Application application, + ImageViewModel(Application application, MessagingManager messagingManager, @DatabaseExecutor Executor dbExecutor, @IoExecutor Executor ioExecutor) { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java b/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java index fcd7fcb14..c3aa1038c 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java @@ -56,6 +56,7 @@ import static android.content.Intent.FLAG_ACTIVITY_NEW_TASK; import static android.os.Build.MANUFACTURER; import static android.os.Build.VERSION.SDK_INT; import static android.provider.Settings.ACTION_REQUEST_IGNORE_BATTERY_OPTIMIZATIONS; +import static android.support.v4.view.ViewCompat.LAYOUT_DIRECTION_RTL; import static android.support.v7.app.AppCompatDelegate.MODE_NIGHT_AUTO; import static android.support.v7.app.AppCompatDelegate.MODE_NIGHT_FOLLOW_SYSTEM; import static android.support.v7.app.AppCompatDelegate.MODE_NIGHT_NO; @@ -354,4 +355,10 @@ public class UiUtils { }); } + public static boolean isRtl(Context ctx) { + if (SDK_INT < 17) return false; + return ctx.getResources().getConfiguration().getLayoutDirection() == + LAYOUT_DIRECTION_RTL; + } + }