Merge "fix(status bar): wifi and mobile icons flickering in large font/display size" into main

4 files changed, 103 insertions, 24 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/binder/MobileIconBinder.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/binder/MobileIconBinder.kt
index 249ca35b610a..7ec8e12e557e 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/binder/MobileIconBinder.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/binder/MobileIconBinder.kt
@@ -19,7 +19,6 @@ package com.android.systemui.statusbar.pipeline.mobile.ui.binder
 import android.content.res.ColorStateList
 import android.view.View
 import android.view.View.GONE
-import android.view.View.INVISIBLE
 import android.view.View.VISIBLE
 import android.view.ViewGroup
 import android.widget.ImageView
@@ -34,12 +33,11 @@ import com.android.systemui.common.ui.binder.ContentDescriptionViewBinder
 import com.android.systemui.common.ui.binder.IconViewBinder
 import com.android.systemui.lifecycle.repeatWhenAttached
 import com.android.systemui.statusbar.StatusBarIconView
-import com.android.systemui.statusbar.StatusBarIconView.STATE_DOT
 import com.android.systemui.statusbar.StatusBarIconView.STATE_HIDDEN
-import com.android.systemui.statusbar.StatusBarIconView.STATE_ICON
 import com.android.systemui.statusbar.pipeline.mobile.ui.MobileViewLogger
 import com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel.LocationBasedMobileViewModel
 import com.android.systemui.statusbar.pipeline.shared.ui.binder.ModernStatusBarViewBinding
+import com.android.systemui.statusbar.pipeline.shared.ui.binder.ModernStatusBarViewVisibilityHelper
 import kotlinx.coroutines.awaitCancellation
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.distinctUntilChanged
@@ -106,20 +104,11 @@ object MobileIconBinder {
 
                     launch {
                         visibilityState.collect { state ->
-                            when (state) {
-                                STATE_ICON -> {
-                                    mobileGroupView.visibility = VISIBLE
-                                    dotView.visibility = GONE
-                                }
-                                STATE_DOT -> {
-                                    mobileGroupView.visibility = INVISIBLE
-                                    dotView.visibility = VISIBLE
-                                }
-                                STATE_HIDDEN -> {
-                                    mobileGroupView.visibility = INVISIBLE
-                                    dotView.visibility = INVISIBLE
-                                }
-                            }
+                            ModernStatusBarViewVisibilityHelper.setVisibilityState(
+                                state,
+                                mobileGroupView,
+                                dotView,
+                            )
                         }
                     }
 
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ui/binder/ModernStatusBarViewVisibilityHelper.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ui/binder/ModernStatusBarViewVisibilityHelper.kt
new file mode 100644
index 000000000000..6668cbc290fc
--- /dev/null
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ui/binder/ModernStatusBarViewVisibilityHelper.kt
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.systemui.statusbar.pipeline.shared.ui.binder
+
+import android.view.View
+import com.android.systemui.statusbar.StatusBarIconView
+import com.android.systemui.statusbar.pipeline.mobile.ui.binder.MobileIconBinder
+import com.android.systemui.statusbar.pipeline.wifi.ui.binder.WifiViewBinder
+
+/**
+ * The helper to update the groupView and dotView visibility based on given visibility state, only
+ * used for [MobileIconBinder] and [WifiViewBinder] now.
+ */
+class ModernStatusBarViewVisibilityHelper {
+    companion object {
+
+        fun setVisibilityState(
+            @StatusBarIconView.VisibleState state: Int,
+            groupView: View,
+            dotView: View,
+        ) {
+            when (state) {
+                StatusBarIconView.STATE_ICON -> {
+                    groupView.visibility = View.VISIBLE
+                    dotView.visibility = View.GONE
+                }
+                StatusBarIconView.STATE_DOT -> {
+                    groupView.visibility = View.INVISIBLE
+                    dotView.visibility = View.VISIBLE
+                }
+                StatusBarIconView.STATE_HIDDEN -> {
+                    groupView.visibility = View.INVISIBLE
+                    dotView.visibility = View.INVISIBLE
+                }
+            }
+        }
+    }
+}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/binder/WifiViewBinder.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/binder/WifiViewBinder.kt
index 3082a6629dc2..e593575bd791 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/binder/WifiViewBinder.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/binder/WifiViewBinder.kt
@@ -27,10 +27,9 @@ import com.android.systemui.R
 import com.android.systemui.common.ui.binder.IconViewBinder
 import com.android.systemui.lifecycle.repeatWhenAttached
 import com.android.systemui.statusbar.StatusBarIconView
-import com.android.systemui.statusbar.StatusBarIconView.STATE_DOT
 import com.android.systemui.statusbar.StatusBarIconView.STATE_HIDDEN
-import com.android.systemui.statusbar.StatusBarIconView.STATE_ICON
 import com.android.systemui.statusbar.pipeline.shared.ui.binder.ModernStatusBarViewBinding
+import com.android.systemui.statusbar.pipeline.shared.ui.binder.ModernStatusBarViewVisibilityHelper
 import com.android.systemui.statusbar.pipeline.wifi.ui.model.WifiIcon
 import com.android.systemui.statusbar.pipeline.wifi.ui.viewmodel.LocationBasedWifiViewModel
 import kotlinx.coroutines.InternalCoroutinesApi
@@ -83,8 +82,18 @@ object WifiViewBinder {
 
                 launch {
                     visibilityState.collect { visibilityState ->
-                        groupView.isVisible = visibilityState == STATE_ICON
-                        dotView.isVisible = visibilityState == STATE_DOT
+                        // for b/296864006, we can not hide all the child views if visibilityState
+                        // is STATE_HIDDEN. Because hiding all child views would cause the
+                        // getWidth() of this view return 0, and that would cause the translation
+                        // calculation fails in StatusIconContainer. Therefore, like class
+                        // MobileIconBinder, instead of set the child views visibility to View.GONE,
+                        // we set their visibility to View.INVISIBLE to make them invisible but
+                        // keep the width.
+                        ModernStatusBarViewVisibilityHelper.setVisibilityState(
+                            visibilityState,
+                            groupView,
+                            dotView,
+                        )
                     }
                 }
 
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt
index a0d4d1390b2c..bbf048d6697f 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt
@@ -22,6 +22,7 @@ import android.testing.TestableLooper
 import android.testing.TestableLooper.RunWithLooper
 import android.testing.ViewUtils
 import android.view.View
+import android.view.ViewGroup
 import android.widget.ImageView
 import androidx.test.filters.SmallTest
 import com.android.systemui.R
@@ -138,7 +139,7 @@ class ModernStatusBarWifiViewTest : SysuiTestCase() {
         ViewUtils.attachView(view)
         testableLooper.processAllMessages()
 
-        assertThat(view.getIconGroupView().visibility).isEqualTo(View.GONE)
+        assertThat(view.getIconGroupView().visibility).isEqualTo(View.INVISIBLE)
         assertThat(view.getDotView().visibility).isEqualTo(View.VISIBLE)
 
         ViewUtils.detachView(view)
@@ -153,8 +154,36 @@ class ModernStatusBarWifiViewTest : SysuiTestCase() {
         ViewUtils.attachView(view)
         testableLooper.processAllMessages()
 
-        assertThat(view.getIconGroupView().visibility).isEqualTo(View.GONE)
-        assertThat(view.getDotView().visibility).isEqualTo(View.GONE)
+        assertThat(view.getIconGroupView().visibility).isEqualTo(View.INVISIBLE)
+        assertThat(view.getDotView().visibility).isEqualTo(View.INVISIBLE)
+
+        ViewUtils.detachView(view)
+    }
+
+    /* Regression test for b/296864006. When STATE_HIDDEN we need to ensure the wifi view width
+     * would not break the StatusIconContainer translation calculation. */
+    @Test
+    fun setVisibleState_hidden_keepWidth() {
+        val view = ModernStatusBarWifiView.constructAndBind(context, SLOT_NAME, viewModel)
+
+        view.setVisibleState(STATE_ICON, /* animate= */ false)
+
+        // get the view width when it's in visible state
+        ViewUtils.attachView(view)
+        val lp = view.layoutParams
+        lp.width = ViewGroup.LayoutParams.WRAP_CONTENT
+        lp.height = ViewGroup.LayoutParams.WRAP_CONTENT
+        view.layoutParams = lp
+        testableLooper.processAllMessages()
+        val currentWidth = view.width
+
+        view.setVisibleState(STATE_HIDDEN, /* animate= */ false)
+        testableLooper.processAllMessages()
+
+        // the view width when STATE_HIDDEN should be at least the width when STATE_ICON. Because
+        // when STATE_HIDDEN the invisible dot view width might be larger than group view width,
+        // then the wifi view width would be enlarged.
+        assertThat(view.width).isAtLeast(currentWidth)
 
         ViewUtils.detachView(view)
     }