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

Author: alexandrevryghem

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');
   });
 });

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>
         @if (isMobile$ | async) {
           <div class="ps-2">
             <button class="navbar-toggler px-0" type="button" (click)="toggleNavbar()"
@@ -20,7 +22,7 @@
             </button>
           </div>
         }
-      </nav>
+      </div>
     </div>
   </div>
 </header>

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;

src/app/navbar/navbar.component.html

@@ -9,7 +9,7 @@
         @if ((isMobile$ | async) && (isAuthenticated$ | async)) {
           <ds-user-menu [inExpandableNavbar]="true"></ds-user-menu>
         }
-        <div class="navbar-nav align-items-md-center me-auto shadow-none gapx-3">
+        <div class="navbar-nav align-items-md-center me-auto shadow-none gapx-3" role="menubar">
           @for (section of (sections | async); track section) {
             <ng-container
             *ngComponentOutlet="(sectionMap$ | async).get(section.id)?.component; injector: (sectionMap$ | async).get(section.id)?.injector;"></ng-container>

src/app/shared/auth-nav-menu/auth-nav-menu.component.html

@@ -1,63 +1,66 @@
+@let isAuthenticated = (isAuthenticated$ | async);
 @if ((isMobile$ | async) !== true) {
   <div class="navbar-nav me-auto" data-test="auth-nav">
-    @if ((isAuthenticated | async) !== true && (showAuth | async)) {
+    @let showAuth = (showAuth$ | async);
+    @if (isAuthenticated !== true && showAuth) {
       <div
         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>
+          <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]="{'ps-3 pe-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>
     }
-    @if ((isAuthenticated | async) && (showAuth | async)) {
+    @if (isAuthenticated && showAuth) {
       <div class="nav-item">
         <div ngbDropdown #loggedInDrop="ngbDropdown" display="dynamic" placement="bottom-right" class="d-inline-block" @fadeInOut>
-          <a href="javascript:void(0);"
-            role="menuitem"
+          <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 px-1"
+            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></a>
-            <div id="logoutDropdownMenu" ngbDropdownMenu>
-              <ds-user-menu [inExpandableNavbar]="false" (changedRoute)="loggedInDrop.close()"></ds-user-menu>
-            </div>
+            <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>
-  } @else {
-    <div data-test="auth-nav">
-      @if ((isAuthenticated | async) !== true) {
-        <a routerLink="/login" routerLinkActive="active" class="loginLink px-0.5" role="button" tabindex="0">
-          {{ 'nav.login' | translate }}<span class="sr-only">(current)</span>
-        </a>
-      }
-      @if ((isAuthenticated | async)) {
-        <a role="button" [attr.aria-label]="'nav.logout' |translate" [title]="'nav.logout' | translate" routerLink="/logout" routerLinkActive="active" class="logoutLink px-1" role="button" tabindex="0">
-          <i class="fas fa-sign-out-alt fa-lg fa-fw"></i>
-          <span class="sr-only">(current)</span>
-        </a>
-      }
-    </div>
-  }
-
-
+      </div>
+    }
+  </div>
+} @else {
+  <div data-test="auth-nav">
+    @if (isAuthenticated === true) {
+      <a [attr.aria-label]="'nav.logout' |translate" [title]="'nav.logout' | translate" routerLink="/logout" routerLinkActive="active" class="logoutLink px-0" role="link" tabindex="0">
+        <i class="fas fa-sign-out-alt fa-lg fa-fw"></i>
+        <span class="sr-only">(current)</span>
+      </a>
+    } @else {
+      <a routerLink="/login" routerLinkActive="active" class="loginLink px-0" role="link" tabindex="0">
+        {{ 'nav.login' | translate }}<span class="sr-only">(current)</span>
+      </a>
+    }
+  </div>
+}
 
-  <!-- Do not use ul/li in this menu as it breaks e2e accessibility tests -->
+<!-- Do not use ul/li in this menu as it breaks e2e accessibility tests -->

src/app/shared/auth-nav-menu/auth-nav-menu.component.scss

@@ -28,3 +28,7 @@
     box-shadow: unset;
   }
 }
+
+.dropdown-toggle::after {
+  margin-left: 0;
+}

src/app/shared/auth-nav-menu/auth-nav-menu.component.ts

@@ -64,7 +64,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.
@@ -74,7 +74,7 @@ export class AuthNavMenuComponent implements OnInit {
 
   public isMobile$: Observable<boolean>;
 
-  public showAuth = observableOf(false);
+  public showAuth$ = observableOf(false);
 
   public user: Observable<EPerson>;
 
@@ -89,14 +89,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)

src/app/shared/lang-switch/lang-switch.component.html

@@ -4,7 +4,7 @@
             [attr.aria-label]="'nav.language' |translate"
             aria-controls="language-menu-list"
             aria-haspopup="menu"
-            class="dropdown-toggle"
+            class="dropdown-toggle btn btn-link px-0"
             [title]="'nav.language' | translate"
             (click)="$event.preventDefault()" data-toggle="dropdown" ngbDropdownToggle
             data-test="lang-switch"
@@ -16,6 +16,7 @@
       @for (lang of translate.getLangs(); track lang) {
         <div class="dropdown-item" tabindex="0"
              role="option"
+             [lang]="lang"
              (keyup.enter)="useLang(lang)"
              (click)="useLang(lang)"
              [attr.aria-selected]="lang === translate.currentLang"

src/app/shared/lang-switch/lang-switch.component.scss

@@ -3,7 +3,6 @@
 }
 
 .dropdown-toggle {
-  all: unset;
   color: var(--ds-header-icon-color);
 
   &:hover, &:focus {

src/app/shared/log-in/methods/password/log-in-password.component.html

@@ -35,11 +35,11 @@
     <div class="mt-2">
       @if (canRegister$ | async) {
         <a class="dropdown-item" [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>
       }
       @if (canForgot$ | async) {
         <a class="dropdown-item" [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>
   }