From 9e8bc95acc33c29144fec577c4119889e5c7105c Mon Sep 17 00:00:00 2001
From: Alexandre Vryghem <alexandre@atmire.com>
Date: Wed, 14 May 2025 16:43:14 +0200
Subject: [PATCH] 129964: Fixed the header role structure being invalid in the
 custom theme

- Replaced the menubar role from the parent of all the header buttons like lang switch, auth menu & help toggle with toolbar
- Replaced the remaining `<a>` buttons in the header with `<button>` to make them expandable with space
- Fixed accessibility issues flagged by axe DevTools in the user menu dropdown
---
 cypress/e2e/header.cy.ts                      | 12 ++--
 src/app/header/header.component.html          | 14 ++--
 src/app/header/header.component.scss          |  2 +-
 src/app/navbar/navbar.component.html          |  2 +-
 .../auth-nav-menu.component.html              | 64 ++++++++++---------
 .../auth-nav-menu.component.scss              |  4 ++
 .../auth-nav-menu/auth-nav-menu.component.ts  |  8 +--
 .../lang-switch/lang-switch.component.html    |  3 +-
 .../password/log-in-password.component.html   |  4 +-
 .../dspace/app/header/header.component.html   |  2 +-
 10 files changed, 64 insertions(+), 51 deletions(-)

diff --git a/cypress/e2e/header.cy.ts b/cypress/e2e/header.cy.ts
index 1471e5ae6c..aa65aee570 100644
--- a/cypress/e2e/header.cy.ts
+++ b/cypress/e2e/header.cy.ts
@@ -15,24 +15,24 @@ describe('Header', () => {
     cy.visit('/');
 
     // Click the language switcher (globe) in header
-    cy.get('a[data-test="lang-switch"]').click();
+    cy.get('button[data-test="lang-switch"]').click();
     // Click on the "Deusch" language in dropdown
-    cy.get('#language-menu-list li').contains('Deutsch').click();
+    cy.get('#language-menu-list div[role="option"]').contains('Deutsch').click();
 
     // HTML "lang" attribute should switch to "de"
     cy.get('html').invoke('attr', 'lang').should('eq', 'de');
 
     // Login menu should now be in German
-    cy.get('a[data-test="login-menu"]').contains('Anmelden');
+    cy.get('[data-test="login-menu"]').contains('Anmelden');
 
     // Change back to English from language switcher
-    cy.get('a[data-test="lang-switch"]').click();
-    cy.get('#language-menu-list li').contains('English').click();
+    cy.get('button[data-test="lang-switch"]').click();
+    cy.get('#language-menu-list div[role="option"]').contains('English').click();
 
     // HTML "lang" attribute should switch to "en"
     cy.get('html').invoke('attr', 'lang').should('eq', 'en');
 
     // Login menu should now be in English
-    cy.get('a[data-test="login-menu"]').contains('Log In');
+    cy.get('[data-test="login-menu"]').contains('Log In');
   });
 });
diff --git a/src/app/header/header.component.html b/src/app/header/header.component.html
index 8dd3df8509..69df856532 100644
--- a/src/app/header/header.component.html
+++ b/src/app/header/header.component.html
@@ -5,12 +5,14 @@
         <img src="assets/images/dspace-logo.svg" [attr.alt]="'menu.header.image.logo' | translate"/>
       </a>
 
-      <nav role="navigation" [attr.aria-label]="'nav.user.description' | translate" class="navbar navbar-light navbar-expand-md flex-shrink-0 px-0">
+      <div class="navbar navbar-light navbar-expand-md flex-shrink-0 px-0">
         <ds-search-navbar></ds-search-navbar>
-        <ds-lang-switch></ds-lang-switch>
-        <ds-context-help-toggle></ds-context-help-toggle>
-        <ds-auth-nav-menu></ds-auth-nav-menu>
-        <ds-impersonate-navbar></ds-impersonate-navbar>
+        <div role="toolbar" [attr.aria-label]="'nav.user.description' | translate">
+          <ds-lang-switch></ds-lang-switch>
+          <ds-context-help-toggle></ds-context-help-toggle>
+          <ds-auth-nav-menu></ds-auth-nav-menu>
+          <ds-impersonate-navbar></ds-impersonate-navbar>
+        </div>
         <div *ngIf="isMobile$ | async" class="pl-2">
           <button class="navbar-toggler px-0" type="button" (click)="toggleNavbar()"
                   aria-controls="collapsingNav"
@@ -18,7 +20,7 @@
             <span class="fas fa-bars fa-fw toggler-icon" aria-hidden="true"></span>
           </button>
         </div>
-      </nav>
+      </div>
     </div>
   </div>
 </header>
diff --git a/src/app/header/header.component.scss b/src/app/header/header.component.scss
index e002e0c347..b709198e38 100644
--- a/src/app/header/header.component.scss
+++ b/src/app/header/header.component.scss
@@ -23,7 +23,7 @@
     }
   }
 
-  .navbar {
+  .navbar, div[role="toolbar"] {
     display: flex;
     gap: calc(var(--bs-spacer) / 3);
     align-items: center;
diff --git a/src/app/navbar/navbar.component.html b/src/app/navbar/navbar.component.html
index 90cf07caa4..0c02a55cb5 100644
--- a/src/app/navbar/navbar.component.html
+++ b/src/app/navbar/navbar.component.html
@@ -9,7 +9,7 @@
         <ng-container *ngIf="(isMobile$ | async) && (isAuthenticated$ | async)">
           <ds-user-menu [inExpandableNavbar]="true"></ds-user-menu>
         </ng-container>
-        <div class="navbar-nav align-items-md-center mr-auto shadow-none gapx-3">
+        <div class="navbar-nav align-items-md-center mr-auto shadow-none gapx-3" role="menubar">
           <ng-container *ngFor="let section of (sections | async)">
             <ng-container
               *ngComponentOutlet="(sectionMap$ | async).get(section.id)?.component; injector: (sectionMap$ | async).get(section.id)?.injector;"></ng-container>
diff --git a/src/app/shared/auth-nav-menu/auth-nav-menu.component.html b/src/app/shared/auth-nav-menu/auth-nav-menu.component.html
index 19966c46d0..ce2227702f 100644
--- a/src/app/shared/auth-nav-menu/auth-nav-menu.component.html
+++ b/src/app/shared/auth-nav-menu/auth-nav-menu.component.html
@@ -1,50 +1,56 @@
-<div class="navbar-nav mr-auto" *ngIf="(isMobile$ | async) !== true; else mobileButtons" data-test="auth-nav">
-  <div *ngIf="(isAuthenticated | async) !== true && (showAuth | async)"
-      class="nav-item"
-      (click)="$event.stopPropagation();">
-    <div ngbDropdown #loginDrop="ngbDropdown" display="dynamic" placement="bottom-right" class="d-inline-block" @fadeInOut>
-      <a href="javascript:void(0);" class="dropdownLogin px-0.5" [attr.aria-label]="'nav.login' |translate"
-         (click)="$event.preventDefault()" [attr.data-test]="'login-menu' | dsBrowserOnly"
-         role="menuitem"
-         tabindex="0"
-         aria-haspopup="menu"
-         aria-controls="loginDropdownMenu"
-         [attr.aria-expanded]="loginDrop.isOpen()"
-         ngbDropdownToggle>{{ 'nav.login' | translate }}</a>
+<div class="navbar-nav mr-auto" *ngIf="(isMobile$ | async) === false; else mobileButtons" data-test="auth-nav">
+  <div *ngIf="(isAuthenticated$ | async) === false && showAuth$ | async"
+       class="nav-item"
+       (click)="$event.stopPropagation();">
+    <div ngbDropdown #loginDrop="ngbDropdown" display="dynamic" placement="bottom-right" class="d-inline-block"
+         @fadeInOut>
+      <button class="dropdownLogin btn btn-link px-0" [attr.aria-label]="'nav.login' |translate"
+              (click)="$event.preventDefault()" [attr.data-test]="'login-menu' | dsBrowserOnly"
+              role="button"
+              tabindex="0"
+              aria-haspopup="menu"
+              aria-controls="loginDropdownMenu"
+              [attr.aria-expanded]="loginDrop.isOpen()"
+              ngbDropdownToggle>
+        {{ 'nav.login' | translate }}
+      </button>
       <div id="loginDropdownMenu" [ngClass]="{'pl-3 pr-3': (loading | async)}" ngbDropdownMenu
-           role="menu"
+           role="dialog"
+           aria-modal="true"
            [attr.aria-label]="'nav.login' | translate">
         <ds-log-in
           [isStandalonePage]="false"></ds-log-in>
       </div>
     </div>
   </div>
-  <div *ngIf="(isAuthenticated | async) && (showAuth | async)" class="nav-item">
+  <div *ngIf="(isAuthenticated$ | async) && showAuth$ | async" class="nav-item">
     <div ngbDropdown #loggedInDrop="ngbDropdown" display="dynamic" placement="bottom-right" class="d-inline-block" @fadeInOut>
-      <a href="javascript:void(0);"
-         role="menuitem"
-         tabindex="0"
-         [attr.aria-label]="'nav.user-profile-menu-and-logout' | translate"
-         aria-controls="user-menu-dropdown"
-         (click)="$event.preventDefault()" [title]="'nav.user-profile-menu-and-logout' | translate"
-         class="dropdownLogout px-1"
-         [attr.data-test]="'user-menu' | dsBrowserOnly"
-         ngbDropdownToggle>
-        <i class="fas fa-user-circle fa-lg fa-fw"></i></a>
+      <button
+        role="button"
+        tabindex="0"
+        [attr.aria-label]="'nav.user-profile-menu-and-logout' | translate"
+        aria-controls="user-menu-dropdown"
+        (click)="$event.preventDefault()" [title]="'nav.user-profile-menu-and-logout' | translate"
+        class="dropdownLogout btn btn-link px-0"
+        [attr.data-test]="'user-menu' | dsBrowserOnly"
+        ngbDropdownToggle>
+        <i class="fas fa-user-circle fa-lg fa-fw"></i></button>
       <div id="logoutDropdownMenu" ngbDropdownMenu>
         <ds-user-menu [inExpandableNavbar]="false" (changedRoute)="loggedInDrop.close()"></ds-user-menu>
       </div>
     </div>
   </div>
 </div>
-
-
 <ng-template #mobileButtons>
   <div data-test="auth-nav">
-    <a *ngIf="(isAuthenticated | async) !== true" routerLink="/login" routerLinkActive="active" class="loginLink px-0.5" role="button" tabindex="0">
+    <a *ngIf="(isAuthenticated$ | async) === false" routerLink="/login" routerLinkActive="active" class="loginLink px-0.5"
+       role="link"
+       tabindex="0">
       {{ 'nav.login' | translate }}<span class="sr-only">(current)</span>
     </a>
-    <a *ngIf="(isAuthenticated | async)" role="button" [attr.aria-label]="'nav.logout' |translate" [title]="'nav.logout' | translate" routerLink="/logout" routerLinkActive="active" class="logoutLink px-1" tabindex="0">
+    <a *ngIf="(isAuthenticated$ | async)" role="link" [attr.aria-label]="'nav.logout' |translate"
+       [title]="'nav.logout' | translate" routerLink="/logout" routerLinkActive="active" class="logoutLink px-1"
+       tabindex="0">
       <i class="fas fa-sign-out-alt fa-lg fa-fw"></i>
       <span class="sr-only">(current)</span>
     </a>
diff --git a/src/app/shared/auth-nav-menu/auth-nav-menu.component.scss b/src/app/shared/auth-nav-menu/auth-nav-menu.component.scss
index 84e4916eff..7d4ec043ac 100644
--- a/src/app/shared/auth-nav-menu/auth-nav-menu.component.scss
+++ b/src/app/shared/auth-nav-menu/auth-nav-menu.component.scss
@@ -28,3 +28,7 @@
     box-shadow: unset;
   }
 }
+
+.dropdown-toggle::after {
+  margin-left: 0;
+}
diff --git a/src/app/shared/auth-nav-menu/auth-nav-menu.component.ts b/src/app/shared/auth-nav-menu/auth-nav-menu.component.ts
index 16899cf73e..ca66ebd0a7 100644
--- a/src/app/shared/auth-nav-menu/auth-nav-menu.component.ts
+++ b/src/app/shared/auth-nav-menu/auth-nav-menu.component.ts
@@ -65,7 +65,7 @@ export class AuthNavMenuComponent implements OnInit {
    * Whether user is authenticated.
    * @type {Observable<string>}
    */
-  public isAuthenticated: Observable<boolean>;
+  public isAuthenticated$: Observable<boolean>;
 
   /**
    * True if the authentication is loading.
@@ -75,7 +75,7 @@ export class AuthNavMenuComponent implements OnInit {
 
   public isMobile$: Observable<boolean>;
 
-  public showAuth = observableOf(false);
+  public showAuth$ = observableOf(false);
 
   public user: Observable<EPerson>;
 
@@ -90,14 +90,14 @@ export class AuthNavMenuComponent implements OnInit {
 
   ngOnInit(): void {
     // set isAuthenticated
-    this.isAuthenticated = this.store.pipe(select(isAuthenticated));
+    this.isAuthenticated$ = this.store.pipe(select(isAuthenticated));
 
     // set loading
     this.loading = this.store.pipe(select(isAuthenticationLoading));
 
     this.user = this.authService.getAuthenticatedUserFromStore();
 
-    this.showAuth = this.store.pipe(
+    this.showAuth$ = this.store.pipe(
       select(routerStateSelector),
       filter((router: RouterReducerState) => isNotUndefined(router) && isNotUndefined(router.state)),
       map((router: RouterReducerState) => (!router.state.url.startsWith(LOGIN_ROUTE)
diff --git a/src/app/shared/lang-switch/lang-switch.component.html b/src/app/shared/lang-switch/lang-switch.component.html
index 15bfd1ee96..0cfc210d0f 100644
--- a/src/app/shared/lang-switch/lang-switch.component.html
+++ b/src/app/shared/lang-switch/lang-switch.component.html
@@ -3,7 +3,7 @@
           [attr.aria-label]="'nav.language' |translate"
           aria-controls="language-menu-list"
           aria-haspopup="menu"
-          [title]="'nav.language' | translate" class="dropdown-toggle btn btn-link px-1"
+          [title]="'nav.language' | translate" class="dropdown-toggle btn btn-link px-0"
           (click)="$event.preventDefault()" data-toggle="dropdown" ngbDropdownToggle
           data-test="lang-switch"
           tabindex="0">
@@ -11,6 +11,7 @@
   </button>
   <div ngbDropdownMenu class="dropdown-menu" role="listbox" [attr.aria-label]="'nav.language' |translate" id="language-menu-list">
     <div class="dropdown-item" tabindex="0" role="option" #langSelect *ngFor="let lang of translate.getLangs()"
+         [lang]="lang"
          (keyup.enter)="useLang(lang)"
          (click)="useLang(lang)"
          [attr.aria-selected]="lang === translate.currentLang"
diff --git a/src/app/shared/log-in/methods/password/log-in-password.component.html b/src/app/shared/log-in/methods/password/log-in-password.component.html
index fd0e1ca513..d076aa5c31 100644
--- a/src/app/shared/log-in/methods/password/log-in-password.component.html
+++ b/src/app/shared/log-in/methods/password/log-in-password.component.html
@@ -30,8 +30,8 @@
 <ng-container *ngIf="canShowDivider$ | async">
   <div class="mt-2">
     <a class="dropdown-item" *ngIf="canRegister$ | async" [routerLink]="[getRegisterRoute()]"
-       [attr.data-test]="'register' | dsBrowserOnly" role="menuitem" tabindex="0">{{"login.form.new-user" | translate}}</a>
+       [attr.data-test]="'register' | dsBrowserOnly" tabindex="0">{{"login.form.new-user" | translate}}</a>
     <a class="dropdown-item" *ngIf="canForgot$ | async" [routerLink]="[getForgotRoute()]"
-       [attr.data-test]="'forgot' | dsBrowserOnly" role="menuitem" tabindex="0">{{"login.form.forgot-password" | translate}}</a>
+       [attr.data-test]="'forgot' | dsBrowserOnly" tabindex="0">{{"login.form.forgot-password" | translate}}</a>
   </div>
 </ng-container>
diff --git a/src/themes/dspace/app/header/header.component.html b/src/themes/dspace/app/header/header.component.html
index e66adf8c05..fce2618f2c 100644
--- a/src/themes/dspace/app/header/header.component.html
+++ b/src/themes/dspace/app/header/header.component.html
@@ -15,7 +15,7 @@
     <!-- Search bar and other menus -->
     <div id="header-right" class="h-100 d-flex flex-row flex-nowrap flex-shrink-0 justify-content-end align-items-center gapx-1 ml-auto">
       <ds-search-navbar></ds-search-navbar>
-      <div role="menubar" class="h-100 d-flex flex-row flex-nowrap align-items-center gapx-1">
+      <div role="toolbar" class="h-100 d-flex flex-row flex-nowrap align-items-center gapx-1">
         <ds-lang-switch></ds-lang-switch>
         <ds-context-help-toggle></ds-context-help-toggle>
         <ds-impersonate-navbar></ds-impersonate-navbar>