From 47550d4c8af9addf6478b0637db0a611379dd5df Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Mon, 20 Jan 2025 13:48:36 +0000 Subject: [PATCH] fix(brightness suw): Hide brightness preference in the suw. Root cause: there's a mismatch in how visibility is determined for the AutoBrightnessPreferenceControllerForSetupWizard. The getAvailabilityStatus method and the displayPreference method (specifically the preference.setVisible call) use different conditions for showing the preference. Solution: To ensure consistency, I propose aligning these conditions by incorporating an aconfig flag check in both places. This will prevent unexpected behavior and make the logic clearer. Bug: 389011125 Flag: com.android.settings.accessibility.add_brightness_settings_in_suw Test: atest AutoBrightnessPreferenceControllerForSetupWizardTest BrightnessLevelPreferenceControllerForSetupWizardTest Change-Id: I004bfe8b1e039734356715c971f0bfbe56ffa9db --- ...htnessPreferenceControllerForSetupWizard.java | 7 +++---- ...sLevelPreferenceControllerForSetupWizard.java | 7 +++---- ...ssPreferenceControllerForSetupWizardTest.java | 16 ++++++++++++++++ ...elPreferenceControllerForSetupWizardTest.java | 13 +++++++++++++ 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/src/com/android/settings/display/AutoBrightnessPreferenceControllerForSetupWizard.java b/src/com/android/settings/display/AutoBrightnessPreferenceControllerForSetupWizard.java index d9979a9a2c2..fda7a65a26a 100644 --- a/src/com/android/settings/display/AutoBrightnessPreferenceControllerForSetupWizard.java +++ b/src/com/android/settings/display/AutoBrightnessPreferenceControllerForSetupWizard.java @@ -49,12 +49,11 @@ public class AutoBrightnessPreferenceControllerForSetupWizard @Override public void displayPreference(PreferenceScreen screen) { - super.displayPreference(screen); - Preference preference = screen.findPreference(getPreferenceKey()); + final Preference preference = screen.findPreference(getPreferenceKey()); if (preference instanceof RestrictedPreferenceHelperProvider helperProvider) { mRestrictedPreferenceHelper = helperProvider.getRestrictedPreferenceHelper(); - preference.setVisible(!isRestricted()); } + super.displayPreference(screen); } @Override @@ -70,4 +69,4 @@ public class AutoBrightnessPreferenceControllerForSetupWizard public CharSequence getSummary() { return ""; } -} +} \ No newline at end of file diff --git a/src/com/android/settings/display/BrightnessLevelPreferenceControllerForSetupWizard.java b/src/com/android/settings/display/BrightnessLevelPreferenceControllerForSetupWizard.java index 197e4fe057d..da6ed7c602c 100644 --- a/src/com/android/settings/display/BrightnessLevelPreferenceControllerForSetupWizard.java +++ b/src/com/android/settings/display/BrightnessLevelPreferenceControllerForSetupWizard.java @@ -52,12 +52,11 @@ public class BrightnessLevelPreferenceControllerForSetupWizard extends @Override public void displayPreference(PreferenceScreen screen) { - super.displayPreference(screen); - Preference preference = screen.findPreference(getPreferenceKey()); + final Preference preference = screen.findPreference(getPreferenceKey()); if (preference instanceof RestrictedPreferenceHelperProvider helperProvider) { mRestrictedPreferenceHelper = helperProvider.getRestrictedPreferenceHelper(); - preference.setVisible(!isRestricted()); } + super.displayPreference(screen); } @Override @@ -68,4 +67,4 @@ public class BrightnessLevelPreferenceControllerForSetupWizard extends } return super.getAvailabilityStatus(); } -} +} \ No newline at end of file diff --git a/tests/robotests/src/com/android/settings/display/AutoBrightnessPreferenceControllerForSetupWizardTest.java b/tests/robotests/src/com/android/settings/display/AutoBrightnessPreferenceControllerForSetupWizardTest.java index 8f0b4a55a69..9419c186c06 100644 --- a/tests/robotests/src/com/android/settings/display/AutoBrightnessPreferenceControllerForSetupWizardTest.java +++ b/tests/robotests/src/com/android/settings/display/AutoBrightnessPreferenceControllerForSetupWizardTest.java @@ -77,6 +77,7 @@ public class AutoBrightnessPreferenceControllerForSetupWizardTest { public void displayPreference_flagOn_preferenceVisibleTrue() { Preference preference = displayPreference(/* configAvailable= */ true, /* restricted= */ false); + assertThat(preference.isVisible()).isTrue(); } @@ -85,6 +86,16 @@ public class AutoBrightnessPreferenceControllerForSetupWizardTest { public void displayPreference_flagOnAndRestricted_preferenceVisibleFalse() { Preference preference = displayPreference(/* configAvailable= */ true, /* restricted= */ true); + + assertThat(preference.isVisible()).isFalse(); + } + + @Test + @DisableFlags(Flags.FLAG_ADD_BRIGHTNESS_SETTINGS_IN_SUW) + public void displayPreference_flagOff_preferenceVisibleFalse() { + Preference preference = + displayPreference(/* configAvailable= */ true, /* restricted= */ false); + assertThat(preference.isVisible()).isFalse(); } @@ -92,6 +103,7 @@ public class AutoBrightnessPreferenceControllerForSetupWizardTest { @EnableFlags(Flags.FLAG_ADD_BRIGHTNESS_SETTINGS_IN_SUW) public void getAvailabilityStatus_configTrueAndFlagOn_availableUnsearchable() { displayPreference(/* configAvailable= */ true, /* restricted= */ false); + assertThat(mController.getAvailabilityStatus()).isEqualTo(AVAILABLE_UNSEARCHABLE); } @@ -99,6 +111,7 @@ public class AutoBrightnessPreferenceControllerForSetupWizardTest { @EnableFlags(Flags.FLAG_ADD_BRIGHTNESS_SETTINGS_IN_SUW) public void getAvailabilityStatus_configTrueAndFlagOnAndRestricted_conditionallyUnavailable() { displayPreference(/* configAvailable= */ true, /* restricted= */ true); + assertThat(mController.getAvailabilityStatus()).isEqualTo(CONDITIONALLY_UNAVAILABLE); } @@ -106,6 +119,7 @@ public class AutoBrightnessPreferenceControllerForSetupWizardTest { @EnableFlags(Flags.FLAG_ADD_BRIGHTNESS_SETTINGS_IN_SUW) public void getAvailabilityStatus_configFalseAndFlagOn_unsupportedOnDevice() { displayPreference(/* configAvailable= */ false, /* restricted= */ false); + assertThat(mController.getAvailabilityStatus()).isEqualTo(UNSUPPORTED_ON_DEVICE); } @@ -113,6 +127,7 @@ public class AutoBrightnessPreferenceControllerForSetupWizardTest { @EnableFlags(Flags.FLAG_ADD_BRIGHTNESS_SETTINGS_IN_SUW) public void getAvailabilityStatus_configFalseAndFlagOnAndRestricted_conditionallyUnavailable() { displayPreference(/* configAvailable= */ false, /* restricted= */ true); + assertThat(mController.getAvailabilityStatus()).isEqualTo(CONDITIONALLY_UNAVAILABLE); } @@ -120,6 +135,7 @@ public class AutoBrightnessPreferenceControllerForSetupWizardTest { @DisableFlags(Flags.FLAG_ADD_BRIGHTNESS_SETTINGS_IN_SUW) public void getAvailabilityStatus_flagOff_conditionallyUnavailable() { displayPreference(/* configAvailable= */ true, /* restricted= */ false); + assertThat(mController.getAvailabilityStatus()).isEqualTo(CONDITIONALLY_UNAVAILABLE); } diff --git a/tests/robotests/src/com/android/settings/display/BrightnessLevelPreferenceControllerForSetupWizardTest.java b/tests/robotests/src/com/android/settings/display/BrightnessLevelPreferenceControllerForSetupWizardTest.java index 6e44464a658..45e1959905d 100644 --- a/tests/robotests/src/com/android/settings/display/BrightnessLevelPreferenceControllerForSetupWizardTest.java +++ b/tests/robotests/src/com/android/settings/display/BrightnessLevelPreferenceControllerForSetupWizardTest.java @@ -68,6 +68,7 @@ public class BrightnessLevelPreferenceControllerForSetupWizardTest { @EnableFlags(Flags.FLAG_ADD_BRIGHTNESS_SETTINGS_IN_SUW) public void displayPreference_flagOn_preferenceVisibleTrue() { Preference preference = displayPreference(/* restricted= */ false); + assertThat(preference.isVisible()).isTrue(); } @@ -75,6 +76,15 @@ public class BrightnessLevelPreferenceControllerForSetupWizardTest { @EnableFlags(Flags.FLAG_ADD_BRIGHTNESS_SETTINGS_IN_SUW) public void displayPreference_flagOnAndRestricted_preferenceVisibleFalse() { Preference preference = displayPreference(/* restricted= */ true); + + assertThat(preference.isVisible()).isFalse(); + } + + @Test + @DisableFlags(Flags.FLAG_ADD_BRIGHTNESS_SETTINGS_IN_SUW) + public void displayPreference_flagOff_preferenceVisibleFalse() { + Preference preference = displayPreference(/* restricted= */ false); + assertThat(preference.isVisible()).isFalse(); } @@ -82,6 +92,7 @@ public class BrightnessLevelPreferenceControllerForSetupWizardTest { @EnableFlags(Flags.FLAG_ADD_BRIGHTNESS_SETTINGS_IN_SUW) public void getAvailabilityStatus_flagOn_available() { displayPreference(/* restricted= */ false); + assertThat(mController.getAvailabilityStatus()).isEqualTo(AVAILABLE); } @@ -89,6 +100,7 @@ public class BrightnessLevelPreferenceControllerForSetupWizardTest { @EnableFlags(Flags.FLAG_ADD_BRIGHTNESS_SETTINGS_IN_SUW) public void getAvailabilityStatus_flagOnAndRestricted_conditionallyUnavailable() { displayPreference(/* restricted= */ true); + assertThat(mController.getAvailabilityStatus()).isEqualTo(CONDITIONALLY_UNAVAILABLE); } @@ -96,6 +108,7 @@ public class BrightnessLevelPreferenceControllerForSetupWizardTest { @DisableFlags(Flags.FLAG_ADD_BRIGHTNESS_SETTINGS_IN_SUW) public void getAvailabilityStatus_flagOff_conditionallyUnavailable() { displayPreference(/* restricted= */ false); + assertThat(mController.getAvailabilityStatus()).isEqualTo(CONDITIONALLY_UNAVAILABLE); }