From d8135c15d0ff947e3d9c3c51730ed503c4a8e7db Mon Sep 17 00:00:00 2001
From: Shiran Pasternak <shiran.pasternak@microsoft.com>
Date: Wed, 7 Jun 2023 14:10:55 -0400
Subject: [PATCH] Fixes AB#786: Better chart accessibility

Removes some tab stops but adds a chart label summarizing the data displayed.

Also localizes some strings.
---
 desktop/i18n/resources.resjson                | 12 ++++++++++
 .../ui/metrics-monitor/metrics-monitor.scss   |  3 ---
 .../account-monitoring-section.html           |  3 ++-
 .../account-monitoring-section.scss           |  3 ---
 .../monitoring/account-monitoring.i18n.yml    | 24 ++++++++++++++-----
 .../monitor-chart/monitor-chart.component.ts  | 21 ++++++++++++----
 .../monitor-chart/monitor-chart.html          | 24 ++++++++++++-------
 7 files changed, 63 insertions(+), 27 deletions(-)

diff --git a/desktop/i18n/resources.resjson b/desktop/i18n/resources.resjson
index 64ac8c128..752689c75 100644
--- a/desktop/i18n/resources.resjson
+++ b/desktop/i18n/resources.resjson
@@ -10,12 +10,24 @@
   "account-details.monitoring": "Monitoring",
   "account-details.resources": "Resources",
   "account-monitoring.aggregatedOver": "Aggregated over {interval} interval",
+  "account-monitoring.aggregationButton": "Toggle aggreggation",
+  "account-monitoring.avg": "Average",
+  "account-monitoring.barChart": "Bar chart",
+  "account-monitoring.chartError": "An error occurred while loading the chart",
+  "account-monitoring.chartLabel": "{chartType} showing {name} with {numDataSets} data sets. {data}",
   "account-monitoring.coreCount": "Core minutes",
+  "account-monitoring.count": "Count",
   "account-monitoring.currentNodeStates": "Current node states",
+  "account-monitoring.doughnutChart": "Doughnut chart",
   "account-monitoring.expandLabel": "Expand chart",
   "account-monitoring.failedTask": "Failed task",
+  "account-monitoring.lineChart": "Line chart",
   "account-monitoring.nodeStates": "Node states",
   "account-monitoring.notARM": "Account monitoring is only enabled for ARM batch accounts",
+  "account-monitoring.pieChart": "Pie chart",
+  "account-monitoring.polarAreaChart": "Polar area chart",
+  "account-monitoring.radarChart": "Radar chart",
+  "account-monitoring.sum": "Sum",
   "account-monitoring.taskStates": "Task states",
   "account-summary-card.resourceGroup": "Resource group",
   "account-summary-card.subscription": "Subscription",
diff --git a/desktop/src/@batch-flask/ui/metrics-monitor/metrics-monitor.scss b/desktop/src/@batch-flask/ui/metrics-monitor/metrics-monitor.scss
index f7eb9d700..511210261 100644
--- a/desktop/src/@batch-flask/ui/metrics-monitor/metrics-monitor.scss
+++ b/desktop/src/@batch-flask/ui/metrics-monitor/metrics-monitor.scss
@@ -7,9 +7,6 @@ bl-metrics-monitor {
         > .tab-navigation-item {
             padding: 0;
         }
-        canvas {
-            pointer-events: none;
-        }
     }
 
     .preview {
diff --git a/desktop/src/app/components/account/details/account-monitoring-section/account-monitoring-section.html b/desktop/src/app/components/account/details/account-monitoring-section/account-monitoring-section.html
index d20a287f2..b36626d2e 100644
--- a/desktop/src/app/components/account/details/account-monitoring-section/account-monitoring-section.html
+++ b/desktop/src/app/components/account/details/account-monitoring-section/account-monitoring-section.html
@@ -18,7 +18,8 @@
 <div class="monitor-charts" *ngIf="isArmAccount">
     <a class="monitor-chart"
         [title]="'account-monitoring.expandLabel' | i18n"
-        [routerLink]="['/accounts', account.id, 'monitor']" *ngFor="let metricType of chartTypes;trackBy: trackMetric"
+        [routerLink]="['/accounts', account.id, 'monitor']"
+        *ngFor="let metricType of chartTypes;trackBy: trackMetric"
         role="application">
         <bl-monitor-chart [metrics]="metricType" [timeRange]="currentRange" [chartType]="chartType"></bl-monitor-chart>
     </a>
diff --git a/desktop/src/app/components/account/details/account-monitoring-section/account-monitoring-section.scss b/desktop/src/app/components/account/details/account-monitoring-section/account-monitoring-section.scss
index f1a2ca240..e78623e4a 100644
--- a/desktop/src/app/components/account/details/account-monitoring-section/account-monitoring-section.scss
+++ b/desktop/src/app/components/account/details/account-monitoring-section/account-monitoring-section.scss
@@ -12,7 +12,4 @@ bl-account-monitoring-section {
     bl-time-range-picker {
         margin-right: 10px;
     }
-    canvas {
-        pointer-events: none;
-    }
 }
diff --git a/desktop/src/app/components/account/monitoring/account-monitoring.i18n.yml b/desktop/src/app/components/account/monitoring/account-monitoring.i18n.yml
index 85589721c..84fa07afe 100644
--- a/desktop/src/app/components/account/monitoring/account-monitoring.i18n.yml
+++ b/desktop/src/app/components/account/monitoring/account-monitoring.i18n.yml
@@ -1,9 +1,21 @@
 account-monitoring:
-  coreCount: Core minutes
-  failedTask: Failed task
-  nodeStates: Node states
-  taskStates: Task states
-  currentNodeStates: Current node states
-  notARM: Account monitoring is only enabled for ARM batch accounts
   aggregatedOver: Aggregated over {interval} interval
+  aggregationButton: Toggle aggreggation
+  avg: Average
+  barChart: Bar chart
+  chartError: An error occurred while loading the chart
+  chartLabel: "{chartType} showing {name} with {numDataSets} data sets. {data}"
+  coreCount: Core minutes
+  count: Count
+  currentNodeStates: Current node states
+  doughnutChart: Doughnut chart
   expandLabel: Expand chart
+  failedTask: Failed task
+  lineChart: Line chart
+  nodeStates: Node states
+  notARM: Account monitoring is only enabled for ARM batch accounts
+  pieChart: Pie chart
+  polarAreaChart: Polar area chart
+  radarChart: Radar chart
+  sum: Sum
+  taskStates: Task states
diff --git a/desktop/src/app/components/account/monitoring/monitor-chart/monitor-chart.component.ts b/desktop/src/app/components/account/monitoring/monitor-chart/monitor-chart.component.ts
index 4385eff7c..0532947bb 100644
--- a/desktop/src/app/components/account/monitoring/monitor-chart/monitor-chart.component.ts
+++ b/desktop/src/app/components/account/monitoring/monitor-chart/monitor-chart.component.ts
@@ -1,7 +1,7 @@
 import {
     ChangeDetectionStrategy, ChangeDetectorRef, Component, Input, OnChanges, OnDestroy, SimpleChanges,
 } from "@angular/core";
-import { ServerError, isNotNullOrUndefined } from "@batch-flask/core";
+import { I18nService, ServerError, isNotNullOrUndefined } from "@batch-flask/core";
 import { ChartType, TimeRange } from "@batch-flask/ui";
 import { LoadingStatus } from "@batch-flask/ui/loading";
 import { log } from "@batch-flask/utils";
@@ -60,7 +60,9 @@ export class MonitorChartComponent implements OnChanges, OnDestroy {
         themeService: ThemeService,
         private accountService: BatchAccountService,
         private changeDetector: ChangeDetectorRef,
-        private metricsService: InsightsMetricsService) {
+        private metricsService: InsightsMetricsService,
+        private i18n: I18nService
+    ) {
         this._setChartOptions();
 
         const chartTheme = themeService.currentTheme.pipe(
@@ -148,6 +150,16 @@ export class MonitorChartComponent implements OnChanges, OnDestroy {
         return dataset.label;
     }
 
+    public chartLabel() {
+        const tallies = this.datasets.map((set, i) => `${set.label}: ${this.total[i]}`).join(", ");
+        return this.i18n.t("account-monitoring.chartLabel", {
+            name: this.i18n.t(`account-monitoring.${this.metrics}`),
+            chartType: this.i18n.t(`account-monitoring.${this.chartType}Chart`),
+            numDataSets: this.datasets.length,
+            data: tallies
+        });
+    }
+
     private _loadMetrics(metrics: MonitorChartType, timeRange: TimeRange): Observable<MonitoringMetricList> {
         switch (metrics) {
             case MonitorChartType.CoreCount:
@@ -199,9 +211,8 @@ export class MonitorChartComponent implements OnChanges, OnDestroy {
                 mode: "single",
                 position: "nearest",
                 callbacks: {
-                    title: (tooltipItems, data) => {
-                        return this._computeTooltipTitle(tooltipItems[0], data);
-                    },
+                    title: (tooltipItems, data) =>
+                        this._computeTooltipTitle(tooltipItems[0], data),
                 },
             },
             scales: {
diff --git a/desktop/src/app/components/account/monitoring/monitor-chart/monitor-chart.html b/desktop/src/app/components/account/monitoring/monitor-chart/monitor-chart.html
index 4a5b5921e..54a54222d 100644
--- a/desktop/src/app/components/account/monitoring/monitor-chart/monitor-chart.html
+++ b/desktop/src/app/components/account/monitoring/monitor-chart/monitor-chart.html
@@ -1,24 +1,24 @@
 <ng-container *ngIf="!preview">
     <div class="header">
-        <h2 tabindex="0" id="{{headerId}}">
+        <h2 id="{{headerId}}">
             {{ 'account-monitoring.' + metrics | i18n }}
         </h2>
-        <div class="aggregation" *ngIf="aggregation" tabindex="0">
-            {{aggregation}}
+        <div class="aggregation" *ngIf="aggregation" role="button"
+        attr.aria-label="{{ 'account-monitoring.aggregationButton' | i18n}}">
+            {{ 'account-monitoring.' + aggregation | i18n }}
         </div>
     </div>
     <ng-container *ngIf="isChartReady">
-        <div class="chart-container" tabindex="0" aria-label="graph">
+        <div class="chart-container" tabindex="0" [attr.aria-label]="chartLabel()">
             <canvas blChart
                 [datasets]="datasets"
                 [options]="options"
                 [chartType]="chartType"
-                [colors]="colors"
-                [attr.aria-labelledby]="headerId">
+                [colors]="colors">
             </canvas>
         </div>
         <div class="legend">
-            <div class="legend-item" *ngFor="let dataset of datasets;trackBy: trackDataSet;let index=index" tabindex="0">
+            <div class="legend-item" *ngFor="let dataset of datasets;trackBy: trackDataSet;let index=index">
                 <div class="value">
                     {{total[index]}}
                 </div>
@@ -32,12 +32,18 @@
         </div>
     </ng-container>
     <div *ngIf="chartError">
-        There is an error occurred when loading this chart
+        {{'account-monitoring.chartError' | i18n}}
         {{chartError.message}}
     </div>
     <bl-loading [status]="loadingStatus"></bl-loading>
 </ng-container>
 
 <div class="monitor-chart-preview" *ngIf="isChartReady && preview">
-    <canvas blChart [datasets]="datasets" [options]="options" [chartType]="chartType" [colors]="colors"></canvas>
+    <canvas blChart
+        [datasets]="datasets"
+        [options]="options"
+        [chartType]="chartType"
+        [colors]="colors"
+        [attr.aria-label]="chartLabel()">
+    </canvas>
 </div>