From 05347220051d85acb0f2a994c58bdb15f4d533af Mon Sep 17 00:00:00 2001
From: Alexandre Vryghem <alexandre@atmire.com>
Date: Sun, 26 Nov 2023 21:28:12 +0100
Subject: [PATCH] Fix Bulk Access Management usability issues

- Added aria-labels to trash icon
- Added aria-labels ui-switch components (had to upgrade ngx-ui-switch to 14.1.0)
- Fixed aria-controls pointing to non-existing ids
- Fixed bulk-access-browse not having the tab role on it's tabs
---
 package.json                                  |   2 +-
 .../browse/bulk-access-browse.component.html  | 107 ++++++++++--------
 .../bulk-access-settings.component.html       |  10 +-
 .../access-control-array-form.component.html  |   1 +
 ...cess-control-form-container.component.html |   2 +
 src/assets/i18n/en.json5                      |  10 ++
 yarn.lock                                     |   8 +-
 7 files changed, 81 insertions(+), 59 deletions(-)

diff --git a/package.json b/package.json
index 553962dfeb7..e5347742c8c 100644
--- a/package.json
+++ b/package.json
@@ -121,7 +121,7 @@
     "ngx-infinite-scroll": "^15.0.0",
     "ngx-pagination": "6.0.3",
     "ngx-sortablejs": "^11.1.0",
-    "ngx-ui-switch": "^14.0.3",
+    "ngx-ui-switch": "^14.1.0",
     "nouislider": "^15.7.1",
     "pem": "1.14.7",
     "prop-types": "^15.8.1",
diff --git a/src/app/access-control/bulk-access/browse/bulk-access-browse.component.html b/src/app/access-control/bulk-access/browse/bulk-access-browse.component.html
index c716aedb8b3..a48736355ff 100644
--- a/src/app/access-control/bulk-access/browse/bulk-access-browse.component.html
+++ b/src/app/access-control/bulk-access/browse/bulk-access-browse.component.html
@@ -1,15 +1,15 @@
 <ngb-accordion #acc="ngbAccordion" [activeIds]="'browse'">
   <ngb-panel [id]="'browse'">
     <ng-template ngbPanelHeader>
-      <div class="w-100 d-flex justify-content-between collapse-toggle" ngbPanelToggle (click)="acc.toggle('browse')"
+      <div class="w-100 d-flex gap-3 justify-content-between collapse-toggle" ngbPanelToggle (click)="acc.toggle('browse')"
            data-test="browse">
         <button type="button" class="btn btn-link p-0" (click)="$event.preventDefault()"
                 [attr.aria-expanded]="!acc.isExpanded('browse')"
-                aria-controls="collapsePanels">
+                aria-controls="bulk-access-browse-panel-content">
           {{ 'admin.access-control.bulk-access-browse.header' | translate }}
         </button>
-        <div class="text-right d-flex">
-          <div class="ml-3 d-inline-block">
+        <div class="text-right d-flex gap-2">
+          <div class="d-flex my-auto">
             <span *ngIf="acc.isExpanded('browse')" class="fas fa-chevron-up fa-fw"></span>
             <span *ngIf="!acc.isExpanded('browse')" class="fas fa-chevron-down fa-fw"></span>
           </div>
@@ -17,51 +17,60 @@
       </div>
     </ng-template>
     <ng-template ngbPanelContent>
-      <ul ngbNav #nav="ngbNav" [(activeId)]="activateId" class="nav-pills">
-        <li [ngbNavItem]="'search'">
-          <a ngbNavLink>{{'admin.access-control.bulk-access-browse.search.header' | translate}}</a>
-          <ng-template ngbNavContent>
-            <div class="mx-n3">
-              <ds-themed-search [configuration]="'administrativeBulkAccess'"
-                                [selectable]="true"
-                                [selectionConfig]="{ repeatable: true, listId: listId }"
-                                [showThumbnails]="false"></ds-themed-search>
-            </div>
-          </ng-template>
-        </li>
-        <li [ngbNavItem]="'selected'">
-          <a ngbNavLink>
-            {{'admin.access-control.bulk-access-browse.selected.header' | translate: {number: ((objectsSelected$ | async)?.payload?.totalElements) ? (objectsSelected$ | async)?.payload?.totalElements : '0'} }}
-          </a>
-          <ng-template ngbNavContent>
-            <ds-pagination
-              [paginationOptions]="(paginationOptions$ | async)"
-              [pageInfoState]="(objectsSelected$|async)?.payload.pageInfo"
-              [collectionSize]="(objectsSelected$|async)?.payload?.totalElements"
-              [objects]="(objectsSelected$|async)"
-              [showPaginator]="false"
-              (prev)="pagePrev()"
-              (next)="pageNext()">
-              <ul *ngIf="(objectsSelected$|async)?.hasSucceeded" class="list-unstyled ml-4">
-                <li *ngFor='let object of (objectsSelected$|async)?.payload?.page | paginate: { itemsPerPage: (paginationOptions$ | async).pageSize,
-                            currentPage: (paginationOptions$ | async).currentPage, totalItems: (objectsSelected$|async)?.payload?.page.length }; let i = index; let last = last '
-                    class="mt-4 mb-4 d-flex"
-                    [attr.data-test]="'list-object' | dsBrowserOnly">
-                  <ds-selectable-list-item-control [index]="i"
-                                                   [object]="object"
-                                                   [selectionConfig]="{ repeatable: true, listId: listId }"></ds-selectable-list-item-control>
-                  <ds-listable-object-component-loader [listID]="listId"
-                                                       [index]="i"
-                                                       [object]="object"
-                                                       [showThumbnails]="false"
-                                                       [viewMode]="'list'"></ds-listable-object-component-loader>
-                </li>
-              </ul>
-            </ds-pagination>
-          </ng-template>
-        </li>
-      </ul>
-      <div [ngbNavOutlet]="nav" class="mt-5"></div>
+      <div id="bulk-access-browse-panel-content">
+        <ul ngbNav #nav="ngbNav" [(activeId)]="activateId" class="nav-pills">
+          <li [attr.aria-selected]="activateId === 'search'" [ngbNavItem]="'search'"
+              aria-controls="bulk-access-browse-panel-tab-search" role="tab">
+            <a id="bulk-access-browse-panel-link-search" ngbNavLink>
+              {{'admin.access-control.bulk-access-browse.search.header' | translate}}
+            </a>
+            <ng-template ngbNavContent>
+              <div aria-labelledby="bulk-access-browse-panel-link-search" class="mx-n3"
+                   id="bulk-access-browse-panel-tab-search">
+                <ds-themed-search [configuration]="'administrativeBulkAccess'"
+                                  [selectable]="true"
+                                  [selectionConfig]="{ repeatable: true, listId: listId }"
+                                  [showThumbnails]="false"></ds-themed-search>
+              </div>
+            </ng-template>
+          </li>
+          <li [attr.aria-selected]="activateId === 'selected'" [ngbNavItem]="'selected'"
+              aria-controls="bulk-access-browse-panel-tab-selected" role="tab">
+            <a id="bulk-access-browse-panel-link-selected" ngbNavLink>
+              {{'admin.access-control.bulk-access-browse.selected.header' | translate: {number: ((objectsSelected$ | async)?.payload?.totalElements) ? (objectsSelected$ | async)?.payload?.totalElements : '0'} }}
+            </a>
+            <ng-template ngbNavContent>
+              <ds-pagination
+                [paginationOptions]="(paginationOptions$ | async)"
+                [pageInfoState]="(objectsSelected$|async)?.payload.pageInfo"
+                [collectionSize]="(objectsSelected$|async)?.payload?.totalElements"
+                [objects]="(objectsSelected$|async)"
+                [showPaginator]="false"
+                (prev)="pagePrev()"
+                (next)="pageNext()"
+                aria-labelledby="bulk-access-browse-panel-link-selected"
+                id="bulk-access-browse-panel-tab-selected">
+                <ul *ngIf="(objectsSelected$|async)?.hasSucceeded" class="list-unstyled ml-4">
+                  <li *ngFor='let object of (objectsSelected$|async)?.payload?.page | paginate: { itemsPerPage: (paginationOptions$ | async).pageSize,
+                              currentPage: (paginationOptions$ | async).currentPage, totalItems: (objectsSelected$|async)?.payload?.page.length }; let i = index; let last = last '
+                      class="mt-4 mb-4 d-flex"
+                      [attr.data-test]="'list-object' | dsBrowserOnly">
+                    <ds-selectable-list-item-control [index]="i"
+                                                     [object]="object"
+                                                     [selectionConfig]="{ repeatable: true, listId: listId }"></ds-selectable-list-item-control>
+                    <ds-listable-object-component-loader [listID]="listId"
+                                                         [index]="i"
+                                                         [object]="object"
+                                                         [showThumbnails]="false"
+                                                         [viewMode]="'list'"></ds-listable-object-component-loader>
+                  </li>
+                </ul>
+              </ds-pagination>
+            </ng-template>
+          </li>
+        </ul>
+        <div [ngbNavOutlet]="nav" class="mt-5"></div>
+      </div>
     </ng-template>
   </ngb-panel>
 </ngb-accordion>
diff --git a/src/app/access-control/bulk-access/settings/bulk-access-settings.component.html b/src/app/access-control/bulk-access/settings/bulk-access-settings.component.html
index 01f36ef03f4..5718e22b256 100644
--- a/src/app/access-control/bulk-access/settings/bulk-access-settings.component.html
+++ b/src/app/access-control/bulk-access/settings/bulk-access-settings.component.html
@@ -1,13 +1,13 @@
 <ngb-accordion #acc="ngbAccordion" [activeIds]="'settings'">
   <ngb-panel [id]="'settings'">
     <ng-template ngbPanelHeader>
-      <div class="w-100 d-flex justify-content-between collapse-toggle" ngbPanelToggle (click)="acc.toggle('settings')" data-test="settings">
+      <div class="w-100 d-flex gap-3 justify-content-between collapse-toggle" ngbPanelToggle (click)="acc.toggle('settings')" data-test="settings">
         <button type="button" class="btn btn-link p-0" (click)="$event.preventDefault()" [attr.aria-expanded]="!acc.isExpanded('browse')"
-                aria-controls="collapsePanels">
+                aria-controls="bulk-access-settings-panel-content">
           {{ 'admin.access-control.bulk-access-settings.header' | translate }}
         </button>
-        <div class="text-right d-flex">
-          <div class="ml-3 d-inline-block">
+        <div class="text-right d-flex gap-2">
+          <div class="d-flex my-auto">
             <span *ngIf="acc.isExpanded('settings')" class="fas fa-chevron-up fa-fw"></span>
             <span *ngIf="!acc.isExpanded('settings')" class="fas fa-chevron-down fa-fw"></span>
           </div>
@@ -15,7 +15,7 @@
       </div>
     </ng-template>
     <ng-template ngbPanelContent>
-      <ds-access-control-form-container #dsAccessControlForm [showSubmit]="false"></ds-access-control-form-container>
+      <ds-access-control-form-container id="bulk-access-settings-panel-content" #dsAccessControlForm [showSubmit]="false"></ds-access-control-form-container>
     </ng-template>
   </ngb-panel>
 </ngb-accordion>
diff --git a/src/app/shared/access-control-form-container/access-control-array-form/access-control-array-form.component.html b/src/app/shared/access-control-form-container/access-control-array-form/access-control-array-form.component.html
index cd56904bd7f..e6ebabd707b 100644
--- a/src/app/shared/access-control-form-container/access-control-array-form/access-control-array-form.component.html
+++ b/src/app/shared/access-control-form-container/access-control-array-form/access-control-array-form.component.html
@@ -92,6 +92,7 @@
         <div class="input-group">
 
           <button type="button" class="btn btn-outline-danger"
+                  [attr.aria-label]="'access-control-remove' | translate"
                   [disabled]="ngForm.disabled || form.accessControls.length === 1"
                   (click)="removeAccessControlItem(control.id)">
             <i class="fas fa-trash"></i>
diff --git a/src/app/shared/access-control-form-container/access-control-form-container.component.html b/src/app/shared/access-control-form-container/access-control-form-container.component.html
index a5173d10d7d..244726620d3 100644
--- a/src/app/shared/access-control-form-container/access-control-form-container.component.html
+++ b/src/app/shared/access-control-form-container/access-control-form-container.component.html
@@ -16,6 +16,7 @@ <h4 class="mb-0 mr-4">
                 {{ 'access-control-item-header-toggle' | translate }}
               </h4>
               <ui-switch
+                [ariaLabel]="(state.bitstream.toggleStatus ? 'access-control-item-toggle.disable' : 'access-control-item-toggle.enable') | translate"
                 [(ngModel)]="state.item.toggleStatus"
                 (ngModelChange)="handleStatusChange('item', $event)">
               </ui-switch>
@@ -69,6 +70,7 @@ <h4 class="mb-0 mr-4">
                 {{'access-control-bitstream-header-toggle' | translate}}
               </h4>
               <ui-switch
+                [ariaLabel]="(state.bitstream.toggleStatus ? 'access-control-bitstream-toggle.disable' : 'access-control-bitstream-toggle.enable') | translate"
                 [(ngModel)]="state.bitstream.toggleStatus"
                 (ngModelChange)="handleStatusChange('bitstream', $event)">
               </ui-switch>
diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5
index 6dee5d54e66..51e6ba0baf4 100644
--- a/src/assets/i18n/en.json5
+++ b/src/assets/i18n/en.json5
@@ -5190,8 +5190,16 @@
 
   "access-control-item-header-toggle": "Item's Metadata",
 
+  "access-control-item-toggle.enable": "Enable option to perform changes on the item's metadata",
+
+  "access-control-item-toggle.disable": "Disable option to perform changes on the item's metadata",
+
   "access-control-bitstream-header-toggle": "Bitstreams",
 
+  "access-control-bitstream-toggle.enable": "Enable option to perform changes on the bitstreams",
+
+  "access-control-bitstream-toggle.disable": "Disable option to perform changes on the bitstreams",
+
   "access-control-mode": "Mode",
 
   "access-control-access-conditions": "Access conditions",
@@ -5214,6 +5222,8 @@
 
   "access-control-add-more": "Add more",
 
+  "access-control-remove": "Remove access condition",
+
   "access-control-select-bitstreams-modal.title": "Select bitstreams",
 
   "access-control-select-bitstreams-modal.no-items": "No items to show.",
diff --git a/yarn.lock b/yarn.lock
index 27307aefa2f..6e04ab27b2a 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -8369,10 +8369,10 @@ ngx-sortablejs@^11.1.0:
   dependencies:
     tslib "^2.0.0"
 
-ngx-ui-switch@^14.0.3:
-  version "14.0.3"
-  resolved "https://registry.npmjs.org/ngx-ui-switch/-/ngx-ui-switch-14.0.3.tgz"
-  integrity sha512-SZ8ZnTCuNJgNWuY3/mOG3TdsRUNPCX3vGdCKKx1ZHVMTUynerJQlTWck2JrYlnrexNnd7wy4P10jDvdtDwoYeg==
+ngx-ui-switch@^14.1.0:
+  version "14.1.0"
+  resolved "https://registry.yarnpkg.com/ngx-ui-switch/-/ngx-ui-switch-14.1.0.tgz#32248361a684257c5ae64cfde61b95de920b901c"
+  integrity sha512-uGGLppBP1FXjyD+x7f8Pm6I3JQTMmsBqPtwERAX27jSZxDWFp/sewnl75fuDvu75qFofJd/BIOtV4xHxzgZOvw==
   dependencies:
     tslib "^2.3.0"