From 29eb2d12768cbb165cede10e864c57dacd04b060 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Fri, 11 Nov 2022 17:00:47 +0000 Subject: [PATCH] [Battery] Ensure we always update the text and content description when necessary. When writing some tests for the new dock defend icon, I came across some existing issues when we don't correctly update the text or content description. This CL adds tests that would've been failing, and updates BatteryMeterView to make them pass. Bug: 255625888 Test: atest BatteryMeterViewTest Change-Id: I3485a50c6471dd75dce64a3daf414208afa8d25e --- packages/SystemUI/res/values/strings.xml | 2 +- .../systemui/battery/BatteryMeterView.java | 54 ++++++++++----- .../systemui/battery/BatteryMeterViewTest.kt | 66 +++++++++++++++++++ 3 files changed, 103 insertions(+), 19 deletions(-) diff --git a/packages/SystemUI/res/values/strings.xml b/packages/SystemUI/res/values/strings.xml index dc84435f40818..53cd059288e31 100644 --- a/packages/SystemUI/res/values/strings.xml +++ b/packages/SystemUI/res/values/strings.xml @@ -439,7 +439,7 @@ Battery %d percent. - Battery %1$s percent, about %2$s left based on your usage + Battery %1$d percent, about %2$s left based on your usage Battery charging, %d percent. diff --git a/packages/SystemUI/src/com/android/systemui/battery/BatteryMeterView.java b/packages/SystemUI/src/com/android/systemui/battery/BatteryMeterView.java index 4c16566fff8fc..b918655dbbb6c 100644 --- a/packages/SystemUI/src/com/android/systemui/battery/BatteryMeterView.java +++ b/packages/SystemUI/src/com/android/systemui/battery/BatteryMeterView.java @@ -76,6 +76,7 @@ public class BatteryMeterView extends LinearLayout implements DarkReceiver { private int mLevel; private int mShowPercentMode = MODE_DEFAULT; private boolean mShowPercentAvailable; + private String mEstimateText = null; private boolean mCharging; private boolean mDisplayShield; private boolean mDisplayShieldEnabled; @@ -171,6 +172,7 @@ public class BatteryMeterView extends LinearLayout implements DarkReceiver { if (mode == mShowPercentMode) return; mShowPercentMode = mode; updateShowPercent(); + updatePercentText(); } @Override @@ -247,11 +249,11 @@ public class BatteryMeterView extends LinearLayout implements DarkReceiver { void updatePercentText() { if (mBatteryStateUnknown) { - setContentDescription(getContext().getString(R.string.accessibility_battery_unknown)); return; } if (mBatteryEstimateFetcher == null) { + setPercentTextAtCurrentLevel(); return; } @@ -263,10 +265,9 @@ public class BatteryMeterView extends LinearLayout implements DarkReceiver { return; } if (estimate != null && mShowPercentMode == MODE_ESTIMATE) { + mEstimateText = estimate; mBatteryPercentView.setText(estimate); - setContentDescription(getContext().getString( - R.string.accessibility_battery_level_with_estimate, - mLevel, estimate)); + updateContentDescription(); } else { setPercentTextAtCurrentLevel(); } @@ -275,28 +276,44 @@ public class BatteryMeterView extends LinearLayout implements DarkReceiver { setPercentTextAtCurrentLevel(); } } else { - setContentDescription( - getContext().getString(mCharging ? R.string.accessibility_battery_level_charging - : R.string.accessibility_battery_level, mLevel)); + updateContentDescription(); } } private void setPercentTextAtCurrentLevel() { - if (mBatteryPercentView == null) { - return; + if (mBatteryPercentView != null) { + mEstimateText = null; + String percentText = NumberFormat.getPercentInstance().format(mLevel / 100f); + // Setting text actually triggers a layout pass (because the text view is set to + // wrap_content width and TextView always relayouts for this). Avoid needless + // relayout if the text didn't actually change. + if (!TextUtils.equals(mBatteryPercentView.getText(), percentText)) { + mBatteryPercentView.setText(percentText); + } } - String percentText = NumberFormat.getPercentInstance().format(mLevel / 100f); - // Setting text actually triggers a layout pass (because the text view is set to - // wrap_content width and TextView always relayouts for this). Avoid needless - // relayout if the text didn't actually change. - if (!TextUtils.equals(mBatteryPercentView.getText(), percentText)) { - mBatteryPercentView.setText(percentText); + updateContentDescription(); + } + + private void updateContentDescription() { + Context context = getContext(); + + String contentDescription; + if (mBatteryStateUnknown) { + contentDescription = context.getString(R.string.accessibility_battery_unknown); + } else if (mShowPercentMode == MODE_ESTIMATE && !TextUtils.isEmpty(mEstimateText)) { + contentDescription = context.getString( + R.string.accessibility_battery_level_with_estimate, + mLevel, + mEstimateText); + } else if (mCharging) { + contentDescription = + context.getString(R.string.accessibility_battery_level_charging, mLevel); + } else { + contentDescription = context.getString(R.string.accessibility_battery_level, mLevel); } - setContentDescription( - getContext().getString(mCharging ? R.string.accessibility_battery_level_charging - : R.string.accessibility_battery_level, mLevel)); + setContentDescription(contentDescription); } void updateShowPercent() { @@ -347,6 +364,7 @@ public class BatteryMeterView extends LinearLayout implements DarkReceiver { } mBatteryStateUnknown = isUnknown; + updateContentDescription(); if (mBatteryStateUnknown) { mBatteryIconView.setImageDrawable(getUnknownStateDrawable()); diff --git a/packages/SystemUI/tests/src/com/android/systemui/battery/BatteryMeterViewTest.kt b/packages/SystemUI/tests/src/com/android/systemui/battery/BatteryMeterViewTest.kt index 851b5408a4e00..b38d0b787dbc8 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/battery/BatteryMeterViewTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/battery/BatteryMeterViewTest.kt @@ -19,6 +19,7 @@ import android.testing.AndroidTestingRunner import android.testing.TestableLooper.RunWithLooper import android.widget.ImageView import androidx.test.filters.SmallTest +import com.android.systemui.R import com.android.systemui.SysuiTestCase import com.android.systemui.battery.BatteryMeterView.BatteryEstimateFetcher import com.android.systemui.statusbar.policy.BatteryController.EstimateFetchCompletion @@ -59,6 +60,71 @@ class BatteryMeterViewTest : SysuiTestCase() { // No assert needed } + @Test + fun contentDescription_unknown() { + mBatteryMeterView.onBatteryUnknownStateChanged(true) + + assertThat(mBatteryMeterView.contentDescription).isEqualTo( + context.getString(R.string.accessibility_battery_unknown) + ) + } + + @Test + fun contentDescription_estimate() { + mBatteryMeterView.onBatteryLevelChanged(15, false) + mBatteryMeterView.setPercentShowMode(BatteryMeterView.MODE_ESTIMATE) + mBatteryMeterView.setBatteryEstimateFetcher(Fetcher()) + + mBatteryMeterView.updatePercentText() + + assertThat(mBatteryMeterView.contentDescription).isEqualTo( + context.getString( + R.string.accessibility_battery_level_with_estimate, 15, ESTIMATE + ) + ) + } + + @Test + fun contentDescription_charging() { + mBatteryMeterView.onBatteryLevelChanged(45, true) + + assertThat(mBatteryMeterView.contentDescription).isEqualTo( + context.getString(R.string.accessibility_battery_level_charging, 45) + ) + } + + @Test + fun contentDescription_notCharging() { + mBatteryMeterView.onBatteryLevelChanged(45, false) + + assertThat(mBatteryMeterView.contentDescription).isEqualTo( + context.getString(R.string.accessibility_battery_level, 45) + ) + } + + @Test + fun changesFromEstimateToPercent_textAndContentDescriptionChanges() { + mBatteryMeterView.onBatteryLevelChanged(15, false) + mBatteryMeterView.setPercentShowMode(BatteryMeterView.MODE_ESTIMATE) + mBatteryMeterView.setBatteryEstimateFetcher(Fetcher()) + + mBatteryMeterView.updatePercentText() + + assertThat(mBatteryMeterView.contentDescription).isEqualTo( + context.getString( + R.string.accessibility_battery_level_with_estimate, 15, ESTIMATE + ) + ) + + // Update the show mode from estimate to percent + mBatteryMeterView.setPercentShowMode(BatteryMeterView.MODE_ON) + + assertThat(mBatteryMeterView.batteryPercentViewText).isEqualTo("15%") + assertThat(mBatteryMeterView.contentDescription).isEqualTo( + context.getString(R.string.accessibility_battery_level, 15) + ) + } + @Test fun isOverheatedChanged_true_drawableGetsTrue() { mBatteryMeterView.setDisplayShieldEnabled(true)