diff --git a/conductor/src/main/java/com/bluelinelabs/conductor/ControllerChangeHandler.java b/conductor/src/main/java/com/bluelinelabs/conductor/ControllerChangeHandler.java index 118d5a8..921fcb6 100644 --- a/conductor/src/main/java/com/bluelinelabs/conductor/ControllerChangeHandler.java +++ b/conductor/src/main/java/com/bluelinelabs/conductor/ControllerChangeHandler.java @@ -145,11 +145,11 @@ public abstract class ControllerChangeHandler { } } - static void executeChange(@Nullable final Controller to, @Nullable final Controller from, final boolean isPush, @Nullable final ViewGroup container, @Nullable final ControllerChangeHandler inHandler, @NonNull final List listeners) { - if (isPush && to != null && to.isDestroyed()) { - throw new IllegalStateException("Trying to push a controller that has already been destroyed. (" + to.getClass().getSimpleName() + ")"); - } + static void executeChange(@NonNull final ChangeTransaction transaction) { + executeChange(transaction.to, transaction.from, transaction.isPush, transaction.container, transaction.changeHandler, transaction.listeners); + } + private static void executeChange(@Nullable final Controller to, @Nullable final Controller from, final boolean isPush, @Nullable final ViewGroup container, @Nullable final ControllerChangeHandler inHandler, @NonNull final List listeners) { if (container != null) { final ControllerChangeHandler handler; if (inHandler == null) { @@ -231,6 +231,24 @@ public abstract class ControllerChangeHandler { forceRemoveViewOnPush = force; } + static class ChangeTransaction { + @Nullable final Controller to; + @Nullable final Controller from; + final boolean isPush; + @Nullable final ViewGroup container; + @Nullable final ControllerChangeHandler changeHandler; + @NonNull final List listeners; + + public ChangeTransaction(@Nullable Controller to, @Nullable Controller from, boolean isPush, @Nullable ViewGroup container, @Nullable ControllerChangeHandler changeHandler, @NonNull List listeners) { + this.to = to; + this.from = from; + this.isPush = isPush; + this.container = container; + this.changeHandler = changeHandler; + this.listeners = listeners; + } + } + /** * A listener interface useful for allowing external classes to be notified of change events. */ diff --git a/conductor/src/main/java/com/bluelinelabs/conductor/Router.java b/conductor/src/main/java/com/bluelinelabs/conductor/Router.java index f8b5718..7a30e5d 100755 --- a/conductor/src/main/java/com/bluelinelabs/conductor/Router.java +++ b/conductor/src/main/java/com/bluelinelabs/conductor/Router.java @@ -14,6 +14,7 @@ import android.view.View; import android.view.ViewGroup; import com.bluelinelabs.conductor.Controller.LifecycleListener; +import com.bluelinelabs.conductor.ControllerChangeHandler.ChangeTransaction; import com.bluelinelabs.conductor.ControllerChangeHandler.ControllerChangeListener; import com.bluelinelabs.conductor.changehandler.SimpleSwapChangeHandler; import com.bluelinelabs.conductor.internal.NoOpControllerChangeHandler; @@ -35,8 +36,9 @@ public abstract class Router { private static final String KEY_BACKSTACK = "Router.backstack"; private static final String KEY_POPS_LAST_VIEW = "Router.popsLastView"; - protected final Backstack backstack = new Backstack(); + final Backstack backstack = new Backstack(); private final List changeListeners = new ArrayList<>(); + private final List pendingControllerChanges = new ArrayList<>(); final List destroyingControllers = new ArrayList<>(); private boolean popsLastView = false; @@ -726,8 +728,8 @@ public abstract class Router { to.ensureValidIndex(getTransactionIndexer()); setControllerRouter(toController); } else if (backstack.size() == 0 && !popsLastView) { - // We're emptying out the backstack. Views get weird if you transition them out, so just no-op it. The hosting - // Activity should be handling this by finishing or at least hiding this view. + // We're emptying out the backstack. Views get weird if you transition them out, so just no-op it. The host + // Activity or controller should be handling this by finishing or at least hiding this view. changeHandler = new NoOpControllerChangeHandler(); } @@ -735,18 +737,39 @@ public abstract class Router { } private void performControllerChange(@Nullable final Controller to, @Nullable final Controller from, final boolean isPush, @Nullable final ControllerChangeHandler changeHandler) { - // If the change handler will remove the from view, we have to make sure the container is fully attached first so we avoid NPEs - // within ViewGroup (details on issue #287). Post this to the container to ensure the attach is complete before we try to remove - // anything. - if (from != null && (changeHandler == null || changeHandler.removesFromViewOnPush()) && !containerFullyAttached) { + if (isPush && to != null && to.isDestroyed()) { + throw new IllegalStateException("Trying to push a controller that has already been destroyed. (" + to.getClass().getSimpleName() + ")"); + } + + final ChangeTransaction transaction = new ChangeTransaction(to, from, isPush, container, changeHandler, changeListeners); + + if (pendingControllerChanges.size() > 0) { + // If we already have changes queued up (awaiting full container attach), queue this one up as well so they don't happen + // out of order. + + pendingControllerChanges.add(transaction); + } else if (from != null && (changeHandler == null || changeHandler.removesFromViewOnPush()) && !containerFullyAttached) { + // If the change handler will remove the from view, we have to make sure the container is fully attached first so we avoid NPEs + // within ViewGroup (details on issue #287). Post this to the container to ensure the attach is complete before we try to remove + // anything. + + pendingControllerChanges.add(transaction); container.post(new Runnable() { @Override public void run() { - ControllerChangeHandler.executeChange(to, from, isPush, container, changeHandler, changeListeners); + performPendingControllerChanges(); } }); } else { - ControllerChangeHandler.executeChange(to, from, isPush, container, changeHandler, changeListeners); + ControllerChangeHandler.executeChange(transaction); + } + } + + void performPendingControllerChanges() { + // We're intentionally using dynamic size checking (list.size()) here so we can account for changes + // that occur during this loop (ex: if a controller is popped from within onAttach) + for (int i = 0; i < pendingControllerChanges.size(); i++) { + ControllerChangeHandler.executeChange(pendingControllerChanges.get(i)); } } diff --git a/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/AnimatorChangeHandler.java b/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/AnimatorChangeHandler.java index e8476fd..5ed7fbe 100755 --- a/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/AnimatorChangeHandler.java +++ b/conductor/src/main/java/com/bluelinelabs/conductor/changehandler/AnimatorChangeHandler.java @@ -21,6 +21,7 @@ public abstract class AnimatorChangeHandler extends ControllerChangeHandler { private static final String KEY_DURATION = "AnimatorChangeHandler.duration"; private static final String KEY_REMOVES_FROM_ON_PUSH = "AnimatorChangeHandler.removesFromViewOnPush"; + @SuppressWarnings("WeakerAccess") public static final long DEFAULT_ANIMATION_DURATION = -1; private long animationDuration; @@ -29,15 +30,19 @@ public abstract class AnimatorChangeHandler extends ControllerChangeHandler { private boolean needsImmediateCompletion; private boolean completed; private Animator animator; + private OnAnimationReadyOrAbortedListener onAnimationReadyOrAbortedListener; + @SuppressWarnings("WeakerAccess") public AnimatorChangeHandler() { this(DEFAULT_ANIMATION_DURATION, true); } + @SuppressWarnings("WeakerAccess") public AnimatorChangeHandler(boolean removesFromViewOnPush) { this(DEFAULT_ANIMATION_DURATION, removesFromViewOnPush); } + @SuppressWarnings("WeakerAccess") public AnimatorChangeHandler(long duration) { this(duration, true); } @@ -68,6 +73,8 @@ public abstract class AnimatorChangeHandler extends ControllerChangeHandler { canceled = true; if (animator != null) { animator.cancel(); + } else if (onAnimationReadyOrAbortedListener != null) { + onAnimationReadyOrAbortedListener.onReadyOrAborted(); } } @@ -78,6 +85,8 @@ public abstract class AnimatorChangeHandler extends ControllerChangeHandler { needsImmediateCompletion = true; if (animator != null) { animator.end(); + } else if (onAnimationReadyOrAbortedListener != null) { + onAnimationReadyOrAbortedListener.onReadyOrAborted(); } } @@ -121,24 +130,8 @@ public abstract class AnimatorChangeHandler extends ControllerChangeHandler { if (to.getWidth() <= 0 && to.getHeight() <= 0) { readyToAnimate = false; - to.getViewTreeObserver().addOnPreDrawListener(new ViewTreeObserver.OnPreDrawListener() { - boolean hasRun; - - @Override - public boolean onPreDraw() { - final ViewTreeObserver observer = to.getViewTreeObserver(); - if (observer.isAlive()) { - observer.removeOnPreDrawListener(this); - } - - // Apparently this gets called multiple times, even if removeOnPreDrawListener is called successfully. - if (!hasRun) { - hasRun = true; - performAnimation(container, from, to, isPush, addingToView, changeListener); - } - return true; - } - }); + onAnimationReadyOrAbortedListener = new OnAnimationReadyOrAbortedListener(container, from, to, isPush, true, changeListener); + to.getViewTreeObserver().addOnPreDrawListener(onAnimationReadyOrAbortedListener); } } @@ -160,6 +153,8 @@ public abstract class AnimatorChangeHandler extends ControllerChangeHandler { animator.cancel(); animator = null; } + + onAnimationReadyOrAbortedListener = null; } private void performAnimation(@NonNull final ViewGroup container, @Nullable final View from, @Nullable final View to, final boolean isPush, final boolean toAddedToContainer, @NonNull final ControllerChangeCompletedListener changeListener) { @@ -213,4 +208,46 @@ public abstract class AnimatorChangeHandler extends ControllerChangeHandler { animator.start(); } + private class OnAnimationReadyOrAbortedListener implements ViewTreeObserver.OnPreDrawListener { + @NonNull final ViewGroup container; + @Nullable final View from; + @Nullable final View to; + final boolean isPush; + final boolean addingToView; + @NonNull final ControllerChangeCompletedListener changeListener; + private boolean hasRun; + + OnAnimationReadyOrAbortedListener(@NonNull ViewGroup container, @Nullable View from, @Nullable View to, boolean isPush, boolean addingToView, @NonNull ControllerChangeCompletedListener changeListener) { + this.container = container; + this.from = from; + this.to = to; + this.isPush = isPush; + this.addingToView = addingToView; + this.changeListener = changeListener; + } + + @Override + public boolean onPreDraw() { + onReadyOrAborted(); + + return true; + } + + void onReadyOrAborted() { + if (!hasRun) { + hasRun = true; + + if (to != null) { + final ViewTreeObserver observer = to.getViewTreeObserver(); + if (observer.isAlive()) { + observer.removeOnPreDrawListener(this); + } + } + + performAnimation(container, from, to, isPush, addingToView, changeListener); + } + } + + } + } diff --git a/demo/src/main/java/com/bluelinelabs/conductor/demo/changehandler/SharedElementDelayingChangeHandler.java b/demo/src/main/java/com/bluelinelabs/conductor/demo/changehandler/SharedElementDelayingChangeHandler.java index d7cc653..98b8c04 100644 --- a/demo/src/main/java/com/bluelinelabs/conductor/demo/changehandler/SharedElementDelayingChangeHandler.java +++ b/demo/src/main/java/com/bluelinelabs/conductor/demo/changehandler/SharedElementDelayingChangeHandler.java @@ -70,6 +70,7 @@ public class SharedElementDelayingChangeHandler extends ArcFadeMoveChangeHandler if (waitForTransitionNames.size() == 0) { to.getViewTreeObserver().removeOnPreDrawListener(onPreDrawListener); + onPreDrawListener = null; to.setVisibility(View.INVISIBLE);