From ebcf5a525622448498bd27b9ba1fd639b7d662f5 Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Mon, 8 May 2023 10:30:01 -0400 Subject: [PATCH] Account for the bouncer's scrim in LightBarController 1) The ScrimController's alpha is irrelevant when the bouncer is showing, because the bouncer's scrim is baked into that view, so we always want `forceForScrim` to be true when the bouncer is showing. 2) Moreover, the bouncer covers expanded QS, so tweak the later logic to ignore the QS override when the bouncer is visible. Fixes: 280434803 Test: atest LightBarControllerTest Change-Id: Id4236235ce11b473d64d36431c9a4e5f70caf2c9 --- .../statusbar/phone/LightBarController.java | 20 +++- .../phone/LightBarControllerTest.java | 96 +++++++++++++++++-- 2 files changed, 104 insertions(+), 12 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/LightBarController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/LightBarController.java index a058bf88e2e4b..a6b2bd89bfa53 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/LightBarController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/LightBarController.java @@ -103,6 +103,7 @@ public class LightBarController implements BatteryController.BatteryStateChangeC private boolean mQsCustomizing; private boolean mQsExpanded; + private boolean mBouncerVisible; private boolean mGlobalActionsVisible; private boolean mDirectReplying; @@ -188,9 +189,10 @@ public class LightBarController implements BatteryController.BatteryStateChangeC final boolean ignoreScrimForce = mDirectReplying && mNavbarColorManagedByIme; final boolean darkForScrim = mForceDarkForScrim && !ignoreScrimForce; final boolean lightForScrim = mForceLightForScrim && !ignoreScrimForce; - final boolean darkForQs = mQsCustomizing || mQsExpanded || mGlobalActionsVisible; + final boolean darkForQs = (mQsCustomizing || mQsExpanded) && !mBouncerVisible; + final boolean darkForTop = darkForQs || mGlobalActionsVisible; mNavigationLight = - ((mHasLightNavigationBar && !darkForScrim) || lightForScrim) && !darkForQs; + ((mHasLightNavigationBar && !darkForScrim) || lightForScrim) && !darkForTop; mLastNavigationBarAppearanceChangedLog = "onNavigationBarAppearanceChanged()" + " appearance=" + appearance + " nbModeChanged=" + nbModeChanged @@ -201,6 +203,7 @@ public class LightBarController implements BatteryController.BatteryStateChangeC + " darkForScrim=" + darkForScrim + " lightForScrim=" + lightForScrim + " darkForQs=" + darkForQs + + " darkForTop=" + darkForTop + " mNavigationLight=" + mNavigationLight + " last=" + last + " timestamp=" + new Date(); @@ -298,15 +301,20 @@ public class LightBarController implements BatteryController.BatteryStateChangeC public void setScrimState(ScrimState scrimState, float scrimBehindAlpha, GradientColors scrimInFrontColor) { if (mUseNewLightBarLogic) { + boolean bouncerVisibleLast = mBouncerVisible; boolean forceDarkForScrimLast = mForceDarkForScrim; boolean forceLightForScrimLast = mForceLightForScrim; - final boolean forceForScrim = - scrimBehindAlpha >= NAV_BAR_INVERSION_SCRIM_ALPHA_THRESHOLD; + mBouncerVisible = + scrimState == ScrimState.BOUNCER || scrimState == ScrimState.BOUNCER_SCRIMMED; + final boolean forceForScrim = mBouncerVisible + || scrimBehindAlpha >= NAV_BAR_INVERSION_SCRIM_ALPHA_THRESHOLD; final boolean scrimColorIsLight = scrimInFrontColor.supportsDarkText(); mForceDarkForScrim = forceForScrim && !scrimColorIsLight; mForceLightForScrim = forceForScrim && scrimColorIsLight; - if (mHasLightNavigationBar) { + if (mBouncerVisible != bouncerVisibleLast) { + reevaluate(); + } else if (mHasLightNavigationBar) { if (mForceDarkForScrim != forceDarkForScrimLast) reevaluate(); } else { if (mForceLightForScrim != forceLightForScrimLast) reevaluate(); @@ -318,6 +326,7 @@ public class LightBarController implements BatteryController.BatteryStateChangeC + " forceForScrim=" + forceForScrim + " scrimColorIsLight=" + scrimColorIsLight + " mHasLightNavigationBar=" + mHasLightNavigationBar + + " mBouncerVisible=" + mBouncerVisible + " mForceDarkForScrim=" + mForceDarkForScrim + " mForceLightForScrim=" + mForceLightForScrim + " timestamp=" + new Date(); @@ -428,6 +437,7 @@ public class LightBarController implements BatteryController.BatteryStateChangeC pw.println(); pw.print(" mQsCustomizing="); pw.println(mQsCustomizing); pw.print(" mQsExpanded="); pw.println(mQsExpanded); + pw.print(" mBouncerVisible="); pw.println(mBouncerVisible); pw.print(" mGlobalActionsVisible="); pw.println(mGlobalActionsVisible); pw.print(" mDirectReplying="); pw.println(mDirectReplying); pw.print(" mNavbarColorManagedByIme="); pw.println(mNavbarColorManagedByIme); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LightBarControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LightBarControllerTest.java index a501556280271..6a4b3c55f2442 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LightBarControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/LightBarControllerTest.java @@ -222,31 +222,103 @@ public class LightBarControllerTest extends SysuiTestCase { // Initial state is set when controller is set mLightBarController.setNavigationBar(mNavBarController); - verifyNavBarIconsDarkSetTo(false); + verifyNavBarIconsDark(false, /* didFireEvent= */ true); // Changing the color of the transparent scrim has no effect mLightBarController.setScrimState(ScrimState.UNLOCKED, 0f, COLORS_LIGHT); - verifyNavBarIconsUnchanged(); // still light + verifyNavBarIconsDark(false, /* didFireEvent= */ false); // Showing the notification shade with white scrim requires dark icons mLightBarController.setScrimState(ScrimState.UNLOCKED, 1f, COLORS_LIGHT); - verifyNavBarIconsDarkSetTo(true); + verifyNavBarIconsDark(true, /* didFireEvent= */ true); // Expanded QS always provides a black background, so icons become light again mLightBarController.setQsExpanded(true); - verifyNavBarIconsDarkSetTo(false); + verifyNavBarIconsDark(false, /* didFireEvent= */ true); // Tapping the QS tile to change to dark theme has no effect in this state mLightBarController.setScrimState(ScrimState.UNLOCKED, 1f, COLORS_DARK); - verifyNavBarIconsUnchanged(); // still light + verifyNavBarIconsDark(false, /* didFireEvent= */ false); // collapsing QS in dark mode doesn't affect button color mLightBarController.setQsExpanded(false); - verifyNavBarIconsUnchanged(); // still light + verifyNavBarIconsDark(false, /* didFireEvent= */ false); // Closing the shade has no affect mLightBarController.setScrimState(ScrimState.UNLOCKED, 0f, COLORS_DARK); - verifyNavBarIconsUnchanged(); // still light + verifyNavBarIconsDark(false, /* didFireEvent= */ false); + } + + @Test + public void navBarHasDarkIconsInLockedShade_lightMode() { + assumeTrue(testNewLightBarLogic()); // Only run in the new suite + + // On the locked shade QS in light mode buttons are light + mLightBarController.setScrimState(ScrimState.SHADE_LOCKED, 1f, COLORS_LIGHT); + mLightBarController.onNavigationBarAppearanceChanged( + 0, /* nbModeChanged = */ true, + MODE_TRANSPARENT, /* navbarColorManagedByIme = */ false); + verifyNavBarIconsUnchanged(); // no changes yet; not attached + + // Initial state is set when controller is set + mLightBarController.setNavigationBar(mNavBarController); + verifyNavBarIconsDark(true, /* didFireEvent= */ true); + } + + @Test + public void navBarHasLightIconsInLockedQs_lightMode() { + // GIVEN dark icons in locked shade in light mdoe + navBarHasDarkIconsInLockedShade_lightMode(); + // WHEN expanding QS + mLightBarController.setQsExpanded(true); + // THEN icons become light + verifyNavBarIconsDark(false, /* didFireEvent= */ true); + } + + @Test + public void navBarHasDarkIconsInBouncerOverQs_lightMode() { + // GIVEN that light icons in locked expanded QS + navBarHasLightIconsInLockedQs_lightMode(); + // WHEN device changes to bouncer + mLightBarController.setScrimState(ScrimState.BOUNCER, 1f, COLORS_LIGHT); + // THEN icons change to dark + verifyNavBarIconsDark(true, /* didFireEvent= */ true); + } + + @Test + public void navBarHasLightIconsInLockedShade_darkMode() { + assumeTrue(testNewLightBarLogic()); // Only run in the new suite + + // On the locked shade QS in light mode buttons are light + mLightBarController.setScrimState(ScrimState.SHADE_LOCKED, 1f, COLORS_DARK); + mLightBarController.onNavigationBarAppearanceChanged( + 0, /* nbModeChanged = */ true, + MODE_TRANSPARENT, /* navbarColorManagedByIme = */ false); + verifyNavBarIconsUnchanged(); // no changes yet; not attached + + // Initial state is set when controller is set + mLightBarController.setNavigationBar(mNavBarController); + verifyNavBarIconsDark(false, /* didFireEvent= */ true); + } + + @Test + public void navBarHasLightIconsInLockedQs_darkMode() { + // GIVEN light icons in the locked shade + navBarHasLightIconsInLockedShade_darkMode(); + // WHEN QS expands + mLightBarController.setQsExpanded(true); + // THEN icons stay light + verifyNavBarIconsDark(false, /* didFireEvent= */ false); + } + + @Test + public void navBarHasLightIconsInBouncerOverQs_darkMode() { + // GIVEN that light icons in locked expanded QS + navBarHasLightIconsInLockedQs_darkMode(); + // WHEN device changes to bouncer + mLightBarController.setScrimState(ScrimState.BOUNCER, 1f, COLORS_DARK); + // THEN icons stay light + verifyNavBarIconsDark(false, /* didFireEvent= */ false); } private void verifyNavBarIconsUnchanged() { @@ -258,4 +330,14 @@ public class LightBarControllerTest extends SysuiTestCase { verify(mNavBarController, never()).setIconsDark(eq(!iconsDark), anyBoolean()); clearInvocations(mNavBarController); } + + private void verifyNavBarIconsDark(boolean iconsDark, boolean didFireEvent) { + if (didFireEvent) { + verifyNavBarIconsDarkSetTo(iconsDark); + } else { + verifyNavBarIconsUnchanged(); + mLightBarController.setNavigationBar(mNavBarController); + verifyNavBarIconsDarkSetTo(iconsDark); + } + } }