Diagnostics: Percent bar chart styling

Screenshot: http://shortn/_ZUvKuMuDuz

Spec: https://carbon.googleplex.com/cros-ux/pages/diagnostics/mvp#db1415fc-6806-4059-958e-459d8da17580
Bug: 1125150
Test: browser_tests --gtest_filter=DiagnosticsApp*
Change-Id: Iabf38a1d3e4600d9e9d29063d6629e0ed23e5818
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2567931
Commit-Queue: Michael Checo <michaelcheco@google.com>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Joon Ahn <joonbug@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842211}

chrome/test/data/webui/chromeos/diagnostics/BUILD.gn

@@ -18,6 +18,7 @@ js_type_check("closure_compile") {
     ":diagnostics_app_test",
     ":diagnostics_app_unified_test",
     ":diagnostics_test_utils",
+    ":diagnostics_utils_test",
     ":fake_method_provider_test",
     ":fake_observables_test",
     ":fake_system_data_provider_test",
@@ -105,6 +106,13 @@ js_library("diagnostics_test_utils") {
   ]
 }
 
+js_library("diagnostics_utils_test") {
+  deps = [
+    "../..:chai_assert",
+    "//chromeos/components/diagnostics_ui/resources:diagnostics_utils",
+  ]
+}
+
 js_library("fake_method_provider_test") {
   deps = [
     "../..:chai_assert",
@@ -141,6 +149,7 @@ js_library("memory_card_test") {
     "../..:chai_assert",
     "../..:test_util.m",
     "//chromeos/components/diagnostics_ui/resources:diagnostics_types",
+    "//chromeos/components/diagnostics_ui/resources:diagnostics_utils",
     "//chromeos/components/diagnostics_ui/resources:fake_data",
     "//chromeos/components/diagnostics_ui/resources:fake_system_data_provider",
     "//chromeos/components/diagnostics_ui/resources:memory_card",

chrome/test/data/webui/chromeos/diagnostics/diagnostics_app_unified_test.js

@@ -9,6 +9,7 @@ import {batteryStatusCardTestSuite} from './battery_status_card_test.js';
 import {cpuCardTestSuite} from './cpu_card_test.js';
 import {dataPointTestSuite} from './data_point_test.js';
 import {appTestSuite} from './diagnostics_app_test.js';
+import {diagnosticsUtilsTestSuite} from './diagnostics_utils_test.js';
 import {fakeMethodResolverTestSuite} from './fake_method_provider_test.js';
 import {fakeObservablesTestSuite} from './fake_observables_test.js';
 import {fakeSystemDataProviderTestSuite} from './fake_system_data_provider_test.js';
@@ -35,6 +36,7 @@ runSuite('App', appTestSuite);
 runSuite('BatteryStatusCard', batteryStatusCardTestSuite);
 runSuite('CpuCard', cpuCardTestSuite);
 runSuite('DataPoint', dataPointTestSuite);
+runSuite('DiagnosticsUtils', diagnosticsUtilsTestSuite);
 runSuite('FakeMethodProvider', fakeMethodResolverTestSuite);
 runSuite('FakeMojoInterface', fakeMojoProviderTestSuite);
 runSuite('FakeObservables', fakeObservablesTestSuite);

chrome/test/data/webui/chromeos/diagnostics/diagnostics_browsertest.js

@@ -44,11 +44,11 @@ this['DiagnosticsApp'] = class extends PolymerTest {
 // You must register all suites in unified test here as well for consistency,
 // although technically is not necessary.
 const debug_suites_list = [
-  'App', 'BatteryStatusCard', 'CpuCard', 'DataPoint', 'FakeMethodProvider',
-  'FakeMojoInterface', 'FakeObservables', 'FakeSystemDataProvider',
-  'FakeSystemRoutineContoller', 'MemoryCard', 'OverviewCard', 'PercentBarChart',
-  'RealtimeCpuChart', 'RoutineListExecutor', 'RoutineResultEntry',
-  'RoutineResultList', 'RoutineSection', 'TextBadge'
+  'App', 'BatteryStatusCard', 'CpuCard', 'DataPoint', 'DiagnosticsUtils',
+  'FakeMethodProvider', 'FakeMojoInterface', 'FakeObservables',
+  'FakeSystemDataProvider', 'FakeSystemRoutineContoller', 'MemoryCard',
+  'OverviewCard', 'PercentBarChart', 'RealtimeCpuChart', 'RoutineListExecutor',
+  'RoutineResultEntry', 'RoutineResultList', 'RoutineSection', 'TextBadge'
 ];
 
 TEST_F('DiagnosticsApp', 'BrowserTest', function() {

chrome/test/data/webui/chromeos/diagnostics/diagnostics_utils_test.js

+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import {convertKibToGibDecimalString} from 'chrome://diagnostics/diagnostics_utils.js';
+import {assertEquals} from '../../chai_assert.js';
+
+export function diagnosticsUtilsTestSuite() {
+  test('ProperlyConvertsKibToGib', () => {
+    assertEquals('0', convertKibToGibDecimalString(0, 0));
+    assertEquals('0.00', convertKibToGibDecimalString(0, 2));
+    assertEquals('0.000000', convertKibToGibDecimalString(0, 6));
+    assertEquals('0', convertKibToGibDecimalString(1, 0));
+    assertEquals('5.72', convertKibToGibDecimalString(6000000, 2));
+    assertEquals('5.722046', convertKibToGibDecimalString(6000000, 6));
+    assertEquals('1.00', convertKibToGibDecimalString(2 ** 20, 2));
+    assertEquals('1.00', convertKibToGibDecimalString(2 ** 20 + 1, 2));
+    assertEquals('1.00', convertKibToGibDecimalString(2 ** 20 - 1, 2));
+    assertEquals('0.999999', convertKibToGibDecimalString(2 ** 20 - 1, 6));
+  });
+}
\ No newline at end of file

chrome/test/data/webui/chromeos/diagnostics/memory_card_test.js

@@ -5,9 +5,11 @@
 import 'chrome://diagnostics/memory_card.js';
 
 import {MemoryUsage} from 'chrome://diagnostics/diagnostics_types.js';
+import {convertKibToGibDecimalString} from 'chrome://diagnostics/diagnostics_utils.js';
 import {fakeMemoryUsage} from 'chrome://diagnostics/fake_data.js';
 import {FakeSystemDataProvider} from 'chrome://diagnostics/fake_system_data_provider.js';
 import {setSystemDataProviderForTesting} from 'chrome://diagnostics/mojo_interface_provider.js';
+import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 
 import {assertEquals, assertFalse, assertTrue} from '../../chai_assert.js';
 import {flushTasks, isChildVisible} from '../../test_util.m.js';
@@ -91,8 +93,14 @@ export function memoryCardTestSuite() {
       const barChart = dx_utils.getPercentBarChartElement(memoryElement);
       const memInUse = fakeMemoryUsage[0].totalMemoryKib -
           fakeMemoryUsage[0].availableMemoryKib;
+      const expectedChartHeader = loadTimeData.getStringF(
+          'memoryAvailable',
+          convertKibToGibDecimalString(
+              fakeMemoryUsage[0].availableMemoryKib, 2),
+          convertKibToGibDecimalString(fakeMemoryUsage[0].totalMemoryKib, 2));
       assertEquals(fakeMemoryUsage[0].totalMemoryKib, barChart.max);
       assertEquals(memInUse, barChart.value);
+      dx_utils.assertTextContains(barChart.header, expectedChartHeader);
 
       // Verify the routine section is in the card.
       assertTrue(!!getRoutineSection());

chrome/test/data/webui/chromeos/diagnostics/percent_bar_chart_test.js

@@ -58,10 +58,6 @@ export function percentBarChartTestSuite() {
 
       assertEquals(
           header, percentBarChartElement.$$('#chartName').textContent.trim());
-      dx_utils.assertElementContainsText(
-          /** @type {!HTMLElement} */ (
-              percentBarChartElement.$$('#percentageLabel')),
-          `${percent}`);
 
       assertFalse(!!percentBarChartElement.$$('#headerIcon'));
     });

chromeos/chromeos_strings.grd

@@ -662,9 +662,6 @@ Try tapping the mic to ask me anything.
       <message name="IDS_DIAGNOSTICS_SESSION_LOG_LABEL" desc="The label for the button that opens the session log for the diagnostics app.">
         Save Session log
       </message>
-      <message name="IDS_DIAGNOSTICS_USED_MEMORY_LABEL" desc="The label for the progress bar that displays the amount of used memory on a users' device." translateable="false">
-        Used memory
-      </message>
       <message name="IDS_DIAGNOSTICS_BATTERY_CHIP_TEXT" desc="The text for a battery's full charge capacity represented in milliamp hours.">
         <ph name="CHARGE_VALUE">$1<ex>6000</ex></ph>mAh Battery
       </message>
@@ -782,8 +779,8 @@ Try tapping the mic to ask me anything.
       <message name="IDS_DIAGNOSTICS_CPU_SPEED_TEXT" desc="The text that displays the CPU speed. GHz is an SI unit for gigahertz.">
         <ph name="CURRENT">$1<ex>1.6</ex></ph>GHz / <ph name="MAX">$2<ex>1.9</ex></ph>GHz
       </message>
-      <message name="IDS_DIAGNOSTICS_TOTAL_MEMORY_LABEL" desc="The label for the total amount of memory on a device." translateable="false">
-        Total Memory
+      <message name="IDS_DIAGNOSTICS_MEMORY_AVAILABLE_TEXT" desc="The text that displays the amount of memory available on a users' device.">
+        <ph name="AVAILABLE_MEMORY">$1<ex>13.12</ex></ph> GB of <ph name="TOTAL_MEMORY">$2<ex>16</ex></ph> GB available.
       </message>
 
       <!-- Quick Answers -->

chromeos/chromeos_strings_grd/IDS_DIAGNOSTICS_MEMORY_AVAILABLE_TEXT.png.sha1

+d9ac09bb04b99206d904d1aa223aff0cdda9e55e
\ No newline at end of file

chromeos/components/diagnostics_ui/diagnostics_ui.cc

@@ -70,6 +70,7 @@ void AddDiagnosticsStrings(content::WebUIDataSource* html_source) {
       {"hideReportText", IDS_DIAGNOSTICS_HIDE_REPORT_TEXT},
       {"learnMore", IDS_DIANOSTICS_LEARN_MORE_LABEL},
       {"learnMoreShort", IDS_DIAGNOSTICS_LEARN_MORE_LABEL_SHORT},
+      {"memoryAvailable", IDS_DIAGNOSTICS_MEMORY_AVAILABLE_TEXT},
       {"memoryRoutineText", IDS_DIAGNOSTICS_MEMORY_ROUTINE_TEXT},
       {"memoryTitle", IDS_DIAGNOSTICS_MEMORY_TITLE},
       {"percentageLabel", IDS_DIAGNOSTICS_PERCENTAGE_LABEL},
@@ -92,8 +93,6 @@ void AddDiagnosticsStrings(content::WebUIDataSource* html_source) {
       {"testRunningBadgeText", IDS_DIAGNOSTICS_TEST_RUNNING_BADGE_TEXT},
       {"testSuccess", IDS_DIAGNOSTICS_TEST_SUCCESS_TEXT},
       {"testSucceededBadgeText", IDS_DIAGNOSTICS_TEST_SUCCESS_BADGE_TEXT},
-      {"totalMemory", IDS_DIAGNOSTICS_TOTAL_MEMORY_LABEL},
-      {"usedMemory", IDS_DIAGNOSTICS_USED_MEMORY_LABEL},
   };
   for (const auto& str : kLocalizedStrings) {
     html_source->AddLocalizedString(str.name, str.id);

chromeos/components/diagnostics_ui/resources/BUILD.gn

@@ -16,6 +16,7 @@ js_type_check("closure_compile_module") {
     ":diagnostics_app",
     ":diagnostics_card",
     ":diagnostics_types",
+    ":diagnostics_utils",
     ":fake_data",
     ":fake_method_resolver",
     ":fake_observables",
@@ -89,6 +90,9 @@ js_library("diagnostics_card") {
 js_library("diagnostics_types") {
 }
 
+js_library("diagnostics_utils") {
+}
+
 js_library("fake_data") {
   deps = [ ":diagnostics_types" ]
 }
@@ -112,9 +116,11 @@ js_library("fake_system_routine_controller") {
 js_library("memory_card") {
   deps = [
     ":data_point",
+    ":diagnostics_utils",
     ":mojo_interface_provider",
     "//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
     "//ui/webui/resources/js:i18n_behavior.m",
+    "//ui/webui/resources/js:load_time_data.m",
   ]
 }
 

chromeos/components/diagnostics_ui/resources/battery_status_card.html

@@ -7,7 +7,7 @@
   </div>
   <!-- TODO(michaelcheco): Replace placeholder icon. -->
   <iron-icon slot="icon" icon="cr:add"></iron-icon>
-  <percent-bar-chart slot="left-panel" header="[[i18n('remainingCharge')]]"
+  <percent-bar-chart slot="left-panel" header="[[powerTimeString_]]"
     value="[[batteryChargeStatus_.chargeNowMilliampHours]]"
     max="[[batteryHealth_.chargeFullNowMilliampHours]]">
   </percent-bar-chart>

chromeos/components/diagnostics_ui/resources/diagnostics_app_resources.grd

@@ -41,6 +41,7 @@
       <include name="IDR_DIAGNOSTICS_SYSTEM_ROUTINE_CONTROLLER_MOJO_LITE_JS" file="${root_gen_dir}/chromeos/components/diagnostics_ui/mojom/system_routine_controller.mojom-lite.js" use_base_dir="false" type="BINDATA" />
       <include name="IDR_DIAGNOSTICS_TEXT_BADGE_JS" file="${root_gen_dir}/chromeos/components/diagnostics_ui/resources/text_badge.js" use_base_dir="false" type="BINDATA"/>
       <include name="IDR_DIAGNOSTICS_TYPES_JS" file="diagnostics_types.js" type="BINDATA"/>
+      <include name="IDR_DIAGNOSTICS_UTILS_JS" file="diagnostics_utils.js" type="BINDATA"/>
       <include name="IDR_D3_SRC_D3_MIN_JS" file="../../../../third_party/d3/src/d3.min.js" type="BINDATA" />
     </includes>
   </release>

chromeos/components/diagnostics_ui/resources/diagnostics_card.html

@@ -6,16 +6,21 @@
     padding: 4px 8px;
   }
   ::slotted([slot=icon]) {
-    background-color: rgba(var(--google-blue-refresh-500-rgb), .12);
+    background-color: var(--google-blue-50);
     border-radius: 50%;
     color: var(--google-blue-600);
+    height: 26px;
     left: 5px;
+    margin-left: 20px;
+    margin-right: 28px;
     padding: 2px;
     position: relative;
+    width: 20px;
   }
 
   ::slotted([slot=left-panel]) {
     flex: 1;
+    margin-bottom: 16px;
   }
 
   .card-container {
@@ -44,10 +49,6 @@
     padding-inline: var(--data-point-container-padding);
   }
 
-  .icon {
-    width: 10%;
-  }
-
   .routine-container {
     @apply --diagnostics-default-font;
   }
@@ -61,11 +62,7 @@
   .top-section {
     border-bottom: 1px solid var(--diagnostics-border-color);
     display: flex;
-    margin-bottom: 6px;
-    margin-top: 12px;
-    padding-bottom: 12px;
-    padding-inline-end: 15px;
-    padding-inline-start: 15px;
+    margin-top: 16px;
   }
 </style>
 <div class="card-container">
@@ -75,9 +72,7 @@
   </div>
   <div class="card-wrapper">
     <div class="top-section">
-      <div class="icon">
       <slot name="icon"></slot>
-      </div>
       <slot name="left-panel"></slot>
     </div>
     <div id="body" class="data-points" hidden$="[[hideDataPoints]]">

chromeos/components/diagnostics_ui/resources/diagnostics_utils.js

+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+/**
+ * Converts a KiB storage value to GiB and returns a fixed-point string
+ * to the desired number of decimal places.
+ * @param {number} value
+ * @param {number} numDecimalPlaces
+ * @return {string}
+ */
+export function convertKibToGibDecimalString(value, numDecimalPlaces) {
+  return (value / 2 ** 20).toFixed(numDecimalPlaces);
+}

chromeos/components/diagnostics_ui/resources/memory_card.html

@@ -4,7 +4,8 @@
 <diagnostics-card hide-data-points="true">
   <div id="cardTitle" slot="title">[[i18n('memoryTitle')]]</div>
   <iron-icon slot="icon" icon="cr:add"></iron-icon>
-  <percent-bar-chart slot="left-panel" header="[[i18n('usedMemory')]]"
+  <percent-bar-chart slot="left-panel"
+    header="[[getAvailableMemory_(memoryUsage_)]]"
     value="[[getTotalUsedMemory_(memoryUsage_)]]"
     max="[[memoryUsage_.totalMemoryKib]]">
   </percent-bar-chart>

chromeos/components/diagnostics_ui/resources/memory_card.js

@@ -12,9 +12,11 @@ import './routine_section.js';
 import './strings.m.js';
 
 import {I18nBehavior} from 'chrome://resources/js/i18n_behavior.m.js';
+import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 import {html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
 import {MemoryUsage, RoutineType, SystemDataProviderInterface} from './diagnostics_types.js'
+import {convertKibToGibDecimalString} from './diagnostics_utils.js';
 import {getSystemDataProvider} from './mojo_interface_provider.js';
 
 /**
@@ -103,5 +105,19 @@ Polymer({
    */
   getTotalUsedMemory_(memoryUsage) {
     return memoryUsage.totalMemoryKib - memoryUsage.availableMemoryKib;
-  }
+  },
+
+  /**
+   * Calculates total available memory from MemoryUsage object.
+   * @return {string}
+   * @protected
+   */
+  getAvailableMemory_() {
+    // Note: The storage value is converted to GiB but we still display "GB" to
+    // the user since this is the convention memory manufacturers use.
+    return loadTimeData.getStringF(
+        'memoryAvailable',
+        convertKibToGibDecimalString(this.memoryUsage_.availableMemoryKib, 2),
+        convertKibToGibDecimalString(this.memoryUsage_.totalMemoryKib, 2));
+  },
 });

chromeos/components/diagnostics_ui/resources/percent_bar_chart.html

 <style include="diagnostics-shared diagnostics-fonts">
-  #barChartContainer > label {
+  #chartName {
     @apply --diagnostics-chart-title-font;
-    color: var(--diagnostics-overview-text-color);
+    color: var(--google-grey-700);
+    font-family: Roboto;
+    font-size: 13px;
+    line-height: 20px;
+    margin-top: 8px;
   }
 
   #headerIcon {
@@ -18,28 +22,20 @@
     width: 85%;
     --paper-progress-active-color: var(--google-blue-500);
     --paper-progress-container-color: var(--google-blue-100);
-    --paper-progress-height: 8px;
+    --paper-progress-height: 4px;
   }
-
-  #percentageLabel {
-    @apply --diagnostics-chart-label-font;
-    display: inline;
-    padding-left: 5px;
-  }
-
 </style>
 
 <div id="barChartContainer">
+  <div class="body">
+    <paper-progress id="barChart"
+      value="[[getAdjustedValue_(value, max)]]" max="[[max]]">
+    </paper-progress>
+  </div>
   <label id="chartName">
     <template is="dom-if" if="[[headerIcon.length]]">
       <iron-icon id="headerIcon" icon="[[headerIcon]]"></iron-icon>
     </template>
     [[header]]
   </label>
-  <div class="body">
-    <paper-progress id="barChart"
-      value="[[getAdjustedValue_(value, max)]]" max="[[max]]">
-    </paper-progress>
-    <label id="percentageLabel">[[computePercentage_(value, max)]]</label>
-  </div>
 </div>

chromeos/components/diagnostics_ui/resources/percent_bar_chart.js

@@ -47,19 +47,6 @@ Polymer({
     },
   },
 
-  /**
-   * Returns the percentage of the current bar chart, rounded to the nearest
-   * whole number.
-   * @param {number} currentValue
-   * @param {number} maxValue
-   * @return {string} i18n string for the percentage value.
-   * @private
-   */
-  computePercentage_(currentValue, maxValue) {
-    return loadTimeData.getStringF(
-        'percentageLabel', Math.round(100 * currentValue / maxValue));
-  },
-
   /**
    * Get adjusted value clamped to max value. paper-progress breaks for a while
    * when value is set higher than max in certain cases (e.g. due to fetching of