Fully remove Toolbar children when action views expand

The previous implementation of toggling visibility resulted
developer-set child visibility getting stomped. Fully detach child
views that shouldn't be shown instead. This is more consistent with
the coming and going of other system child views.

Bug 18156260

Change-Id: I8469d45eeb950dc0232f703efe8401233241c657

diff --git a/core/java/android/widget/Toolbar.java b/core/java/android/widget/Toolbar.java
index 8ace0f3..2ea2667 100644 (file)
--- a/core/java/android/widget/Toolbar.java
+++ b/core/java/android/widget/Toolbar.java
@@ -26,7 +26,6 @@ import android.annotation.StyleRes;
 import android.app.ActionBar;
 import android.content.Context;
 import android.content.res.TypedArray;
-import android.graphics.RectF;
 import android.graphics.drawable.Drawable;
 import android.os.Parcel;
 import android.os.Parcelable;
@@ -148,6 +147,9 @@ public class Toolbar extends ViewGroup {
     // Clear me after use.
     private final ArrayList<View> mTempViews = new ArrayList<View>();
 
+    // Used to hold views that will be removed while we have an expanded action view.
+    private final ArrayList<View> mHiddenViews = new ArrayList<>();
+
     private final int[] mTempMargins = new int[2];
 
     private OnMenuItemClickListener mOnMenuItemClickListener;
@@ -435,12 +437,12 @@ public class Toolbar extends ViewGroup {
     public void setLogo(Drawable drawable) {
         if (drawable != null) {
             ensureLogoView();
-            if (mLogoView.getParent() == null) {
-                addSystemView(mLogoView);
-                updateChildVisibilityForExpandedActionView(mLogoView);
+            if (!isChildOrHidden(mLogoView)) {
+                addSystemView(mLogoView, true);
             }
-        } else if (mLogoView != null && mLogoView.getParent() != null) {
+        } else if (mLogoView != null && isChildOrHidden(mLogoView)) {
             removeView(mLogoView);
+            mHiddenViews.remove(mLogoView);
         }
         if (mLogoView != null) {
             mLogoView.setImageDrawable(drawable);
@@ -577,12 +579,12 @@ public class Toolbar extends ViewGroup {
                     mTitleTextView.setTextColor(mTitleTextColor);
                 }
             }
-            if (mTitleTextView.getParent() == null) {
-                addSystemView(mTitleTextView);
-                updateChildVisibilityForExpandedActionView(mTitleTextView);
+            if (!isChildOrHidden(mTitleTextView)) {
+                addSystemView(mTitleTextView, true);
             }
-        } else if (mTitleTextView != null && mTitleTextView.getParent() != null) {
+        } else if (mTitleTextView != null && isChildOrHidden(mTitleTextView)) {
             removeView(mTitleTextView);
+            mHiddenViews.remove(mTitleTextView);
         }
         if (mTitleTextView != null) {
             mTitleTextView.setText(title);
@@ -631,12 +633,12 @@ public class Toolbar extends ViewGroup {
                     mSubtitleTextView.setTextColor(mSubtitleTextColor);
                 }
             }
-            if (mSubtitleTextView.getParent() == null) {
-                addSystemView(mSubtitleTextView);
-                updateChildVisibilityForExpandedActionView(mSubtitleTextView);
+            if (!isChildOrHidden(mSubtitleTextView)) {
+                addSystemView(mSubtitleTextView, true);
             }
-        } else if (mSubtitleTextView != null && mSubtitleTextView.getParent() != null) {
+        } else if (mSubtitleTextView != null && isChildOrHidden(mSubtitleTextView)) {
             removeView(mSubtitleTextView);
+            mHiddenViews.remove(mSubtitleTextView);
         }
         if (mSubtitleTextView != null) {
             mSubtitleTextView.setText(subtitle);
@@ -772,12 +774,12 @@ public class Toolbar extends ViewGroup {
     public void setNavigationIcon(@Nullable Drawable icon) {
         if (icon != null) {
             ensureNavButtonView();
-            if (mNavButtonView.getParent() == null) {
-                addSystemView(mNavButtonView);
-                updateChildVisibilityForExpandedActionView(mNavButtonView);
+            if (!isChildOrHidden(mNavButtonView)) {
+                addSystemView(mNavButtonView, true);
             }
-        } else if (mNavButtonView != null && mNavButtonView.getParent() != null) {
+        } else if (mNavButtonView != null && isChildOrHidden(mNavButtonView)) {
             removeView(mNavButtonView);
+            mHiddenViews.remove(mNavButtonView);
         }
         if (mNavButtonView != null) {
             mNavButtonView.setImageDrawable(icon);
@@ -866,7 +868,7 @@ public class Toolbar extends ViewGroup {
             final LayoutParams lp = generateDefaultLayoutParams();
             lp.gravity = Gravity.END | (mButtonGravity & Gravity.VERTICAL_GRAVITY_MASK);
             mMenuView.setLayoutParams(lp);
-            addSystemView(mMenuView);
+            addSystemView(mMenuView, false);
         }
     }
 
@@ -1041,7 +1043,7 @@ public class Toolbar extends ViewGroup {
         }
     }
 
-    private void addSystemView(View v) {
+    private void addSystemView(View v, boolean allowHide) {
         final ViewGroup.LayoutParams vlp = v.getLayoutParams();
         final LayoutParams lp;
         if (vlp == null) {
@@ -1052,7 +1054,13 @@ public class Toolbar extends ViewGroup {
             lp = (LayoutParams) vlp;
         }
         lp.mViewType = LayoutParams.SYSTEM;
-        addView(v, lp);
+
+        if (allowHide && mExpandedActionView != null) {
+            v.setLayoutParams(lp);
+            mHiddenViews.add(v);
+        } else {
+            addView(v, lp);
+        }
     }
 
     @Override
@@ -1741,22 +1749,30 @@ public class Toolbar extends ViewGroup {
         return mWrapper;
     }
 
-    private void setChildVisibilityForExpandedActionView(boolean expand) {
+    void removeChildrenForExpandedActionView() {
         final int childCount = getChildCount();
-        for (int i = 0; i < childCount; i++) {
+        // Go backwards since we're removing from the list
+        for (int i = childCount - 1; i >= 0; i--) {
             final View child = getChildAt(i);
             final LayoutParams lp = (LayoutParams) child.getLayoutParams();
             if (lp.mViewType != LayoutParams.EXPANDED && child != mMenuView) {
-                child.setVisibility(expand ? GONE : VISIBLE);
+                removeViewAt(i);
+                mHiddenViews.add(child);
             }
         }
     }
 
-    private void updateChildVisibilityForExpandedActionView(View child) {
-        final LayoutParams lp = (LayoutParams) child.getLayoutParams();
-        if (lp.mViewType != LayoutParams.EXPANDED && child != mMenuView) {
-            child.setVisibility(mExpandedActionView != null ? GONE : VISIBLE);
+    void addChildrenForExpandedActionView() {
+        final int count = mHiddenViews.size();
+        // Re-add in reverse order since we removed in reverse order
+        for (int i = count - 1; i >= 0; i--) {
+            addView(mHiddenViews.get(i));
         }
+        mHiddenViews.clear();
+    }
+
+    private boolean isChildOrHidden(View child) {
+        return child.getParent() == this || mHiddenViews.contains(child);
     }
 
     /**
@@ -1971,7 +1987,7 @@ public class Toolbar extends ViewGroup {
                 addView(mExpandedActionView);
             }
 
-            setChildVisibilityForExpandedActionView(true);
+            removeChildrenForExpandedActionView();
             requestLayout();
             item.setActionViewExpanded(true);
 
@@ -1994,7 +2010,7 @@ public class Toolbar extends ViewGroup {
             removeView(mCollapseButtonView);
             mExpandedActionView = null;
 
-            setChildVisibilityForExpandedActionView(false);
+            addChildrenForExpandedActionView();
             mCurrentExpandedItem = null;
             requestLayout();
             item.setActionViewExpanded(false);