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.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

@@ -7,9 +7,9 @@
     <div class="w-100">
       <div id="collapsingNav">
         <ng-container *ngIf="(isMobile$ | async) && (isAuthenticated$ | async)">
-          <ds-themed-user-menu [inExpandableNavbar]="true"></ds-themed-user-menu>
+          <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>

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

@@ -1,50 +1,56 @@
 <div class="navbar-nav mr-auto" *ngIf="!(isMobile$ | async); else mobileButtons" data-test="auth-nav">
-  <div *ngIf="!(isAuthenticated | async) && (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 *ngIf="!(isAuthenticated$ | async) && 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-themed-log-in
           [isStandalonePage]="false"></ds-themed-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 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-themed-user-menu [inExpandableNavbar]="false"></ds-themed-user-menu>
+        <ds-user-menu [inExpandableNavbar]="false"></ds-user-menu>
       </div>
     </div>
   </div>
 </div>
-
-
 <ng-template #mobileButtons>
   <div data-test="auth-nav">
-    <a *ngIf="!(isAuthenticated | async)" routerLink="/login" routerLinkActive="active" class="loginLink px-0.5" role="button" tabindex="0">
+    <a *ngIf="!(isAuthenticated$ | async)" 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>

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.spec.ts

@@ -252,7 +252,7 @@ describe('AuthNavMenuComponent', () => {
           component = null;
         });
         it('should render UserMenuComponent component', () => {
-          const logoutDropdownMenu = deNavMenuItem.query(By.css('ds-themed-user-menu'));
+          const logoutDropdownMenu = deNavMenuItem.query(By.css('ds-user-menu'));
           expect(logoutDropdownMenu.nativeElement).toBeDefined();
         });
       });

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

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

@@ -3,14 +3,15 @@
           [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">
     <i class="fas fa-globe-asia fa-lg fa-fw"></i>
   </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"

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

@@ -28,10 +28,10 @@
 </form>
 
 <div class="mt-2">
-  <a class="dropdown-item" *ngIf="canRegister$ | async" [routerLink]="[getRegisterRoute()]" [attr.data-test]="'register' | dsBrowserOnly" role="menuitem" tabindex="0">
+  <a class="dropdown-item" *ngIf="canRegister$ | async" [routerLink]="[getRegisterRoute()]" [attr.data-test]="'register' | dsBrowserOnly" tabindex="0">
     {{ 'login.form.new-user' | translate }}
   </a>
-  <a class="dropdown-item" [routerLink]="[getForgotRoute()]" [attr.data-test]="'forgot' | dsBrowserOnly" role="menuitem" tabindex="0">
+  <a class="dropdown-item" [routerLink]="[getForgotRoute()]" [attr.data-test]="'forgot' | dsBrowserOnly" tabindex="0">
     {{ 'login.form.forgot-password' | translate }}
   </a>
 </div>

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 justify-content-end align-items-center gapx-1 ml-auto">
       <ds-themed-search-navbar></ds-themed-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-themed-lang-switch></ds-themed-lang-switch>
         <ds-context-help-toggle></ds-context-help-toggle>
         <ds-impersonate-navbar></ds-impersonate-navbar>