From 4873815699383f8d051a75196f968c456807d7a2 Mon Sep 17 00:00:00 2001
From: Alexandre Vryghem <alexandre@atmire.com>
Date: Sun, 26 Nov 2023 23:53:44 +0100
Subject: [PATCH] Fix Health Panel usability issues

- Fixed aria-controls on HealthComponentComponent, HealthInfoComponent, HealthInfoComponentComponent, HealthPanelComponent pointing to non-existing id
- Fixed the tabs not having the role tab on its tabs
- Fixed aria-expanded being set to true when collapsed and backwards for HealthPanelComponent & HealthInfoComponent
- Fixed minor alignment issues
---
 .../health-info-component.component.html      |  8 +++----
 .../health-info/health-info.component.html    | 15 ++++++-------
 .../health-page/health-page.component.html    |  6 +++---
 src/app/health-page/health-page.component.ts  |  3 +++
 .../health-component.component.html           |  8 +++----
 .../health-panel/health-panel.component.html  | 21 +++++++++++--------
 .../health-status.component.html              |  6 +++---
 .../health-status.component.scss              |  3 +++
 8 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/src/app/health-page/health-info/health-info-component/health-info-component.component.html b/src/app/health-page/health-info/health-info-component/health-info-component.component.html
index dbaaa7a6b6e..84cba4d92f3 100644
--- a/src/app/health-page/health-info/health-info-component/health-info-component.component.html
+++ b/src/app/health-page/health-info/health-info-component/health-info-component.component.html
@@ -1,16 +1,16 @@
 <div *ngFor="let entry of healthInfoComponent | dsObjNgFor" data-test="collapse">
   <div *ngIf="entry && !isPlainProperty(entry.value)" class="mb-3 border-bottom">
-    <div class="w-100 d-flex justify-content-between collapse-toggle" (click)="collapse.toggle()">
+    <div class="w-100 d-flex gap-3 justify-content-between collapse-toggle" (click)="collapse.toggle()">
       <button type="button" class="btn btn-link p-0" (click)="$event.preventDefault()" [attr.aria-expanded]="!collapse.collapsed"
-              aria-controls="collapseExample">
+              [attr.aria-controls]="'health-info-component-' + entry.key + '-content'">
         {{ entry.key | titlecase }}
       </button>
-      <div class="d-inline-block">
+      <div class="d-flex my-auto">
         <span *ngIf="collapse.collapsed" class="fas fa-chevron-down"></span>
         <span *ngIf="!collapse.collapsed" class="fas fa-chevron-up"></span>
       </div>
     </div>
-    <div #collapse="ngbCollapse" [ngbCollapse]="isCollapsed">
+    <div #collapse="ngbCollapse" [id]="'health-info-component-' + entry.key + '-content'" [ngbCollapse]="isCollapsed">
       <div class="card border-0">
         <div class="card-body">
           <ds-health-info-component [healthInfoComponent]="entry.value"
diff --git a/src/app/health-page/health-info/health-info.component.html b/src/app/health-page/health-info/health-info.component.html
index 4bafcaa2d8a..b590695b0b0 100644
--- a/src/app/health-page/health-info/health-info.component.html
+++ b/src/app/health-page/health-info/health-info.component.html
@@ -2,14 +2,14 @@
   <ngb-accordion #acc="ngbAccordion" [activeIds]="activeId">
     <ngb-panel [id]="entry.key" *ngFor="let entry of healthInfoResponse | dsObjNgFor">
       <ng-template ngbPanelHeader>
-        <div class="w-100 d-flex justify-content-between collapse-toggle" ngbPanelToggle (click)="acc.toggle(entry.key)" data-test="info-component">
-          <button type="button" class="btn btn-link p-0" (click)="$event.preventDefault()" [attr.aria-expanded]="!acc.isExpanded(entry.key)"
-                  aria-controls="collapsePanels">
+        <div class="w-100 d-flex gap-3 justify-content-between collapse-toggle" ngbPanelToggle (click)="acc.toggle(entry.key)" data-test="info-component">
+          <button type="button" class="btn btn-link p-0" (click)="$event.preventDefault()" [attr.aria-expanded]="acc.isExpanded(entry.key)"
+                  [attr.aria-controls]="'health-info-' + entry.key + '-content'">
             {{ getPanelLabel(entry.key) | titlecase }}
           </button>
-          <div class="text-right d-flex">
+          <div class="text-right d-flex gap-2">
             <ds-health-status [status]="entry.value?.status"></ds-health-status>
-            <div class="ml-3 d-inline-block">
+            <div class="d-flex my-auto">
               <span *ngIf="acc.isExpanded(entry.key)" class="fas fa-chevron-up fa-fw"></span>
               <span *ngIf="!acc.isExpanded(entry.key)" class="fas fa-chevron-down fa-fw"></span>
             </div>
@@ -17,8 +17,9 @@
         </div>
       </ng-template>
       <ng-template ngbPanelContent>
-        <ds-health-info-component [healthInfoComponentName]="entry.key"
-                                  [healthInfoComponent]="entry.value"></ds-health-info-component>
+        <ds-health-info-component [healthInfoComponentName]="entry.key" [healthInfoComponent]="entry.value"
+                                  [id]="'health-info-' + entry.key + '-content'">
+        </ds-health-info-component>
       </ng-template>
     </ngb-panel>
   </ngb-accordion>
diff --git a/src/app/health-page/health-page.component.html b/src/app/health-page/health-page.component.html
index 8083389e1b4..cdfca1bb8ad 100644
--- a/src/app/health-page/health-page.component.html
+++ b/src/app/health-page/health-page.component.html
@@ -2,7 +2,7 @@
   <h2>{{'health-page.heading' | translate}}</h2>
   <div *ngIf="(healthResponse | async) && (healthInfoResponse | async)">
     <ul ngbNav #nav="ngbNav" [activeId]="'status'" class="nav-tabs">
-      <li [ngbNavItem]="'status'">
+      <li [ngbNavItem]="'status'" role="tab">
         <a ngbNavLink>{{'health-page.status-tab' | translate}}</a>
         <ng-template ngbNavContent>
           <div id="status">
@@ -10,7 +10,7 @@ <h2>{{'health-page.heading' | translate}}</h2>
           </div>
         </ng-template>
       </li>
-      <li [ngbNavItem]="'info'">
+      <li [ngbNavItem]="'info'" role="tab">
         <a ngbNavLink>{{'health-page.info-tab' | translate}}</a>
         <ng-template ngbNavContent>
           <div id="info">
@@ -21,7 +21,7 @@ <h2>{{'health-page.heading' | translate}}</h2>
     </ul>
     <div [ngbNavOutlet]="nav" class="mt-2"></div>
   </div>
-  <ds-alert *ngIf="!(healthResponse | async) || !(healthInfoResponse | async)" [type]="'alert-danger'" [content]="'health-page.error.msg'"></ds-alert>
+  <ds-alert *ngIf="!(healthResponse | async) || !(healthInfoResponse | async)" [type]="AlertType.Error" [content]="'health-page.error.msg'"></ds-alert>
 </div>
 
 
diff --git a/src/app/health-page/health-page.component.ts b/src/app/health-page/health-page.component.ts
index aa7bd7cba46..5c3c7ed70e2 100644
--- a/src/app/health-page/health-page.component.ts
+++ b/src/app/health-page/health-page.component.ts
@@ -5,6 +5,7 @@ import { take } from 'rxjs/operators';
 
 import { HealthService } from './health.service';
 import { HealthInfoResponse, HealthResponse } from './models/health-component.model';
+import { AlertType } from '../shared/alert/alert-type';
 
 @Component({
   selector: 'ds-health-page',
@@ -33,6 +34,8 @@ export class HealthPageComponent implements OnInit {
    */
   healthInfoResponseInitialised: BehaviorSubject<boolean> = new BehaviorSubject<boolean>(false);
 
+  readonly AlertType = AlertType;
+
   constructor(private healthDataService: HealthService) {
   }
 
diff --git a/src/app/health-page/health-panel/health-component/health-component.component.html b/src/app/health-page/health-panel/health-component/health-component.component.html
index 1f29c8c9fcc..f962e66052e 100644
--- a/src/app/health-page/health-panel/health-component/health-component.component.html
+++ b/src/app/health-page/health-panel/health-component/health-component.component.html
@@ -1,16 +1,16 @@
 <ng-container *ngIf="healthComponent?.components">
   <div *ngFor="let entry of healthComponent?.components | dsObjNgFor" class="mb-3 border-bottom" data-test="collapse">
-    <div class="w-100 d-flex justify-content-between collapse-toggle" (click)="collapse.toggle()">
+    <div class="w-100 d-flex gap-3 justify-content-between collapse-toggle" (click)="collapse.toggle()">
       <button type="button" class="btn btn-link p-0" (click)="$event.preventDefault()" [attr.aria-expanded]="!collapse.collapsed"
-              aria-controls="collapseExample">
+              [attr.aria-controls]="'health-component-' + entry.key + 'content'">
         {{ entry.key | titlecase }}
       </button>
-      <div class="d-inline-block">
+      <div class="d-flex my-auto">
         <span *ngIf="collapse.collapsed" class="fas fa-chevron-down"></span>
         <span *ngIf="!collapse.collapsed" class="fas fa-chevron-up"></span>
       </div>
     </div>
-    <div #collapse="ngbCollapse" [ngbCollapse]="isCollapsed">
+    <div #collapse="ngbCollapse" [id]="'health-component-' + entry.key + 'content'" [ngbCollapse]="isCollapsed">
       <div class="card border-0">
         <div class="card-body">
           <ds-health-component [healthComponent]="entry.value"
diff --git a/src/app/health-page/health-panel/health-panel.component.html b/src/app/health-page/health-panel/health-panel.component.html
index 2d67fa537b6..dae14d97a4a 100644
--- a/src/app/health-page/health-panel/health-panel.component.html
+++ b/src/app/health-page/health-panel/health-panel.component.html
@@ -1,15 +1,18 @@
-<p class="h4">{{'health-page.status' | translate}} : <ds-health-status [status]="healthResponse.status"></ds-health-status></p>
+<p class="h4">
+  {{'health-page.status' | translate}}:
+  <ds-health-status [status]="healthResponse.status" class="d-inline-flex"></ds-health-status>
+</p>
 <ngb-accordion #acc="ngbAccordion" [activeIds]="activeId">
   <ngb-panel [id]="entry.key" *ngFor="let entry of healthResponse.components | dsObjNgFor">
     <ng-template ngbPanelHeader>
-      <div class="w-100 d-flex justify-content-between collapse-toggle" ngbPanelToggle (click)="acc.toggle(entry.key)" data-test="component">
-        <button type="button" class="btn btn-link p-0" (click)="$event.preventDefault()" [attr.aria-expanded]="!acc.isExpanded(entry.key)"
-                aria-controls="collapsePanels">
+      <div class="w-100 d-flex gap-3 justify-content-between collapse-toggle" ngbPanelToggle (click)="acc.toggle(entry.key)" data-test="component">
+        <button type="button" class="btn btn-link p-0" (click)="$event.preventDefault()" [attr.aria-expanded]="acc.isExpanded(entry.key)"
+                [attr.aria-controls]="'health-panel-' + entry.key + '-content'">
           {{ getPanelLabel(entry.key) | titlecase }}
         </button>
-        <div class="text-right d-flex">
+        <div class="text-right d-flex gap-2">
           <ds-health-status [status]="entry.value?.status"></ds-health-status>
-          <div class="ml-3 d-inline-block">
+          <div class="d-flex my-auto">
             <span *ngIf="acc.isExpanded(entry.key)" class="fas fa-chevron-up fa-fw"></span>
             <span *ngIf="!acc.isExpanded(entry.key)" class="fas fa-chevron-down fa-fw"></span>
           </div>
@@ -17,9 +20,9 @@
       </div>
     </ng-template>
     <ng-template ngbPanelContent>
-      <ds-health-component [healthComponent]="entry.value" [healthComponentName]="entry.key"></ds-health-component>
+      <ds-health-component [healthComponent]="entry.value" [healthComponentName]="entry.key"
+                           [id]="'health-panel-' + entry.key + '-content'">
+      </ds-health-component>
     </ng-template>
   </ngb-panel>
 </ngb-accordion>
-
-
diff --git a/src/app/health-page/health-panel/health-status/health-status.component.html b/src/app/health-page/health-panel/health-status/health-status.component.html
index 38a6f726018..e8710a07600 100644
--- a/src/app/health-page/health-panel/health-status/health-status.component.html
+++ b/src/app/health-page/health-panel/health-status/health-status.component.html
@@ -1,12 +1,12 @@
 <ng-container [ngSwitch]="status">
   <i *ngSwitchCase="HealthStatus.UP"
-     class="fa fa-check-circle text-success ml-2 mt-1"
+     class="fa fa-check-circle text-success my-auto"
      ngbTooltip="{{'health-page.status.ok.info' | translate}}" container="body" ></i>
   <i *ngSwitchCase="HealthStatus.UP_WITH_ISSUES"
-     class="fa fa-exclamation-triangle text-warning ml-2 mt-1"
+     class="fa fa-exclamation-triangle text-warning my-auto"
      ngbTooltip="{{'health-page.status.warning.info' | translate}}" container="body"></i>
   <i *ngSwitchCase="HealthStatus.DOWN"
-     class="fa fa-times-circle text-danger ml-2 mt-1"
+     class="fa fa-times-circle text-danger my-auto"
      ngbTooltip="{{'health-page.status.error.info' | translate}}" container="body"></i>
 
 </ng-container>
diff --git a/src/app/health-page/health-panel/health-status/health-status.component.scss b/src/app/health-page/health-panel/health-status/health-status.component.scss
index e69de29bb2d..79ff2d52696 100644
--- a/src/app/health-page/health-panel/health-status/health-status.component.scss
+++ b/src/app/health-page/health-panel/health-status/health-status.component.scss
@@ -0,0 +1,3 @@
+:host {
+  display: flex;
+}