From d4f9588a3dc92178b72e8d9f3a636bf356d6fb82 Mon Sep 17 00:00:00 2001 From: Zaiyue Xue Date: Tue, 19 Jul 2022 10:40:45 +0800 Subject: [PATCH] Refactor Battery Chart View State Controll When users click the battery chart, the orignal behavior is that the view changes the state by itself. This cl refactors the bahavior to that the view callbacks to the controller, and the controller changes the view's state. In this way, the controller is the only source of truth of the state. This meets the controller-view model. Test: manual Bug: 239491373, 236101166 Change-Id: I754ded2dba20319f1571374dfdbef27f2420ed78 --- .../BatteryChartPreferenceControllerV2.java | 1 + .../batteryusage/BatteryChartViewV2.java | 21 +++--- .../batteryusage/BatteryChartViewV2Test.java | 65 +++++++++---------- 3 files changed, 42 insertions(+), 45 deletions(-) diff --git a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerV2.java b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerV2.java index dc80b84b06e..d5491018ba9 100644 --- a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerV2.java +++ b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerV2.java @@ -347,6 +347,7 @@ public class BatteryChartPreferenceControllerV2 extends AbstractPreferenceContro trapezoidIndex, mBatteryIndexedMap.size(), isForce)); mTrapezoidIndex = trapezoidIndex; + mBatteryChartView.setSelectedIndex(mTrapezoidIndex); mHandler.post(() -> { final long start = System.currentTimeMillis(); removeAndCacheAllPrefs(); diff --git a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewV2.java b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewV2.java index 5db5d52c6d9..15a3ca6f69f 100644 --- a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewV2.java +++ b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewV2.java @@ -110,9 +110,11 @@ public class BatteryChartViewV2 extends AppCompatImageView implements View.OnCli @VisibleForTesting Paint mTrapezoidCurvePaint = null; - private TrapezoidSlot[] mTrapezoidSlots; + @VisibleForTesting + TrapezoidSlot[] mTrapezoidSlots; // Records the location to calculate selected index. - private float mTouchUpEventX = Float.MIN_VALUE; + @VisibleForTesting + float mTouchUpEventX = Float.MIN_VALUE; private BatteryChartViewV2.OnSelectListener mOnSelectListener; public BatteryChartViewV2(Context context) { @@ -161,10 +163,6 @@ public class BatteryChartViewV2 extends AppCompatImageView implements View.OnCli if (mSelectedIndex != index) { mSelectedIndex = index; invalidate(); - // Callbacks to the listener if we have. - if (mOnSelectListener != null) { - mOnSelectListener.onSelect(mSelectedIndex); - } } } @@ -301,11 +299,9 @@ public class BatteryChartViewV2 extends AppCompatImageView implements View.OnCli || !isValidToDraw(trapezoidIndex)) { return; } - // Selects all if users click the same trapezoid item two times. - if (trapezoidIndex == mSelectedIndex) { - setSelectedIndex(SELECTED_INDEX_ALL); - } else { - setSelectedIndex(trapezoidIndex); + if (mOnSelectListener != null) { + mOnSelectListener.onSelect( + trapezoidIndex == mSelectedIndex ? SELECTED_INDEX_ALL : trapezoidIndex); } view.performHapticFeedback(HapticFeedbackConstants.CONTEXT_CLICK); } @@ -614,7 +610,8 @@ public class BatteryChartViewV2 extends AppCompatImageView implements View.OnCli } // A container class for each trapezoid left and right location. - private static final class TrapezoidSlot { + @VisibleForTesting + static final class TrapezoidSlot { public float mLeft; public float mRight; diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewV2Test.java b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewV2Test.java index 2879cf6989f..34ba2ebeeb2 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewV2Test.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartViewV2Test.java @@ -26,6 +26,7 @@ import static org.mockito.Mockito.when; import android.accessibilityservice.AccessibilityServiceInfo; import android.content.Context; import android.os.LocaleList; +import android.view.View; import android.view.accessibility.AccessibilityManager; import com.android.settings.fuelgauge.PowerUsageFeatureProvider; @@ -55,6 +56,8 @@ public final class BatteryChartViewV2Test { private AccessibilityServiceInfo mMockAccessibilityServiceInfo; @Mock private AccessibilityManager mMockAccessibilityManager; + @Mock + private View mMockView; @Before public void setUp() { @@ -74,13 +77,13 @@ public final class BatteryChartViewV2Test { } @Test - public void testIsAccessibilityEnabled_disable_returnFalse() { + public void isAccessibilityEnabled_disable_returnFalse() { doReturn(false).when(mMockAccessibilityManager).isEnabled(); assertThat(BatteryChartViewV2.isAccessibilityEnabled(mContext)).isFalse(); } @Test - public void testIsAccessibilityEnabled_emptyInfo_returnFalse() { + public void isAccessibilityEnabled_emptyInfo_returnFalse() { doReturn(true).when(mMockAccessibilityManager).isEnabled(); doReturn(new ArrayList()) .when(mMockAccessibilityManager) @@ -90,45 +93,41 @@ public final class BatteryChartViewV2Test { } @Test - public void testIsAccessibilityEnabled_validServiceId_returnTrue() { + public void isAccessibilityEnabled_validServiceId_returnTrue() { doReturn(true).when(mMockAccessibilityManager).isEnabled(); assertThat(BatteryChartViewV2.isAccessibilityEnabled(mContext)).isTrue(); } @Test - public void testSetSelectedIndex_invokesCallback() { + public void onClick_invokesCallback() { + mBatteryChartView.setLevels(new int[] {90, 80, 70, 60}); + for (int i = 0; i < mBatteryChartView.mTrapezoidSlots.length; i++) { + mBatteryChartView.mTrapezoidSlots[i] = new BatteryChartViewV2.TrapezoidSlot(); + mBatteryChartView.mTrapezoidSlots[i].mLeft = i; + mBatteryChartView.mTrapezoidSlots[i].mRight = i + 0.5f; + } + mBatteryChartView.mSelectedIndex = 2; final int[] selectedIndex = new int[1]; - final int expectedIndex = 2; - mBatteryChartView.mSelectedIndex = 1; mBatteryChartView.setOnSelectListener( trapezoidIndex -> { selectedIndex[0] = trapezoidIndex; }); - mBatteryChartView.setSelectedIndex(expectedIndex); + // Verify onClick() a different index 1. + mBatteryChartView.mTouchUpEventX = 1; + selectedIndex[0] = Integer.MIN_VALUE; + mBatteryChartView.onClick(mMockView); + assertThat(selectedIndex[0]).isEqualTo(1); - assertThat(mBatteryChartView.mSelectedIndex) - .isEqualTo(expectedIndex); - assertThat(selectedIndex[0]).isEqualTo(expectedIndex); + // Verify onClick() the same index 2. + mBatteryChartView.mTouchUpEventX = 2; + selectedIndex[0] = Integer.MIN_VALUE; + mBatteryChartView.onClick(mMockView); + assertThat(selectedIndex[0]).isEqualTo(BatteryChartViewV2.SELECTED_INDEX_ALL); } @Test - public void testSetSelectedIndex_sameIndex_notInvokesCallback() { - final int[] selectedIndex = new int[1]; - final int expectedIndex = 1; - mBatteryChartView.mSelectedIndex = expectedIndex; - mBatteryChartView.setOnSelectListener( - trapezoidIndex -> { - selectedIndex[0] = trapezoidIndex; - }); - - mBatteryChartView.setSelectedIndex(expectedIndex); - - assertThat(selectedIndex[0]).isNotEqualTo(expectedIndex); - } - - @Test - public void testClickable_isChartGraphSlotsEnabledIsFalse_notClickable() { + public void clickable_isChartGraphSlotsEnabledIsFalse_notClickable() { mBatteryChartView.setClickableForce(true); when(mPowerUsageFeatureProvider.isChartGraphSlotsEnabled(mContext)) .thenReturn(false); @@ -139,7 +138,7 @@ public final class BatteryChartViewV2Test { } @Test - public void testClickable_accessibilityIsDisabled_clickable() { + public void clickable_accessibilityIsDisabled_clickable() { mBatteryChartView.setClickableForce(true); when(mPowerUsageFeatureProvider.isChartGraphSlotsEnabled(mContext)) .thenReturn(true); @@ -151,7 +150,7 @@ public final class BatteryChartViewV2Test { } @Test - public void testClickable_accessibilityIsEnabledWithoutValidId_clickable() { + public void clickable_accessibilityIsEnabledWithoutValidId_clickable() { mBatteryChartView.setClickableForce(true); when(mPowerUsageFeatureProvider.isChartGraphSlotsEnabled(mContext)) .thenReturn(true); @@ -166,7 +165,7 @@ public final class BatteryChartViewV2Test { } @Test - public void testClickable_accessibilityIsEnabledWithValidId_notClickable() { + public void clickable_accessibilityIsEnabledWithValidId_notClickable() { mBatteryChartView.setClickableForce(true); when(mPowerUsageFeatureProvider.isChartGraphSlotsEnabled(mContext)) .thenReturn(true); @@ -178,7 +177,7 @@ public final class BatteryChartViewV2Test { } @Test - public void testClickable_restoreFromNonClickableState() { + public void clickable_restoreFromNonClickableState() { final int[] levels = new int[13]; for (int index = 0; index < levels.length; index++) { levels[index] = index + 1; @@ -200,14 +199,14 @@ public final class BatteryChartViewV2Test { } @Test - public void testOnAttachedToWindow_addAccessibilityStateChangeListener() { + public void onAttachedToWindow_addAccessibilityStateChangeListener() { mBatteryChartView.onAttachedToWindow(); verify(mMockAccessibilityManager) .addAccessibilityStateChangeListener(mBatteryChartView); } @Test - public void testOnDetachedFromWindow_removeAccessibilityStateChangeListener() { + public void onDetachedFromWindow_removeAccessibilityStateChangeListener() { mBatteryChartView.onAttachedToWindow(); mBatteryChartView.mHandler.postDelayed( mBatteryChartView.mUpdateClickableStateRun, 1000); @@ -222,7 +221,7 @@ public final class BatteryChartViewV2Test { } @Test - public void testOnAccessibilityStateChanged_postUpdateStateRunnable() { + public void onAccessibilityStateChanged_postUpdateStateRunnable() { mBatteryChartView.mHandler = spy(mBatteryChartView.mHandler); mBatteryChartView.onAccessibilityStateChanged(/*enabled=*/ true);