Merge branch '1475-image-transitions' into 'master'

Resolve main issues with image transition animation

See merge request briar/briar!1016

briar-android/src/main/AndroidManifest.xml

@@ -1,6 +1,7 @@
 <?xml version="1.0" encoding="utf-8"?>
 <manifest
 	package="org.briarproject.briar"
+	xmlns:tools="http://schemas.android.com/tools"
 	xmlns:android="http://schemas.android.com/apk/res/android">
 
 	<uses-feature android:name="android.hardware.bluetooth" android:required="false"/>
@@ -29,7 +30,8 @@
 		android:label="@string/app_name"
 		android:logo="@mipmap/ic_launcher_round"
 		android:supportsRtl="true"
-		android:theme="@style/BriarTheme">
+		android:theme="@style/BriarTheme"
+		tools:ignore="GoogleAppIndexingWarning">
 
 		<receiver
 			android:name="org.briarproject.briar.android.login.SignInReminderReceiver"
@@ -117,7 +119,7 @@
 		<activity
 			android:name=".android.conversation.ImageActivity"
 			android:parentActivityName="org.briarproject.briar.android.conversation.ConversationActivity"
-			android:theme="@style/BriarTheme.Transparent.NoActionBar">
+			android:theme="@style/BriarTheme.ActionBarOverlay">
 			<meta-data
 				android:name="android.support.PARENT_ACTIVITY"
 				android:value="org.briarproject.briar.android.conversation.ConversationActivity"/>

briar-android/src/main/java/org/briarproject/briar/android/conversation/AttachmentController.java

@@ -86,8 +86,7 @@ class AttachmentController {
 		List<Pair<AttachmentHeader, Attachment>> attachments =
 				new ArrayList<>(headers.size());
 		for (AttachmentHeader h : headers) {
-			Attachment a =
-					messagingManager.getAttachment(h.getMessageId());
+			Attachment a = messagingManager.getAttachment(h.getMessageId());
 			attachments.add(new Pair<>(h, a));
 		}
 		logDuration(LOG, "Loading attachment", start);
@@ -96,7 +95,7 @@ class AttachmentController {
 
 	/**
 	 * Creates {@link AttachmentItem}s from the passed headers and Attachments.
-	 *
+	 * <p>
 	 * Note: This closes the {@link Attachment}'s {@link InputStream}.
 	 */
 	List<AttachmentItem> getAttachmentItems(

briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationActivity.java

@@ -8,6 +8,7 @@ import android.content.DialogInterface;
 import android.content.Intent;
 import android.net.Uri;
 import android.os.Bundle;
+import android.os.Parcelable;
 import android.support.annotation.Nullable;
 import android.support.annotation.UiThread;
 import android.support.design.widget.Snackbar;
@@ -102,6 +103,7 @@ import im.delight.android.identicons.IdenticonDrawable;
 import uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt;
 import uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt.PromptStateChangeListener;
 
+import static android.arch.lifecycle.Lifecycle.State.STARTED;
 import static android.os.Build.VERSION.SDK_INT;
 import static android.support.v4.app.ActivityOptionsCompat.makeSceneTransitionAnimation;
 import static android.support.v4.view.ViewCompat.setTransitionName;
@@ -193,6 +195,8 @@ public class ConversationActivity extends BriarActivity
 	ViewModelProvider.Factory viewModelFactory;
 
 	private volatile ContactId contactId;
+	@Nullable
+	private Parcelable layoutManagerState;
 
 	private final Observer<String> contactNameObserver = name -> {
 		requireNonNull(name);
@@ -265,6 +269,12 @@ public class ConversationActivity extends BriarActivity
 		textInputView.setSendController(sendController);
 		textInputView.setMaxTextLength(MAX_PRIVATE_MESSAGE_TEXT_LENGTH);
 		textInputView.setEnabled(false);
+		textInputView.addOnKeyboardShownListener(this::scrollToBottom);
+	}
+
+	private void scrollToBottom() {
+		int items = adapter.getItemCount();
+		if (items > 0) list.scrollToPosition(items - 1);
 	}
 
 	@Override
@@ -317,6 +327,21 @@ public class ConversationActivity extends BriarActivity
 		list.stopPeriodicUpdate();
 	}
 
+	@Override
+	protected void onSaveInstanceState(Bundle outState) {
+		super.onSaveInstanceState(outState);
+		if (layoutManager != null) {
+			layoutManagerState = layoutManager.onSaveInstanceState();
+			outState.putParcelable("layoutManager", layoutManagerState);
+		}
+	}
+
+	@Override
+	protected void onRestoreInstanceState(Bundle savedInstanceState) {
+		super.onRestoreInstanceState(savedInstanceState);
+		layoutManagerState = savedInstanceState.getParcelable("layoutManager");
+	}
+
 	@Override
 	public boolean onCreateOptionsMenu(Menu menu) {
 		// Inflate the menu items for use in the action bar
@@ -389,33 +414,10 @@ public class ConversationActivity extends BriarActivity
 						Long.compare(b.getTimestamp(), a.getTimestamp()));
 				if (!sorted.isEmpty()) {
 					// If the latest header is a private message, eagerly load
-					// its text so we can set the scroll position correctly
+					// its size so we can set the scroll position correctly
 					ConversationMessageHeader latest = sorted.get(0);
 					if (latest instanceof PrivateMessageHeader) {
-						MessageId id = latest.getId();
-						PrivateMessageHeader h = (PrivateMessageHeader) latest;
-						if (h.hasText()) {
-							String text = textCache.get(id);
-							if (text == null) {
-								LOG.info(
-										"Eagerly loading text of latest message");
-								text = messagingManager.getMessageText(id);
-								textCache.put(id, text);
-							}
-						}
-						if (h.getAttachmentHeaders().size() == 1) {
-							List<AttachmentItem> items =
-									attachmentController.get(id);
-							if (items == null) {
-								LOG.info(
-										"Eagerly loading image size for latest message");
-								items = attachmentController.getAttachmentItems(
-										attachmentController
-												.getMessageAttachments(
-														h.getAttachmentHeaders()));
-								attachmentController.put(id, items);
-							}
-						}
+						eagerlyLoadMessageSize((PrivateMessageHeader) latest);
 					}
 				}
 				displayMessages(revision, sorted);
@@ -427,6 +429,32 @@ public class ConversationActivity extends BriarActivity
 		});
 	}
 
+	private void eagerlyLoadMessageSize(PrivateMessageHeader h)
+			throws DbException {
+		MessageId id = h.getId();
+		// If the message has text, load it
+		if (h.hasText()) {
+			String text = textCache.get(id);
+			if (text == null) {
+				LOG.info("Eagerly loading text for latest message");
+				text = messagingManager.getMessageText(id);
+				textCache.put(id, text);
+			}
+		}
+		// If the message has a single image, load its size - for multiple
+		// images we use a grid so the size is fixed
+		if (h.getAttachmentHeaders().size() == 1) {
+			List<AttachmentItem> items = attachmentController.get(id);
+			if (items == null) {
+				LOG.info("Eagerly loading image size for latest message");
+				items = attachmentController.getAttachmentItems(
+						attachmentController.getMessageAttachments(
+								h.getAttachmentHeaders()));
+				attachmentController.put(id, items);
+			}
+		}
+	}
+
 	private void displayMessages(int revision,
 			Collection<ConversationMessageHeader> headers) {
 		runOnUiThreadUnlessDestroyed(() -> {
@@ -436,8 +464,12 @@ public class ConversationActivity extends BriarActivity
 				List<ConversationItem> items = createItems(headers);
 				adapter.addAll(items);
 				list.showData();
-				// Scroll to the bottom
-				list.scrollToPosition(adapter.getItemCount() - 1);
+				if (layoutManagerState == null) {
+					scrollToBottom();
+				} else {
+					// Restore the previous scroll position
+					layoutManager.onRestoreInstanceState(layoutManagerState);
+				}
 			} else {
 				LOG.info("Concurrent update, reloading");
 				loadMessages();
@@ -478,14 +510,21 @@ public class ConversationActivity extends BriarActivity
 			Pair<Integer, ConversationMessageItem> pair =
 					adapter.getMessageItem(m);
 			if (pair != null) {
+				boolean scroll = shouldScrollWhenUpdatingMessage();
 				pair.getSecond().setText(text);
-				boolean bottom = adapter.isScrolledToBottom(layoutManager);
 				adapter.notifyItemChanged(pair.getFirst());
-				if (bottom) list.scrollToPosition(adapter.getItemCount() - 1);
+				if (scroll) scrollToBottom();
 			}
 		});
 	}
 
+	// When a message's text or attachments are loaded, scroll to the bottom
+	// if the conversation is visible and we were previously at the bottom
+	private boolean shouldScrollWhenUpdatingMessage() {
+		return getLifecycle().getCurrentState().isAtLeast(STARTED)
+				&& adapter.isScrolledToBottom(layoutManager);
+	}
+
 	private void loadMessageAttachments(MessageId messageId,
 			List<AttachmentHeader> headers) {
 		runOnDbThread(() -> {
@@ -509,10 +548,10 @@ public class ConversationActivity extends BriarActivity
 			Pair<Integer, ConversationMessageItem> pair =
 					adapter.getMessageItem(m);
 			if (pair != null) {
+				boolean scroll = shouldScrollWhenUpdatingMessage();
 				pair.getSecond().setAttachments(items);
-				boolean bottom = adapter.isScrolledToBottom(layoutManager);
 				adapter.notifyItemChanged(pair.getFirst());
-				if (bottom) list.scrollToPosition(adapter.getItemCount() - 1);
+				if (scroll) scrollToBottom();
 			}
 		});
 	}
@@ -561,10 +600,13 @@ public class ConversationActivity extends BriarActivity
 
 	private void addConversationItem(ConversationItem item) {
 		runOnUiThreadUnlessDestroyed(() -> {
-			boolean bottom = adapter.isScrolledToBottom(layoutManager);
 			adapter.incrementRevision();
 			adapter.add(item);
-			if (bottom) list.scrollToPosition(adapter.getItemCount() - 1);
+			// When adding a new message, scroll to the bottom if the
+			// conversation is visible, even if we're not currently at
+			// the bottom
+			if (getLifecycle().getCurrentState().isAtLeast(STARTED))
+				scrollToBottom();
 		});
 	}
 
@@ -828,15 +870,11 @@ public class ConversationActivity extends BriarActivity
 		i.putExtra(ATTACHMENT_POSITION, attachments.indexOf(item));
 		i.putExtra(NAME, name);
 		i.putExtra(DATE, messageItem.getTime());
-		if (SDK_INT >= 23) {
-			String transitionName = item.getTransitionName();
-			ActivityOptionsCompat options =
-					makeSceneTransitionAnimation(this, view, transitionName);
-			ActivityCompat.startActivity(this, i, options.toBundle());
-		} else {
-			// work-around for android bug #224270
-			startActivity(i);
-		}
+		// restoring list position should not trigger android bug #224270
+		String transitionName = item.getTransitionName();
+		ActivityOptionsCompat options =
+				makeSceneTransitionAnimation(this, view, transitionName);
+		ActivityCompat.startActivity(this, i, options.toBundle());
 	}
 
 	@DatabaseExecutor

briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageActivity.java

@@ -27,6 +27,8 @@ import android.view.ViewTreeObserver.OnGlobalLayoutListener;
 import android.view.Window;
 import android.widget.TextView;
 
+import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
+import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
 import org.briarproject.briar.R;
 import org.briarproject.briar.android.activity.ActivityComponent;
 import org.briarproject.briar.android.activity.BriarActivity;
@@ -55,6 +57,8 @@ import static java.util.Objects.requireNonNull;
 import static org.briarproject.briar.android.activity.RequestCodes.REQUEST_SAVE_ATTACHMENT;
 import static org.briarproject.briar.android.util.UiUtils.formatDateAbsolute;
 
+@MethodsNotNullByDefault
+@ParametersNotNullByDefault
 public class ImageActivity extends BriarActivity
 		implements PullDownLayout.Callback, OnGlobalLayoutListener {
 
@@ -82,7 +86,7 @@ public class ImageActivity extends BriarActivity
 		super.onCreate(state);
 
 		// Transitions
-		supportPostponeEnterTransition();
+		if (state == null) supportPostponeEnterTransition();
 		Window window = getWindow();
 		if (SDK_INT >= 21) {
 			Transition transition = new Fade();
@@ -97,7 +101,6 @@ public class ImageActivity extends BriarActivity
 		// inflate layout
 		setContentView(R.layout.activity_image);
 		layout = findViewById(R.id.layout);
-		layout.getBackground().setAlpha(255);
 		layout.setCallback(this);
 		layout.getViewTreeObserver().addOnGlobalLayoutListener(this);
 
@@ -174,9 +177,11 @@ public class ImageActivity extends BriarActivity
 	}
 
 	@Override
-	protected void onActivityResult(int request, int result, Intent data) {
+	protected void onActivityResult(int request, int result,
+			@Nullable Intent data) {
 		super.onActivityResult(request, result, data);
-		if (request == REQUEST_SAVE_ATTACHMENT && result == RESULT_OK) {
+		if (request == REQUEST_SAVE_ATTACHMENT && result == RESULT_OK &&
+				data != null) {
 			viewModel.saveImage(getVisibleAttachment(), data.getData());
 		}
 	}
@@ -190,7 +195,6 @@ public class ImageActivity extends BriarActivity
 
 	@Override
 	public void onPull(float progress) {
-		layout.getBackground().setAlpha(Math.round((1 - progress) * 255));
 	}
 
 	@Override
@@ -298,13 +302,18 @@ public class ImageActivity extends BriarActivity
 
 	private class ImagePagerAdapter extends FragmentStatePagerAdapter {
 
+		private boolean isFirst = true;
+
 		private ImagePagerAdapter(FragmentManager fm) {
 			super(fm);
 		}
 
 		@Override
 		public Fragment getItem(int position) {
-			return ImageFragment.newInstance(attachments.get(position));
+			Fragment f = ImageFragment
+					.newInstance(attachments.get(position), isFirst);
+			isFirst = false;
+			return f;
 		}
 
 		@Override

briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageFragment.java

@@ -35,17 +35,21 @@ import static org.briarproject.briar.android.conversation.ImageActivity.ATTACHME
 @ParametersAreNonnullByDefault
 public class ImageFragment extends Fragment {
 
+	private final static String IS_FIRST = "isFirst";
+
 	@Inject
 	ViewModelProvider.Factory viewModelFactory;
 
 	private AttachmentItem attachment;
+	private boolean isFirst;
 	private ImageViewModel viewModel;
 	private PhotoView photoView;
 
-	static ImageFragment newInstance(AttachmentItem a) {
+	static ImageFragment newInstance(AttachmentItem a, boolean isFirst) {
 		ImageFragment f = new ImageFragment();
 		Bundle args = new Bundle();
 		args.putParcelable(ATTACHMENT_POSITION, a);
+		args.putBoolean(IS_FIRST, isFirst);
 		f.setArguments(args);
 		return f;
 	}
@@ -63,6 +67,7 @@ public class ImageFragment extends Fragment {
 
 		Bundle args = requireNonNull(getArguments());
 		attachment = requireNonNull(args.getParcelable(ATTACHMENT_POSITION));
+		isFirst = args.getBoolean(IS_FIRST);
 	}
 
 	@Nullable
@@ -86,7 +91,7 @@ public class ImageFragment extends Fragment {
 			public boolean onLoadFailed(@Nullable GlideException e,
 					Object model, Target<Drawable> target,
 					boolean isFirstResource) {
-				if (getActivity() != null)
+				if (getActivity() != null && isFirst)
 					getActivity().supportStartPostponedEnterTransition();
 				return false;
 			}
@@ -105,8 +110,9 @@ public class ImageFragment extends Fragment {
 				if (viewModel.isOverlappingToolbar(photoView, resource)) {
 					photoView.setScaleType(FIT_START);
 				}
-				if (getActivity() != null)
+				if (getActivity() != null && isFirst) {
 					getActivity().supportStartPostponedEnterTransition();
+				}
 				return false;
 			}
 		};

briar-android/src/main/java/org/briarproject/briar/android/conversation/ImageViewHolder.java

@@ -15,6 +15,7 @@ import org.briarproject.briar.android.conversation.glide.BriarImageTransformatio
 import org.briarproject.briar.android.conversation.glide.GlideApp;
 import org.briarproject.briar.android.conversation.glide.Radii;
 
+import static android.os.Build.VERSION.SDK_INT;
 import static com.bumptech.glide.load.engine.DiskCacheStrategy.NONE;
 import static com.bumptech.glide.load.resource.drawable.DrawableTransitionOptions.withCrossFade;
 
@@ -42,6 +43,9 @@ class ImageViewHolder extends ViewHolder {
 		} else {
 			setImageViewDimensions(attachment, single, needsStretch);
 			loadImage(attachment, r);
+			if (SDK_INT >= 21) {
+				imageView.setTransitionName(attachment.getTransitionName());
+			}
 		}
 	}
 

briar-android/src/main/res/layout/activity_conversation.xml

@@ -49,7 +49,8 @@
 		android:id="@+id/conversationView"
 		android:layout_width="match_parent"
 		android:layout_height="0dp"
-		android:layout_weight="2"/>
+		android:layout_weight="2"
+		app:scrollToEnd="false"/>
 
 	<org.briarproject.briar.android.view.ImagePreview
 		android:id="@+id/imagePreview"

briar-android/src/main/res/values/themes.xml

@@ -18,9 +18,7 @@
 		<item name="toolbarStyle">@style/BriarToolbar</item>
 	</style>
 
-	<style name="BriarTheme.Transparent.NoActionBar" parent="BriarTheme.NoActionBar">
-		<item name="android:windowBackground">@android:color/transparent</item>
-		<item name="android:windowIsTranslucent">true</item>
+	<style name="BriarTheme.ActionBarOverlay" parent="BriarTheme.NoActionBar">
 		<item name="android:windowActionBarOverlay">true</item>
 	</style>