From ca24713d6d36afa275bd9d85a486c47931ca2ce5 Mon Sep 17 00:00:00 2001
From: Alexandre Vryghem <alexandre@atmire.com>
Date: Tue, 26 Dec 2023 11:26:08 +0100
Subject: [PATCH] Fixed dropdown accessibility issue

- Removed non-existing dropdownMenuButton references
- Added appropriate roles to dropdown menus
---
 ...space-new-external-dropdown.component.html |  4 +--
 ...ace-new-submission-dropdown.component.html |  4 +--
 .../collection-dropdown.component.html        | 22 +++++++++-------
 .../collection-dropdown.component.scss        |  3 ++-
 .../shared/ds-select/ds-select.component.html |  2 +-
 .../entity-dropdown.component.html            | 26 ++++++++++---------
 .../entity-dropdown.component.scss            | 10 ++++---
 .../entity-dropdown.component.spec.ts         |  2 +-
 .../lookup/dynamic-lookup.component.html      |  1 -
 .../submission-form-collection.component.html |  2 +-
 src/assets/i18n/en.json5                      |  2 ++
 11 files changed, 44 insertions(+), 34 deletions(-)

diff --git a/src/app/my-dspace-page/my-dspace-new-submission/my-dspace-new-external-dropdown/my-dspace-new-external-dropdown.component.html b/src/app/my-dspace-page/my-dspace-new-submission/my-dspace-new-external-dropdown/my-dspace-new-external-dropdown.component.html
index 392134dbce5..23438c205f3 100644
--- a/src/app/my-dspace-page/my-dspace-new-submission/my-dspace-new-external-dropdown/my-dspace-new-external-dropdown.component.html
+++ b/src/app/my-dspace-page/my-dspace-new-submission/my-dspace-new-external-dropdown/my-dspace-new-external-dropdown.component.html
@@ -15,10 +15,10 @@
             [attr.data-test]="'import-dropdown' | dsBrowserOnly"
             title="{{'mydspace.new-submission-external' | translate}}">
       <i class="fa fa-file-import" aria-hidden="true"></i>
-      <span class="caret"></span>
+      <span class="caret" aria-hidden="true"></span>
     </button>
     <div ngbDropdownMenu
-         class="dropdown-menu"
+         class="dropdown-menu p-0"
          id="importControlsDropdownMenu"
          aria-labelledby="dropdownImport">
       <ds-entity-dropdown [isSubmission]="false" (selectionChange)="openPage($event)"></ds-entity-dropdown>
diff --git a/src/app/my-dspace-page/my-dspace-new-submission/my-dspace-new-submission-dropdown/my-dspace-new-submission-dropdown.component.html b/src/app/my-dspace-page/my-dspace-new-submission/my-dspace-new-submission-dropdown/my-dspace-new-submission-dropdown.component.html
index 73fbeb30c2b..566e40d98a3 100644
--- a/src/app/my-dspace-page/my-dspace-new-submission/my-dspace-new-submission-dropdown/my-dspace-new-submission-dropdown.component.html
+++ b/src/app/my-dspace-page/my-dspace-new-submission/my-dspace-new-submission-dropdown/my-dspace-new-submission-dropdown.component.html
@@ -13,10 +13,10 @@
             [attr.data-test]="'submission-dropdown' | dsBrowserOnly"
             title="{{'mydspace.new-submission' | translate}}">
       <i class="fa fa-plus-circle" aria-hidden="true"></i>
-      <span class="caret"></span>
+      <span class="caret" aria-hidden="true"></span>
     </button>
     <div ngbDropdownMenu
-         class="dropdown-menu"
+         class="dropdown-menu p-0"
          id="entityControlsDropdownMenu"
          aria-labelledby="dropdownSubmission">
       <ds-entity-dropdown [isSubmission]="true" (selectionChange)="openDialog($event)"></ds-entity-dropdown>
diff --git a/src/app/shared/collection-dropdown/collection-dropdown.component.html b/src/app/shared/collection-dropdown/collection-dropdown.component.html
index 470291373fb..a27aafa31d1 100644
--- a/src/app/shared/collection-dropdown/collection-dropdown.component.html
+++ b/src/app/shared/collection-dropdown/collection-dropdown.component.html
@@ -1,4 +1,4 @@
-<div *ngIf="searchField" class="form-group w-100 pr-2 pl-2">
+<div *ngIf="searchField" class="form-group w-100 pr-2 pl-2 my-2">
   <input type="search"
          class="form-control w-100"
          (click)="$event.stopPropagation();"
@@ -6,9 +6,9 @@
          [formControl]="searchField"
          #searchFieldEl>
 </div>
-<div class="dropdown-divider"></div>
-<ul class="scrollable-menu p-0"
-     aria-labelledby="dropdownMenuButton"
+<div class="dropdown-divider m-0"></div>
+<ul class="scrollable-menu p-0 m-0"
+    role="menu"
      (scroll)="onScroll($event)"
      infiniteScroll
      [infiniteScrollDistance]="1.5"
@@ -18,12 +18,13 @@
      [scrollWindow]="false"
      (scrolled)="onScrollDown()">
 
-  <li class="dropdown-item disabled" *ngIf="searchListCollection?.length == 0 && !(isLoading | async)">
+  <li class="dropdown-item disabled" role="menuitem" *ngIf="searchListCollection?.length == 0 && !(isLoading | async)">
     {{'submission.sections.general.no-collection' | translate}}
   </li>
   <ng-container *ngIf="searchListCollection?.length > 0">
     <li *ngFor="let listItem of searchListCollection"
             class="dropdown-item collection-item"
+            role="menuitem"
             title="{{ listItem.collection.name }}"
             (click)="onSelect(listItem)">
       <div class="list-unstyled mb-0">
@@ -34,9 +35,10 @@
       </div>
     </li>
   </ng-container>
-  <button class="dropdown-item disabled" *ngIf="(isLoading | async)">
-    <ds-themed-loading message="{{'loading.default' | translate}}">
-    </ds-themed-loading>
-  </button>
-
+  <li *ngIf="(isLoading | async)">
+    <button class="dropdown-item disabled">
+      <ds-themed-loading message="{{'loading.default' | translate}}">
+      </ds-themed-loading>
+    </button>
+  </li>
 </ul>
diff --git a/src/app/shared/collection-dropdown/collection-dropdown.component.scss b/src/app/shared/collection-dropdown/collection-dropdown.component.scss
index 907a111900c..8a28a3ff7e1 100644
--- a/src/app/shared/collection-dropdown/collection-dropdown.component.scss
+++ b/src/app/shared/collection-dropdown/collection-dropdown.component.scss
@@ -4,7 +4,8 @@
   overflow-x: hidden;
 }
 
-.collection-item {
+li:not(:last-of-type), .dropdown-divider {
+  border-top: none;
   border-bottom: var(--bs-dropdown-border-width) solid var(--bs-dropdown-border-color);
 }
 
diff --git a/src/app/shared/ds-select/ds-select.component.html b/src/app/shared/ds-select/ds-select.component.html
index 5a15155d4f4..c12ea613472 100644
--- a/src/app/shared/ds-select/ds-select.component.html
+++ b/src/app/shared/ds-select/ds-select.component.html
@@ -22,7 +22,7 @@
          class="dropdown-menu"
          id="dsSelectDropdownMenu"
          aria-labelledby="dsSelectMenuButton">
-      <div aria-labelledby="dropdownMenuButton">
+      <div>
         <ng-content select=".menu"></ng-content>
       </div>
     </div>
diff --git a/src/app/shared/entity-dropdown/entity-dropdown.component.html b/src/app/shared/entity-dropdown/entity-dropdown.component.html
index a4d539625f5..38b4ecc53ef 100644
--- a/src/app/shared/entity-dropdown/entity-dropdown.component.html
+++ b/src/app/shared/entity-dropdown/entity-dropdown.component.html
@@ -1,6 +1,5 @@
-<div
-    class="scrollable-menu"
-    aria-labelledby="dropdownMenuButton"
+<ul class="scrollable-menu list-unstyled mb-0"
+    role="menu"
     (scroll)="onScroll($event)"
     infiniteScroll
     [infiniteScrollDistance]="5"
@@ -9,20 +8,23 @@
     [fromRoot]="true"
     [scrollWindow]="false"
     (scrolled)="onScrollDown()">
-    <button class="dropdown-item disabled" *ngIf="searchListEntity?.length == 0 && !(isLoadingList | async)">
+  <li *ngIf="searchListEntity?.length == 0 && !(isLoadingList | async)">
+    <button class="dropdown-item disabled" role="menuitem">
       {{'submission.sections.general.no-entity' | translate}}
     </button>
-    <button *ngFor="let listItem of searchListEntity"
-            class="dropdown-item entity-item"
+  </li>
+  <li *ngFor="let listItem of searchListEntity" class="entity-item text-primary">
+    <button class="dropdown-item"
+            role="menuitem"
             title="{{ listItem.label }}"
             (click)="onSelect(listItem)">
-      <ul class="list-unstyled mb-0">
-        <li class="list-item text-truncate text-primary font-weight-bold">{{ listItem.label.toLowerCase() + '.listelement.badge' | translate }}</li>
-      </ul>
+      <span class="text-truncate font-weight-bold">{{ listItem.label.toLowerCase() + '.listelement.badge' | translate }}</span>
     </button>
-    <button class="dropdown-item disabled" *ngIf="(isLoadingList | async)" >
+  </li>
+  <li *ngIf="(isLoadingList | async)">
+    <button class="dropdown-item disabled" role="menuitem">
       <ds-themed-loading message="{{'loading.default' | translate}}">
       </ds-themed-loading>
     </button>
-
-</div>
+  </li>
+</ul>
diff --git a/src/app/shared/entity-dropdown/entity-dropdown.component.scss b/src/app/shared/entity-dropdown/entity-dropdown.component.scss
index a5f43f359bd..4f9ca623107 100644
--- a/src/app/shared/entity-dropdown/entity-dropdown.component.scss
+++ b/src/app/shared/entity-dropdown/entity-dropdown.component.scss
@@ -1,5 +1,9 @@
-.list-item:active {
-  color: white !important;
+.dropdown-item {
+  padding: 0.35rem 1rem;
+
+  &:active {
+    color: white !important;
+  }
 }
 
 .scrollable-menu {
@@ -8,7 +12,7 @@
   overflow-x: hidden;
 }
 
-.entity-item {
+li:not(:last-of-type) .dropdown-item {
   border-bottom: var(--bs-dropdown-border-width) solid var(--bs-dropdown-border-color);
 }
 
diff --git a/src/app/shared/entity-dropdown/entity-dropdown.component.spec.ts b/src/app/shared/entity-dropdown/entity-dropdown.component.spec.ts
index 13e6dbc9c5d..2f3aa763bcc 100644
--- a/src/app/shared/entity-dropdown/entity-dropdown.component.spec.ts
+++ b/src/app/shared/entity-dropdown/entity-dropdown.component.spec.ts
@@ -123,7 +123,7 @@ describe('EntityDropdownComponent', () => {
     scheduler.flush();
 
     spyOn(component, 'onSelect');
-    const entityItem = fixture.debugElement.query(By.css('.entity-item:nth-child(2)'));
+    const entityItem = fixture.debugElement.query(By.css('.entity-item:nth-child(2) button'));
     entityItem.triggerEventHandler('click', null);
 
     scheduler.schedule(() => fixture.detectChanges());
diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.html
index c624c46d2dc..0ecddf92780 100644
--- a/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.html
+++ b/src/app/shared/form/builder/ds-dynamic-form-ui/models/lookup/dynamic-lookup.component.html
@@ -88,7 +88,6 @@
        aria-expanded="false"
        [attr.aria-labelledby]="'label_' + model.id">
     <div class="scrollable-menu"
-         aria-labelledby="scrollableDropdownMenuButton"
          infiniteScroll
          [infiniteScrollDistance]="2"
          [infiniteScrollThrottle]="50"
diff --git a/src/app/submission/form/collection/submission-form-collection.component.html b/src/app/submission/form/collection/submission-form-collection.component.html
index a78d737640c..f967ffe132a 100644
--- a/src/app/submission/form/collection/submission-form-collection.component.html
+++ b/src/app/submission/form/collection/submission-form-collection.component.html
@@ -32,7 +32,7 @@
     </button>
 
     <div ngbDropdownMenu
-         class="dropdown-menu"
+         class="dropdown-menu p-0"
          id="collectionControlsDropdownMenu"
          aria-labelledby="collectionControlsMenuButton">
       <ds-themed-collection-dropdown
diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5
index 462d2f2c92c..20199cf9ccd 100644
--- a/src/assets/i18n/en.json5
+++ b/src/assets/i18n/en.json5
@@ -4600,6 +4600,8 @@
 
   "submission.sections.general.no-collection": "No collection found",
 
+  "submission.sections.general.no-entity": "No entity types found",
+
   "submission.sections.general.no-sections": "No options available",
 
   "submission.sections.general.save_error_notice": "There was an issue when saving the item, please try again later.",