From 423ecc003bd4994203449da70abb7a2b7e506e50 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Tue, 5 Feb 2019 16:53:12 -0200 Subject: [PATCH 1/3] [android] Get notified when all image previews have been loaded Also fix crash when attaching image fails --- .../briar/android/view/ImagePreview.java | 7 ++++++ .../android/view/ImagePreviewAdapter.java | 1 + .../android/view/ImagePreviewViewHolder.java | 3 ++- .../view/TextAttachmentController.java | 24 ++++++++++++++++++- 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java index 8921b668c..f4158a495 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreview.java @@ -79,6 +79,13 @@ public class ImagePreview extends ConstraintLayout { interface ImagePreviewListener { + void onPreviewLoaded(); + + /** + * Called when Glide can't load a preview image. + * + * Warning: Glide may call this multiple times. + */ void onUriError(Uri uri); void onCancel(); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java index 20bf9d4c3..a6508e4eb 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewAdapter.java @@ -54,6 +54,7 @@ class ImagePreviewAdapter extends Adapter { void removeUri(Uri uri) { int pos = items.indexOf(uri); + if (pos == -1) return; items.remove(uri); notifyItemRemoved(pos); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java index d63569024..c16ce14ca 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/ImagePreviewViewHolder.java @@ -54,8 +54,8 @@ class ImagePreviewViewHolder extends ViewHolder { public boolean onLoadFailed(@Nullable GlideException e, Object model, Target target, boolean isFirstResource) { - listener.onUriError(uri); progressBar.setVisibility(INVISIBLE); + listener.onUriError(uri); return false; } @@ -64,6 +64,7 @@ class ImagePreviewViewHolder extends ViewHolder { Object model, Target target, DataSource dataSource, boolean isFirstResource) { progressBar.setVisibility(INVISIBLE); + listener.onPreviewLoaded(); return false; } }) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java b/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java index 6248b6669..c3afded7a 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java @@ -54,6 +54,7 @@ public class TextAttachmentController extends TextSendController private CharSequence textHint; private boolean hasImageSupport = false; private List imageUris = emptyList(); + private int previewsLoaded = 0; public TextAttachmentController(TextInputView v, ImagePreview imagePreview, SendListener listener, AttachImageListener imageListener) { @@ -185,6 +186,8 @@ public class TextAttachmentController extends TextSendController imageUris = emptyList(); // show the image button again, so images can get attached showImageButton(true); + // no preview has been loaded + previewsLoaded = 0; } @Override @@ -204,21 +207,40 @@ public class TextAttachmentController extends TextSendController return state.getSuperState(); } + @Override + public void onPreviewLoaded() { + previewsLoaded++; + checkAllPreviewsLoaded(); + } + @Override public void onUriError(Uri uri) { - imageUris.remove(uri); + boolean removed = imageUris.remove(uri); + if (!removed) { + // we have removed this Uri already, do not remove it again + return; + } imagePreview.removeUri(uri); if (imageUris.isEmpty()) onCancel(); Toast.makeText(textInput.getContext(), R.string.image_attach_error, LENGTH_LONG).show(); + checkAllPreviewsLoaded(); } @Override public void onCancel() { textInput.clearText(); + sendButton.setEnabled(true); reset(); } + private void checkAllPreviewsLoaded() { + if (previewsLoaded == imageUris.size()) { + // all previews were loaded + // TODO allow sending + } + } + public void showImageOnboarding(Activity activity, Runnable onOnboardingSeen) { PromptStateChangeListener listener = (prompt, state) -> { From 5130c835560a2597f2e73ef0ec35a46eb684d72f Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 6 Feb 2019 14:29:20 -0200 Subject: [PATCH 2/3] [android] Show progress bar while image previews are loading This refactors the send buttons out into their own composite view --- .../briar/android/blog/ReblogFragment.java | 4 +- .../android/blog/WriteBlogPostActivity.java | 1 + .../conversation/ConversationActivity.java | 4 +- .../IntroductionMessageFragment.java | 6 +- .../android/threaded/ThreadListActivity.java | 2 + .../android/view/CompositeSendButton.java | 114 ++++++++++++++++++ .../android/view/LargeTextInputView.java | 2 +- .../view/TextAttachmentController.java | 90 +++++--------- .../briar/android/view/TextInputView.java | 8 +- .../android/view/TextSendController.java | 41 ++++--- .../src/main/res/layout/text_input_view.xml | 36 +----- .../main/res/layout/text_input_view_large.xml | 2 +- .../res/layout/view_composite_send_button.xml | 47 ++++++++ 13 files changed, 231 insertions(+), 126 deletions(-) create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/view/CompositeSendButton.java create mode 100644 briar-android/src/main/res/layout/view_composite_send_button.xml diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/ReblogFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/ReblogFragment.java index 685b41877..17e596cb6 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/ReblogFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/ReblogFragment.java @@ -86,7 +86,7 @@ public class ReblogFragment extends BaseFragment implements SendListener { TextSendController sendController = new TextSendController(ui.input, this, true); ui.input.setSendController(sendController); - ui.input.setEnabled(false); + ui.input.setReady(false); ui.input.setMaxTextLength(MAX_BLOG_POST_TEXT_LENGTH); showProgressBar(); @@ -116,7 +116,7 @@ public class ReblogFragment extends BaseFragment implements SendListener { ui.post.bindItem(item); ui.post.hideReblogButton(); - ui.input.setEnabled(true); + ui.input.setReady(true); ui.scrollView.post(() -> ui.scrollView.fullScroll(FOCUS_DOWN)); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/WriteBlogPostActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/WriteBlogPostActivity.java index 0acda8309..5b68499cd 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/WriteBlogPostActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/WriteBlogPostActivity.java @@ -80,6 +80,7 @@ public class WriteBlogPostActivity extends BriarActivity new TextSendController(input, this, false); input.setSendController(sendController); input.setMaxTextLength(MAX_BLOG_POST_TEXT_LENGTH); + input.setReady(true); progressBar = findViewById(R.id.progressBar); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java index 2766501a8..b223e2ca9 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java @@ -276,7 +276,7 @@ public class ConversationActivity extends BriarActivity } textInputView.setSendController(sendController); textInputView.setMaxTextLength(MAX_PRIVATE_MESSAGE_TEXT_LENGTH); - textInputView.setEnabled(false); + textInputView.setReady(false); textInputView.addOnKeyboardShownListener(this::scrollToBottom); } @@ -477,7 +477,7 @@ public class ConversationActivity extends BriarActivity runOnUiThreadUnlessDestroyed(() -> { if (revision == adapter.getRevision()) { adapter.incrementRevision(); - textInputView.setEnabled(true); + textInputView.setReady(true); // start observing onboarding after enabling (only once, because // we only update this when an onboarding should be shown) observeOnce(viewModel.showImageOnboarding(), this, diff --git a/briar-android/src/main/java/org/briarproject/briar/android/introduction/IntroductionMessageFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/introduction/IntroductionMessageFragment.java index 6312a9ef1..dee769cf3 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/introduction/IntroductionMessageFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/introduction/IntroductionMessageFragment.java @@ -116,7 +116,7 @@ public class IntroductionMessageFragment extends BaseFragment new TextSendController(ui.message, this, true); ui.message.setSendController(sendController); ui.message.setMaxTextLength(MAX_INTRODUCTION_TEXT_LENGTH); - ui.message.setEnabled(false); + ui.message.setReady(false); // get contacts and then show view prepareToSetUpViews(contactId1, contactId2); @@ -171,7 +171,7 @@ public class IntroductionMessageFragment extends BaseFragment // show views ui.notPossible.setVisibility(GONE); ui.message.setVisibility(VISIBLE); - ui.message.setEnabled(true); + ui.message.setReady(true); ui.message.showSoftKeyboard(); } else { ui.notPossible.setVisibility(VISIBLE); @@ -195,7 +195,7 @@ public class IntroductionMessageFragment extends BaseFragment @Override public void onSendClick(@Nullable String text, List imageUris) { // disable button to prevent accidental double invitations - ui.message.setEnabled(false); + ui.message.setReady(false); makeIntroduction(contact1, contact2, text); 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 b766ff1cd..f0412a26a 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 @@ -91,6 +91,8 @@ public abstract class ThreadListActivity { + sendButton.setVisibility(INVISIBLE); + imageButton.setEnabled(true); + }).start(); + imageButton.clearAnimation(); + imageButton.animate().alpha(1f).start(); + } + } else { + sendButton.setVisibility(VISIBLE); + // enable/disable buttons right away to allow fast sending + sendButton.setEnabled(sendEnabled); + imageButton.setEnabled(false); + if (SDK_INT <= 15) { + imageButton.setVisibility(INVISIBLE); + } else { + sendButton.clearAnimation(); + sendButton.animate().alpha(1f).start(); + imageButton.clearAnimation(); + imageButton.animate().alpha(0f).withEndAction(() -> + imageButton.setVisibility(INVISIBLE) + ).start(); + } + } + } + + public void showProgress(boolean show) { + sendButton.setVisibility(show ? INVISIBLE : VISIBLE); + imageButton.setVisibility(show ? INVISIBLE : VISIBLE); + progressBar.setVisibility(show ? VISIBLE : INVISIBLE); + } + +} diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/LargeTextInputView.java b/briar-android/src/main/java/org/briarproject/briar/android/view/LargeTextInputView.java index 5ed4ce0f1..df55d6807 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/LargeTextInputView.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/LargeTextInputView.java @@ -65,7 +65,7 @@ public class LargeTextInputView extends TextInputView { } public void setButtonText(String text) { - ((Button) findViewById(R.id.btn_send)).setText(text); + ((Button) findViewById(R.id.compositeSendButton)).setText(text); } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java b/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java index c3afded7a..3a73ba695 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/TextAttachmentController.java @@ -11,7 +11,6 @@ import android.support.annotation.Nullable; import android.support.annotation.UiThread; import android.support.v4.view.AbsSavedState; import android.support.v7.app.AlertDialog.Builder; -import android.support.v7.widget.AppCompatImageButton; import android.widget.Toast; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; @@ -32,8 +31,6 @@ import static android.os.Build.VERSION.SDK_INT; import static android.support.v4.content.ContextCompat.getColor; import static android.support.v4.view.AbsSavedState.EMPTY_STATE; import static android.view.View.GONE; -import static android.view.View.INVISIBLE; -import static android.view.View.VISIBLE; import static android.widget.Toast.LENGTH_LONG; import static java.util.Collections.emptyList; import static java.util.Objects.requireNonNull; @@ -46,15 +43,14 @@ import static uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt.S public class TextAttachmentController extends TextSendController implements ImagePreviewListener { - private final AppCompatImageButton imageButton; private final ImagePreview imagePreview; - private final AttachImageListener imageListener; + private final CompositeSendButton sendButton; private CharSequence textHint; - private boolean hasImageSupport = false; private List imageUris = emptyList(); private int previewsLoaded = 0; + private boolean loadingPreviews = false; public TextAttachmentController(TextInputView v, ImagePreview imagePreview, SendListener listener, AttachImageListener imageListener) { @@ -63,18 +59,26 @@ public class TextAttachmentController extends TextSendController this.imagePreview = imagePreview; this.imagePreview.setImagePreviewListener(this); - imageButton = v.findViewById(R.id.imageButton); - imageButton.setOnClickListener(view -> onImageButtonClicked()); + sendButton = (CompositeSendButton) compositeSendButton; + sendButton.setOnImageClickListener(view -> onImageButtonClicked()); textHint = textInput.getHint(); - - // show image button - showImageButton(true); } @Override - public void onTextIsEmptyChanged(boolean isEmpty) { - if (imageUris.isEmpty()) showImageButton(isEmpty); + protected void updateViewState() { + textInput.setEnabled(ready && !loadingPreviews); + boolean sendEnabled = ready && !loadingPreviews && + (!textIsEmpty || canSendEmptyText()); + if (loadingPreviews) { + sendButton.showProgress(true); + } else if (imageUris.isEmpty()) { + sendButton.showProgress(false); + sendButton.showImageButton(textIsEmpty, sendEnabled); + } else { + sendButton.showProgress(false); + sendButton.showImageButton(false, sendEnabled); + } } @Override @@ -90,19 +94,13 @@ public class TextAttachmentController extends TextSendController return !imageUris.isEmpty(); } - /*** - * By default, image support is disabled. - * Once you know that it is supported in the current context, - * call this method to enable it. - */ public void setImagesSupported() { - hasImageSupport = true; - imageButton.setImageResource(R.drawable.ic_image); + sendButton.setImagesSupported(); } private void onImageButtonClicked() { - if (!hasImageSupport) { - Context ctx = imageButton.getContext(); + if (!sendButton.hasImageSupport()) { + Context ctx = imagePreview.getContext(); Builder builder = new Builder(ctx, R.style.OnboardingDialogTheme); builder.setTitle( ctx.getString(R.string.dialog_title_no_image_support)); @@ -138,45 +136,12 @@ public class TextAttachmentController extends TextSendController private void onNewUris() { if (imageUris.isEmpty()) return; - showImageButton(false); + loadingPreviews = true; + updateViewState(); textInput.setHint(R.string.image_caption_hint); imagePreview.showPreview(imageUris); } - private void showImageButton(boolean showImageButton) { - if (showImageButton) { - imageButton.setVisibility(VISIBLE); - sendButton.setEnabled(false); - if (SDK_INT <= 15) { - sendButton.setVisibility(INVISIBLE); - imageButton.setEnabled(true); - } else { - sendButton.clearAnimation(); - sendButton.animate().alpha(0f).withEndAction(() -> { - sendButton.setVisibility(INVISIBLE); - imageButton.setEnabled(true); - }).start(); - imageButton.clearAnimation(); - imageButton.animate().alpha(1f).start(); - } - } else { - sendButton.setVisibility(VISIBLE); - // enable/disable buttons right away to allow fast sending - sendButton.setEnabled(enabled); - imageButton.setEnabled(false); - if (SDK_INT <= 15) { - imageButton.setVisibility(INVISIBLE); - } else { - sendButton.clearAnimation(); - sendButton.animate().alpha(1f).start(); - imageButton.clearAnimation(); - imageButton.animate().alpha(0f).withEndAction(() -> - imageButton.setVisibility(INVISIBLE) - ).start(); - } - } - } - private void reset() { // restore hint textInput.setHint(textHint); @@ -184,10 +149,11 @@ public class TextAttachmentController extends TextSendController imagePreview.setVisibility(GONE); // reset image URIs imageUris = emptyList(); - // show the image button again, so images can get attached - showImageButton(true); // no preview has been loaded previewsLoaded = 0; + loadingPreviews = false; + // show the image button again, so images can get attached + updateViewState(); } @Override @@ -230,14 +196,14 @@ public class TextAttachmentController extends TextSendController @Override public void onCancel() { textInput.clearText(); - sendButton.setEnabled(true); reset(); } private void checkAllPreviewsLoaded() { if (previewsLoaded == imageUris.size()) { + loadingPreviews = false; // all previews were loaded - // TODO allow sending + updateViewState(); } } @@ -250,7 +216,7 @@ public class TextAttachmentController extends TextSendController }; int color = resolveColorAttribute(activity, R.attr.colorControlNormal); new MaterialTapTargetPrompt.Builder(activity, - R.style.OnboardingDialogTheme).setTarget(imageButton) + R.style.OnboardingDialogTheme).setTarget(sendButton) .setPrimaryText(R.string.dialog_title_image_support) .setSecondaryText(R.string.dialog_message_image_support) .setBackgroundColour(getColor(activity, R.color.briar_primary)) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/TextInputView.java b/briar-android/src/main/java/org/briarproject/briar/android/view/TextInputView.java index 1b414c3d6..89a32c1e4 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/TextInputView.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/TextInputView.java @@ -99,9 +99,11 @@ public class TextInputView extends LinearLayout { @Override public void setEnabled(boolean enabled) { - super.setEnabled(enabled); - textInput.setEnabled(enabled); - requireNonNull(textSendController).setEnabled(enabled); + throw new RuntimeException("Use controllers to enable/disable"); + } + + public void setReady(boolean ready) { + requireNonNull(textSendController).setReady(ready); } @Override diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/TextSendController.java b/briar-android/src/main/java/org/briarproject/briar/android/view/TextSendController.java index ad35f728f..68edf4a12 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/TextSendController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/TextSendController.java @@ -2,7 +2,6 @@ package org.briarproject.briar.android.view; import android.net.Uri; import android.os.Parcelable; -import android.support.annotation.CallSuper; import android.support.annotation.Nullable; import android.support.annotation.UiThread; import android.support.design.widget.Snackbar; @@ -22,18 +21,17 @@ import static java.util.Collections.emptyList; public class TextSendController implements TextInputListener { protected final EmojiTextInputView textInput; - protected final View sendButton; + protected final View compositeSendButton; protected final SendListener listener; - protected boolean enabled = true; - protected final boolean allowEmptyText; - private boolean wasEmpty = true; + protected boolean ready = true, textIsEmpty = true; + + private final boolean allowEmptyText; public TextSendController(TextInputView v, SendListener listener, boolean allowEmptyText) { - this.sendButton = v.findViewById(R.id.btn_send); - this.sendButton.setOnClickListener(view -> onSendEvent()); - this.sendButton.setEnabled(allowEmptyText); + this.compositeSendButton = v.findViewById(R.id.compositeSendButton); + this.compositeSendButton.setOnClickListener(view -> onSendEvent()); this.listener = listener; this.textInput = v.getEmojiTextInputView(); this.allowEmptyText = allowEmptyText; @@ -41,8 +39,8 @@ public class TextSendController implements TextInputListener { @Override public void onTextIsEmptyChanged(boolean isEmpty) { - sendButton.setEnabled(enabled && (!isEmpty || canSendEmptyText())); - wasEmpty = isEmpty; + textIsEmpty = isEmpty; + updateViewState(); } @Override @@ -52,13 +50,24 @@ public class TextSendController implements TextInputListener { } } + public void setReady(boolean ready) { + this.ready = ready; + updateViewState(); + } + + protected void updateViewState() { + textInput.setEnabled(ready); + compositeSendButton + .setEnabled(ready && (!textIsEmpty || canSendEmptyText())); + } + protected final boolean canSend() { if (textInput.isTooLong()) { - Snackbar.make(sendButton, R.string.text_too_long, LENGTH_SHORT) - .show(); + Snackbar.make(compositeSendButton, R.string.text_too_long, + LENGTH_SHORT).show(); return false; } - return enabled && (canSendEmptyText() || !textInput.isEmpty()); + return ready && (canSendEmptyText() || !textInput.isEmpty()); } protected boolean canSendEmptyText() { @@ -75,12 +84,6 @@ public class TextSendController implements TextInputListener { return state; } - @CallSuper - public void setEnabled(boolean enabled) { - sendButton.setEnabled(enabled && (!wasEmpty || canSendEmptyText())); - this.enabled = enabled; - } - public interface SendListener { void onSendClick(@Nullable String text, List imageUris); } diff --git a/briar-android/src/main/res/layout/text_input_view.xml b/briar-android/src/main/res/layout/text_input_view.xml index 474ce90c1..26028b376 100644 --- a/briar-android/src/main/res/layout/text_input_view.xml +++ b/briar-android/src/main/res/layout/text_input_view.xml @@ -24,41 +24,11 @@ android:layout_weight="1" app:maxTextLines="4"/> - - - - - - - + android:layout_gravity="bottom"/> diff --git a/briar-android/src/main/res/layout/text_input_view_large.xml b/briar-android/src/main/res/layout/text_input_view_large.xml index e57ce798d..9cf294733 100644 --- a/briar-android/src/main/res/layout/text_input_view_large.xml +++ b/briar-android/src/main/res/layout/text_input_view_large.xml @@ -32,7 +32,7 @@