fix slow 2-pane transitions; fix 2-pane animations in portrait
The instant visibility changes on layout after mode changes were
preventing the animations from running. Defer any cases when moving to
invisible state until after the transition is complete.
Move FAB reveal earlier when going CV->TL on 2-pane portrait by
triggering list-visibility-changed manually when entering TL mode. TL-
>CV hides the FAB after transition complete as usual.
Position and animate 2-pane elements after normal layout, since some
size calculations depend on having performed a layout already. Fixes
some FAB-related race conditions I started seeing with the animations
working again. b/17514925 tracks the fact that FAB shouldn't need to
move about in this way in the first place (since it's pretty much always
tethered to TL).
Also short-circuit an infinite loop that triggered continuous relayouts
when an Ad is present. This change reverts to the older logic of only
positioning/animating panels on known layout change events, which also
has the side effect of smoothing out the animation.
Bug: 17477114
Bug: 16818991
Change-Id: Ice1f6b896b9d7dc434debb3c2cf15aec87551ebc
diff --git a/src/com/android/mail/ui/AbstractActivityController.java b/src/com/android/mail/ui/AbstractActivityController.java
index f9d6305..431b06c 100644
--- a/src/com/android/mail/ui/AbstractActivityController.java
+++ b/src/com/android/mail/ui/AbstractActivityController.java
@@ -1162,6 +1162,8 @@
@Override
public void onConversationListVisibilityChanged(boolean visible) {
+ mFloatingComposeButton.setVisibility(visible ? View.VISIBLE : View.GONE);
+
informCursorVisiblity(visible);
commitAutoAdvanceOperation();
@@ -2208,23 +2210,12 @@
protected abstract void resetActionBarIcon();
/**
- * When should the compose button be visible
- */
- protected boolean isComposeVisible(int mode) {
- return mode == ViewMode.CONVERSATION_LIST;
- }
-
- /**
* {@inheritDoc} Subclasses must override this to listen to mode changes
* from the ViewMode. Subclasses <b>must</b> call the parent's
* onViewModeChanged since the parent will handle common state changes.
*/
@Override
public void onViewModeChanged(int newMode) {
- // The floating action compose button is only visible in the conversation/search lists
- final int composeVisible = isComposeVisible(newMode) ? View.VISIBLE : View.GONE;
- mFloatingComposeButton.setVisibility(composeVisible);
-
// When we step away from the conversation mode, we don't have a current conversation
// anymore. Let's blank it out so clients calling getCurrentConversation are not misled.
if (!ViewMode.isConversationMode(newMode)) {
diff --git a/src/com/android/mail/ui/TwoPaneController.java b/src/com/android/mail/ui/TwoPaneController.java
index 1ae9ed2..71d73d6 100644
--- a/src/com/android/mail/ui/TwoPaneController.java
+++ b/src/com/android/mail/ui/TwoPaneController.java
@@ -358,12 +358,6 @@
}
}
- @Override
- protected boolean isComposeVisible(int mode) {
- return super.isComposeVisible(mode) ||
- (mIsTabletLandscape && mode == ViewMode.CONVERSATION);
- }
-
/**
* Enable or disable the CAB mode based on the visibility of the conversation list fragment.
*/
diff --git a/src/com/android/mail/ui/TwoPaneLayout.java b/src/com/android/mail/ui/TwoPaneLayout.java
index fa8ae65..c4f612e 100644
--- a/src/com/android/mail/ui/TwoPaneLayout.java
+++ b/src/com/android/mail/ui/TwoPaneLayout.java
@@ -17,6 +17,8 @@
package com.android.mail.ui;
+import java.util.List;
+
import android.animation.Animator;
import android.animation.AnimatorListenerAdapter;
import android.animation.TimeInterpolator;
@@ -35,6 +37,7 @@
import com.android.mail.utils.Utils;
import com.android.mail.utils.ViewUtils;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.Lists;
/**
* This is a custom layout that manages the possible views of Gmail's large screen (read: tablet)
@@ -82,10 +85,15 @@
*/
private int mCurrentMode = ViewMode.UNKNOWN;
/**
- * This mode represents the current positions of the three panes. This is split out from the
- * current mode to give context to state transitions.
+ * This is a copy of {@link #mCurrentMode} that layout/positioning/animating code uses to
+ * compare to the 'new' current mode, to avoid unnecessarily calculation.
*/
private int mPositionedMode = ViewMode.UNKNOWN;
+ /**
+ * Similar to {@link #mPositionedMode}; this is the value of {@link #isDrawerOpen()} from the
+ * last time layout ran, so we know not to run layout again if this hasn't changed.
+ */
+ private boolean mPositionedIsDrawerOpen;
private TwoPaneController mController;
private LayoutListener mListener;
@@ -99,16 +107,16 @@
private View mFloatingActionButton;
private int mFloatingActionButtonEndMargin;
- private int mPrevComposeEdgeX = -1;
+
+ private final List<Runnable> mTransitionCompleteJobs = Lists.newArrayList();
+
+ private final PaneAnimationListener mPaneAnimationListener = new PaneAnimationListener();
public static final int MISCELLANEOUS_VIEW_ID = R.id.miscellaneous_pane;
- private final Runnable mTransitionCompleteRunnable = new Runnable() {
- @Override
- public void run() {
- onTransitionComplete();
- }
- };
+ public interface ConversationListLayoutListener {
+ void onConversationListLayout(int xEnd, boolean drawerOpen);
+ }
public TwoPaneLayout(Context context) {
this(context, null);
@@ -176,8 +184,15 @@
@Override
protected void onLayout(boolean changed, int l, int t, int r, int b) {
LogUtils.d(Utils.VIEW_DEBUGGING_TAG, "TPL(%s).onLayout()", this);
- positionPanes(getMeasuredWidth());
super.onLayout(changed, l, t, r, b);
+ // Position/animate the panes only if the layout has truly changed in a way that affects
+ // their positioning (e.g. mode change or drawer state change).
+ // And do so only after the normal layout has happened to give children their positions,
+ // in case they depend on them (e.g. if they call child.getWidth()).
+ if (changed || mPositionedMode != mCurrentMode
+ || isDrawerOpen() != mPositionedIsDrawerOpen) {
+ positionPanes(getMeasuredWidth());
+ }
}
/**
@@ -204,15 +219,11 @@
*/
private void positionPanes(int width) {
final boolean isRtl = ViewUtils.isViewRtl(this);
- // Always reset the FAB position to previous X (in case of rotation)
- if (mPrevComposeEdgeX >= 0) {
- mFloatingActionButton.setX(computeFloatingActionButtonX(mPrevComposeEdgeX, isRtl));
- mFloatingActions.setVisibility(VISIBLE);
- }
+ final boolean isDrawerOpen = isDrawerOpen();
final int convX, listX, foldersX;
- final int foldersW = isDrawerOpen() ? mDrawerWidthOpen : mDrawerWidthMini;
+ final int foldersW = isDrawerOpen ? mDrawerWidthOpen : mDrawerWidthMini;
final int listW = getPaneWidth(mListView);
boolean cvOnScreen = true;
@@ -254,37 +265,29 @@
}
}
+ // manually set FAB position the first time so it doesn't animate to its initial position
+ if (mFloatingActions.getVisibility() != VISIBLE && mCurrentMode != ViewMode.UNKNOWN) {
+ mFloatingActionButton.setX(computeFloatingActionButtonX(isRtl ? listX : convX, isRtl));
+ mFloatingActions.setVisibility(VISIBLE);
+ }
+
animatePanes(foldersX, listX, convX, isRtl);
// For views that are not on the screen, let's set their visibility for accessibility.
final boolean folderVisible = isRtl ?
foldersX + mFoldersView.getWidth() >= 0 : foldersX >= 0;
final boolean listVisible = isRtl ? listX + mListView.getWidth() >= 0 : listX >= 0;
- mFoldersView.setVisibility(folderVisible ? VISIBLE : INVISIBLE);
- mListView.setVisibility(listVisible ? VISIBLE : INVISIBLE);
- if (mConversationView.getVisibility() != GONE) {
- mConversationView.setVisibility(cvOnScreen ? VISIBLE : INVISIBLE);
- }
- if (mMiscellaneousView.getVisibility() != GONE) {
- mMiscellaneousView.setVisibility(cvOnScreen ? VISIBLE : INVISIBLE);
- }
+ adjustPaneVisibility(folderVisible, listVisible, cvOnScreen);
if (mConversationListLayoutListener != null) {
mConversationListLayoutListener.onConversationListLayout(
- isRtl ? listX : convX, isDrawerOpen());
+ isRtl ? listX : convX, isDrawerOpen);
}
mPositionedMode = mCurrentMode;
- mPrevComposeEdgeX = isRtl ? listX : convX;
+ mPositionedIsDrawerOpen = isDrawerOpen;
}
- private final AnimatorListenerAdapter mPaneAnimationListener = new AnimatorListenerAdapter() {
- @Override
- public void onAnimationEnd(Animator animation) {
- onTransitionComplete();
- }
- };
-
private void animatePanes(int foldersX, int listX, int convX, boolean isRtl) {
// If positioning has not yet happened, we don't need to animate panes into place.
// This happens on first layout, rotate, and when jumping straight to a conversation from
@@ -297,7 +300,7 @@
// listeners need to know that the "transition" is complete, even if one is not run.
// defer notifying listeners because we're in a layout pass, and they might do layout.
- post(mTransitionCompleteRunnable);
+ post(mPaneAnimationListener);
return;
}
@@ -308,9 +311,7 @@
}
mFoldersView.animate().x(foldersX);
- mListView.animate()
- .x(listX)
- .setListener(mPaneAnimationListener);
+ mListView.animate().x(listX).setListener(mPaneAnimationListener);
mFloatingActionButton.animate()
.x(computeFloatingActionButtonX(isRtl ? listX : convX, isRtl));
@@ -331,6 +332,41 @@
}
}
+ /**
+ * Adjusts the visibility of each pane before and after a transition. After the transition,
+ * any invisible panes should be marked invisible. But visible panes should not wait for the
+ * transition to finish-- they should be marked visible immediately.
+ *
+ */
+ private void adjustPaneVisibility(final boolean folderVisible, final boolean listVisible,
+ final boolean cvVisible) {
+ applyPaneVisibility(VISIBLE, folderVisible, listVisible, cvVisible);
+ mTransitionCompleteJobs.add(new Runnable() {
+ @Override
+ public void run() {
+ applyPaneVisibility(INVISIBLE, !folderVisible, !listVisible, !cvVisible);
+ }
+ });
+ }
+
+ private void applyPaneVisibility(int visibility, boolean applyToFolders, boolean applyToList,
+ boolean applyToCV) {
+ if (applyToFolders) {
+ mFoldersView.setVisibility(visibility);
+ }
+ if (applyToList) {
+ mListView.setVisibility(visibility);
+ }
+ if (applyToCV) {
+ if (mConversationView.getVisibility() != GONE) {
+ mConversationView.setVisibility(visibility);
+ }
+ if (mMiscellaneousView.getVisibility() != GONE) {
+ mMiscellaneousView.setVisibility(visibility);
+ }
+ }
+ }
+
private void onTransitionComplete() {
if (mController.isDestroyed()) {
// quit early if the hosting activity was destroyed before the animation finished
@@ -338,6 +374,11 @@
return;
}
+ for (Runnable job : mTransitionCompleteJobs) {
+ job.run();
+ }
+ mTransitionCompleteJobs.clear();
+
switch (mCurrentMode) {
case ViewMode.CONVERSATION:
case ViewMode.SEARCH_RESULTS_CONVERSATION:
@@ -439,6 +480,12 @@
mController.disablePagerUpdates();
}
+ // notify of list visibility change up-front when going to list mode
+ // (so the transition runs with the full TL in view)
+ if (newMode == ViewMode.CONVERSATION_LIST) {
+ dispatchConversationListVisibilityChange(true);
+ }
+
mCurrentMode = newMode;
LogUtils.i(LOG_TAG, "onViewModeChanged(%d)", newMode);
@@ -483,7 +530,18 @@
mConversationListLayoutListener = listener;
}
- public interface ConversationListLayoutListener {
- void onConversationListLayout(int xEnd, boolean drawerOpen);
+ private class PaneAnimationListener extends AnimatorListenerAdapter implements Runnable {
+
+ @Override
+ public void run() {
+ onTransitionComplete();
+ }
+
+ @Override
+ public void onAnimationEnd(Animator animation) {
+ onTransitionComplete();
+ }
+
}
+
}