Merge pull request #1295 from atmire/w2p-81901_Fix-sidebar-a11y-issues

Fix sidebar a11y issues

src/app/admin/admin-sidebar/admin-sidebar-section/admin-sidebar-section.component.html

@@ -1,10 +1,23 @@
-<li class="sidebar-section">
-    <a href="javascript:void(0);" class="nav-item nav-link shortcut-icon" attr.aria-labelledby="sidebarName-{{section.id}}" [title]="('menu.section.icon.' + section.id) | translate" [routerLink]="itemModel.link">
+<div class="sidebar-section">
+    <a class="nav-item nav-link d-flex flex-row flex-nowrap"
+       [ngClass]="{ disabled: !hasLink }"
+       [attr.aria-disabled]="!hasLink"
+       [attr.aria-labelledby]="'sidebarName-' + section.id"
+       [title]="('menu.section.icon.' + section.id) | translate"
+       [routerLink]="itemModel.link"
+       (keyup.space)="navigate($event)"
+       (keyup.enter)="navigate($event)"
+       href="javascript:void(0);"
+    >
+      <div class="shortcut-icon">
         <i class="fas fa-{{section.icon}} fa-fw"></i>
+      </div>
+      <div class="sidebar-collapsible">
+        <div class="toggle">
+          <span id="sidebarName-{{section.id}}" class="section-header-text">
+            {{itemModel.text | translate}}
+          </span>
+        </div>
+      </div>
     </a>
-    <div class="sidebar-collapsible">
-        <span id="sidebarName-{{section.id}}" class="section-header-text">
-        <a class="nav-item nav-link"  tabindex="-1" [routerLink]="itemModel.link">{{itemModel.text | translate}}</a>
-        </span>
-    </div>
-</li>
+</div>

src/app/admin/admin-sidebar/admin-sidebar-section/admin-sidebar-section.component.ts

@@ -5,12 +5,15 @@ import { MenuService } from '../../../shared/menu/menu.service';
 import { rendersSectionForMenu } from '../../../shared/menu/menu-section.decorator';
 import { LinkMenuItemModel } from '../../../shared/menu/menu-item/models/link.model';
 import { MenuSection } from '../../../shared/menu/menu.reducer';
+import { isNotEmpty } from '../../../shared/empty.util';
+import { Router } from '@angular/router';
 
 /**
  * Represents a non-expandable section in the admin sidebar
  */
 @Component({
-  selector: 'ds-admin-sidebar-section',
+  /* tslint:disable:component-selector */
+  selector: 'li[ds-admin-sidebar-section]',
   templateUrl: './admin-sidebar-section.component.html',
   styleUrls: ['./admin-sidebar-section.component.scss'],
 
@@ -23,12 +26,26 @@ export class AdminSidebarSectionComponent extends MenuSectionComponent implement
    */
   menuID: MenuID = MenuID.ADMIN;
   itemModel;
-  constructor(@Inject('sectionDataProvider') menuSection: MenuSection, protected menuService: MenuService, protected injector: Injector,) {
+  hasLink: boolean;
+  constructor(
+    @Inject('sectionDataProvider') menuSection: MenuSection,
+    protected menuService: MenuService,
+    protected injector: Injector,
+    protected router: Router,
+  ) {
     super(menuSection, menuService, injector);
     this.itemModel = menuSection.model as LinkMenuItemModel;
   }
 
   ngOnInit(): void {
+    this.hasLink = isNotEmpty(this.itemModel?.link);
     super.ngOnInit();
   }
+
+  navigate(event: any): void {
+    event.preventDefault();
+    if (this.hasLink) {
+      this.router.navigate(this.itemModel.link);
+    }
+  }
 }

src/app/admin/admin-sidebar/admin-sidebar.component.html

@@ -4,24 +4,26 @@
       value: (!(sidebarExpanded | async) ? 'collapsed' : 'expanded'),
       params: {sidebarWidth: (sidebarWidth | async)}
      }" (@slideSidebar.done)="finishSlide($event)" (@slideSidebar.start)="startSlide($event)"
-     *ngIf="menuVisible | async" (mouseenter)="expandPreview($event)"
-     (mouseleave)="collapsePreview($event)"
+     *ngIf="menuVisible | async"
+     (mouseenter)="handleMouseEnter($event)"
+     (mouseleave)="handleMouseLeave($event)"
      role="navigation" [attr.aria-label]="'menu.header.admin.description' |translate">
     <div class="sidebar-top-level-items">
         <ul class="navbar-nav">
-            <li class="admin-menu-header sidebar-section">
-                <a class="shortcut-icon navbar-brand mr-0" href="javascript:void(0);">
-                <span class="logo-wrapper">
+            <li class="admin-menu-header">
+              <div class="sidebar-section">
+                <div href="javascript:void(0);" class="nav-item d-flex flex-row flex-nowrap py-0">
+                  <div class="shortcut-icon navbar-brand logo-wrapper">
                     <img class="admin-logo" src="assets/images/dspace-logo-mini.svg"
                          [alt]="('menu.header.image.logo') | translate">
-                </span>
-                </a>
-                <div class="sidebar-collapsible">
-                    <a class="navbar-brand mr-0" href="javascript:void(0);">
-                        <h4 class="section-header-text mb-0">{{'menu.header.admin' |
-                            translate}}</h4>
-                    </a>
+                  </div>
+                  <div class="sidebar-collapsible navbar-brand">
+                    <div class="mr-0">
+                      <h4 class="section-header-text mb-0">{{ 'menu.header.admin' | translate }}</h4>
+                    </div>
+                  </div>
                 </div>
+              </div>
             </li>
 
             <ng-container *ngFor="let section of (sections | async)">
@@ -32,22 +34,22 @@
     </div>
     <div class="navbar-nav">
         <div class="sidebar-section" id="sidebar-collapse-toggle">
-            <a class="nav-item nav-link shortcut-icon"
+            <a class="nav-item nav-link sidebar-section d-flex flex-row flex-nowrap"
                href="javascript:void(0);"
-               (click)="toggle($event)">
+               (click)="toggle($event)"
+               (keyup.space)="toggle($event)"
+            >
+              <div class="shortcut-icon">
                 <i *ngIf="(menuCollapsed | async)" class="fas fa-fw fa-angle-double-right"
                    [title]="'menu.section.icon.pin' | translate"></i>
                 <i *ngIf="!(menuCollapsed | async)" class="fas fa-fw fa-angle-double-left"
                    [title]="'menu.section.icon.unpin' | translate"></i>
+              </div>
+              <div class="sidebar-collapsible">
+                <span *ngIf="menuCollapsed | async" class="section-header-text">{{'menu.section.pin' | translate }}</span>
+                <span *ngIf="!(menuCollapsed | async)" class="section-header-text">{{'menu.section.unpin' | translate }}</span>
+              </div>
             </a>
-            <div class="sidebar-collapsible">
-                <a class="nav-item nav-link sidebar-section"
-                   href="javascript:void(0);"
-                   (click)="toggle($event)">
-                    <span *ngIf="menuCollapsed | async" class="section-header-text">{{'menu.section.pin' | translate }}</span>
-                    <span *ngIf="!(menuCollapsed | async)" class="section-header-text">{{'menu.section.unpin' | translate }}</span>
-                </a>
-            </div>
         </div>
     </div>
 </nav>

src/app/admin/admin-sidebar/admin-sidebar.component.scss

@@ -25,6 +25,11 @@
         .navbar-nav {
             .admin-menu-header {
                 background-color: var(--ds-admin-sidebar-header-bg);
+
+                .sidebar-section {
+                  background-color: inherit;
+                }
+
                 .logo-wrapper {
                     img {
                         height: 20px;
@@ -34,6 +39,10 @@
                     line-height: 1.5;
                 }
 
+                .navbar-brand {
+                  margin-right: 0;
+                }
+
             }
         }
 
@@ -44,26 +53,64 @@
                     display: flex;
                     align-content: stretch;
                     background-color: var(--ds-admin-sidebar-bg);
+                    overflow-x: visible;
+
                     .nav-item {
                         padding-top: var(--bs-spacer);
                         padding-bottom: var(--bs-spacer);
+                        background-color: inherit;
+
+                      &:focus-visible {
+                        // since links fill the whole sidebar, we _inset_ the outline
+                        outline-offset: -4px;
+
+                        // replace padding with margins so it doesn't extend over the :focus-visible outline
+                        //  → can't remove the padding altogether; the icon needs to fill out
+                        //    the collapsed width of the sidebar for the slide animation to look decent.
+                        .shortcut-icon {
+                          padding-left: 0;
+                          padding-right: 0;
+                          margin-left: var(--ds-icon-padding);
+                          margin-right: var(--ds-icon-padding);
+                        }
+                        .logo-wrapper {
+                          margin-right: var(--bs-navbar-padding-x) !important;
+                        }
+                        .navbar-brand {
+                          padding-top: 0;
+                          padding-bottom: 0;
+                          margin-top: var(--bs-navbar-brand-padding-y);
+                          margin-bottom: var(--bs-navbar-brand-padding-y);
+                        }
+                      }
                     }
+
                     .shortcut-icon {
+                        background-color: inherit;
                         padding-left: var(--ds-icon-padding);
                         padding-right: var(--ds-icon-padding);
-                    }
-                    .shortcut-icon, .icon-wrapper {
-                        background-color: inherit;
                         z-index: var(--ds-icon-z-index);
+                        align-self: baseline;
                     }
+
                     .sidebar-collapsible {
+                        padding-left: 0;
+                        padding-right: var(--bs-spacer);
                         width: var(--ds-sidebar-items-width);
                         position: relative;
-                        a {
-                            padding-right: var(--bs-spacer);
-                            width: 100%;
+                        .toggle {
+                          width: 100%;
+                        }
+
+                        ul {
+                          padding-top: var(--bs-spacer);
+
+                          li a {
+                            padding-left: var(--bs-spacer);
+                          }
                         }
                     }
+
                     &.active > .sidebar-collapsible > .nav-link {
                         color: var(--bs-navbar-dark-active-color);
                     }

src/app/admin/admin-sidebar/admin-sidebar.component.spec.ts

@@ -113,25 +113,10 @@ describe('AdminSidebarComponent', () => {
     });
   });
 
-  describe('when the collapse icon is clicked', () => {
-    beforeEach(() => {
-      spyOn(menuService, 'toggleMenu');
-      const sidebarToggler = fixture.debugElement.query(By.css('#sidebar-collapse-toggle')).query(By.css('a.shortcut-icon'));
-      sidebarToggler.triggerEventHandler('click', {
-        preventDefault: () => {/**/
-        }
-      });
-    });
-
-    it('should call toggleMenu on the menuService', () => {
-      expect(menuService.toggleMenu).toHaveBeenCalled();
-    });
-  });
-
   describe('when the collapse link is clicked', () => {
     beforeEach(() => {
       spyOn(menuService, 'toggleMenu');
-      const sidebarToggler = fixture.debugElement.query(By.css('#sidebar-collapse-toggle')).query(By.css('.sidebar-collapsible')).query(By.css('a'));
+      const sidebarToggler = fixture.debugElement.query(By.css('#sidebar-collapse-toggle > a'));
       sidebarToggler.triggerEventHandler('click', {
         preventDefault: () => {/**/
         }

src/app/admin/admin-sidebar/admin-sidebar.component.ts

@@ -1,7 +1,7 @@
-import { Component, Injector, OnInit } from '@angular/core';
+import { Component, HostListener, Injector, OnInit } from '@angular/core';
 import { NgbModal } from '@ng-bootstrap/ng-bootstrap';
-import { combineLatest, combineLatest as observableCombineLatest, Observable } from 'rxjs';
-import { first, map, take } from 'rxjs/operators';
+import { combineLatest, combineLatest as observableCombineLatest, Observable, BehaviorSubject } from 'rxjs';
+import { debounceTime, first, map, take, distinctUntilChanged, withLatestFrom } from 'rxjs/operators';
 import { AuthService } from '../../core/auth/auth.service';
 import { ScriptDataService } from '../../core/data/processes/script-data.service';
 import { slideHorizontal, slideSidebar } from '../../shared/animations/slide';
@@ -60,6 +60,8 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit {
    */
   sidebarExpanded: Observable<boolean>;
 
+  inFocus$: BehaviorSubject<boolean>;
+
   constructor(protected menuService: MenuService,
               protected injector: Injector,
               private variableService: CSSVariableService,
@@ -69,6 +71,7 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit {
               private scriptDataService: ScriptDataService,
   ) {
     super(menuService, injector);
+    this.inFocus$ = new BehaviorSubject(false);
   }
 
   /**
@@ -89,10 +92,25 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit {
         this.sidebarOpen = !collapsed;
         this.sidebarClosed = collapsed;
       });
-    this.sidebarExpanded = observableCombineLatest(this.menuCollapsed, this.menuPreviewCollapsed)
+    this.sidebarExpanded = combineLatest([this.menuCollapsed, this.menuPreviewCollapsed])
       .pipe(
         map(([collapsed, previewCollapsed]) => (!collapsed || !previewCollapsed))
       );
+    this.inFocus$.pipe(
+      debounceTime(50),
+      distinctUntilChanged(),  // disregard focusout in situations like --(focusout)-(focusin)--
+      withLatestFrom(
+        combineLatest([this.menuCollapsed, this.menuPreviewCollapsed])
+      ),
+    ).subscribe(([inFocus, [collapsed, previewCollapsed]]) => {
+      if (collapsed) {
+        if (inFocus && previewCollapsed) {
+          this.expandPreview(new Event('focusin → expand'));
+        } else if (!inFocus && !previewCollapsed) {
+          this.collapsePreview(new Event('focusout → collapse'));
+        }
+      }
+    });
   }
 
   /**
@@ -590,6 +608,32 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit {
     });
   }
 
+  @HostListener('focusin')
+  public handleFocusIn() {
+    this.inFocus$.next(true);
+  }
+
+  @HostListener('focusout')
+  public handleFocusOut() {
+    this.inFocus$.next(false);
+  }
+
+  public handleMouseEnter(event: any) {
+    if (!this.inFocus$.getValue()) {
+      this.expandPreview(event);
+    } else {
+      event.preventDefault();
+    }
+  }
+
+  public handleMouseLeave(event: any) {
+    if (!this.inFocus$.getValue()) {
+      this.collapsePreview(event);
+    } else {
+      event.preventDefault();
+    }
+  }
+
   /**
    * Method to change this.collapsed to false when the slide animation ends and is sliding open
    * @param event The animation event

src/app/admin/admin-sidebar/expandable-admin-sidebar-section/expandable-admin-sidebar-section.component.html

@@ -1,27 +1,36 @@
-<li class="sidebar-section" [ngClass]="{'expanded': (expanded | async)}"
+<div class="sidebar-section" [ngClass]="{'expanded': (expanded | async)}"
     [@bgColor]="{
      value: ((expanded | async) ? 'endBackground' : 'startBackground'),
      params: {endColor: (sidebarActiveBg | async)}}">
-    <div class="icon-wrapper">
-        <a class="nav-item nav-link shortcut-icon" attr.aria.labelledby="sidebarName-{{section.id}}" [title]="('menu.section.icon.' + section.id) | translate" (click)="toggleSection($event)" href="javascript:void(0);">
-            <i class="fas fa-{{section.icon}} fa-fw"></i>
-        </a>
+  <div class="nav-item nav-link d-flex flex-row flex-nowrap"
+     role="button" tabindex="0"
+     [attr.aria-labelledby]="'sidebarName-' + section.id"
+     [attr.aria-expanded]="expanded | async"
+     [title]="('menu.section.icon.' + section.id) | translate"
+     (click)="toggleSection($event)"
+     (keyup.space)="toggleSection($event)"
+     (keyup.enter)="toggleSection($event)"
+  >
+    <div class="shortcut-icon h-100">
+      <i class="fas fa-{{section.icon}} fa-fw"></i>
     </div>
     <div class="sidebar-collapsible">
-        <a class="nav-item nav-link" href="javascript:void(0);" tabindex="-1"
-           (click)="toggleSection($event)">
-            <span id="sidebarName-{{section.id}}" class="section-header-text">
-                <ng-container
-                        *ngComponentOutlet="(sectionMap$ | async).get(section.id).component; injector: (sectionMap$ | async).get(section.id).injector;"></ng-container>
-            </span>
-            <i class="fas fa-chevron-right fa-pull-right"
-               [@rotate]="(expanded | async) ? 'expanded' : 'collapsed'" [title]="('menu.section.toggle.' + section.id) | translate"></i>
-        </a>
-        <ul class="sidebar-sub-level-items list-unstyled" @slide *ngIf="(expanded | async)">
-            <li *ngFor="let subSection of (subSections$ | async)">
-                <ng-container
-                        *ngComponentOutlet="(sectionMap$ | async).get(subSection.id).component; injector: (sectionMap$ | async).get(subSection.id).injector;"></ng-container>
-            </li>
-        </ul>
+      <div class="toggle">
+        <span id="sidebarName-{{section.id}}" class="section-header-text">
+          <ng-container
+            *ngComponentOutlet="(sectionMap$ | async).get(section.id).component; injector: (sectionMap$ | async).get(section.id).injector;"></ng-container>
+        </span>
+        <i class="fas fa-chevron-right fa-pull-right"
+           [@rotate]="(expanded | async) ? 'expanded' : 'collapsed'"
+           [title]="('menu.section.toggle.' + section.id) | translate"
+        ></i>
+      </div>
+      <ul class="sidebar-sub-level-items list-unstyled" @slide *ngIf="(expanded | async)">
+        <li *ngFor="let subSection of (subSections$ | async)">
+          <ng-container
+            *ngComponentOutlet="(sectionMap$ | async).get(subSection.id).component; injector: (sectionMap$ | async).get(subSection.id).injector;"></ng-container>
+        </li>
+      </ul>
     </div>
-</li>
+  </div>
+</div>

src/app/admin/admin-sidebar/expandable-admin-sidebar-section/expandable-admin-sidebar-section.component.scss

@@ -9,7 +9,7 @@
         list-style: disc;
         color: var(--bs-navbar-dark-color);
         overflow: hidden;
-
+        margin-bottom: calc(-1 * var(--bs-spacer)); // the bottom-most nav-item is padded, no need for double spacing
     }
 
     .sidebar-collapsible {

src/app/admin/admin-sidebar/expandable-admin-sidebar-section/expandable-admin-sidebar-section.component.spec.ts

@@ -10,6 +10,8 @@ import { Component } from '@angular/core';
 import { NoopAnimationsModule } from '@angular/platform-browser/animations';
 import { By } from '@angular/platform-browser';
 import { TranslateModule } from '@ngx-translate/core';
+import { Router } from '@angular/router';
+import { RouterStub } from '../../../shared/testing/router.stub';
 
 describe('ExpandableAdminSidebarSectionComponent', () => {
   let component: ExpandableAdminSidebarSectionComponent;
@@ -24,6 +26,7 @@ describe('ExpandableAdminSidebarSectionComponent', () => {
         { provide: 'sectionDataProvider', useValue: { icon: iconString } },
         { provide: MenuService, useValue: menuService },
         { provide: CSSVariableService, useClass: CSSVariableServiceStub },
+        { provide: Router, useValue: new RouterStub() },
       ]
     }).overrideComponent(ExpandableAdminSidebarSectionComponent, {
       set: {
@@ -46,29 +49,14 @@ describe('ExpandableAdminSidebarSectionComponent', () => {
   });
 
   it('should set the right icon', () => {
-    const icon = fixture.debugElement.query(By.css('.icon-wrapper')).query(By.css('i.fas'));
+    const icon = fixture.debugElement.query(By.css('.shortcut-icon > i.fas'));
     expect(icon.nativeElement.getAttribute('class')).toContain('fa-' + iconString);
   });
 
-  describe('when the icon is clicked', () => {
-    beforeEach(() => {
-      spyOn(menuService, 'toggleActiveSection');
-      const sidebarToggler = fixture.debugElement.query(By.css('a.shortcut-icon'));
-      sidebarToggler.triggerEventHandler('click', {
-        preventDefault: () => {/**/
-        }
-      });
-    });
-
-    it('should call toggleActiveSection on the menuService', () => {
-      expect(menuService.toggleActiveSection).toHaveBeenCalled();
-    });
-  });
-
   describe('when the header text is clicked', () => {
     beforeEach(() => {
       spyOn(menuService, 'toggleActiveSection');
-      const sidebarToggler = fixture.debugElement.query(By.css('.sidebar-collapsible')).query(By.css('a'));
+      const sidebarToggler = fixture.debugElement.query(By.css('.sidebar-section > div.nav-item'));
       sidebarToggler.triggerEventHandler('click', {
         preventDefault: () => {/**/
         }

src/app/admin/admin-sidebar/expandable-admin-sidebar-section/expandable-admin-sidebar-section.component.ts

@@ -9,12 +9,14 @@ import { MenuService } from '../../../shared/menu/menu.service';
 import { combineLatest as combineLatestObservable, Observable } from 'rxjs';
 import { map } from 'rxjs/operators';
 import { rendersSectionForMenu } from '../../../shared/menu/menu-section.decorator';
+import { Router } from '@angular/router';
 
 /**
  * Represents a expandable section in the sidebar
  */
 @Component({
-  selector: 'ds-expandable-admin-sidebar-section',
+  /* tslint:disable:component-selector */
+  selector: 'li[ds-expandable-admin-sidebar-section]',
   templateUrl: './expandable-admin-sidebar-section.component.html',
   styleUrls: ['./expandable-admin-sidebar-section.component.scss'],
   animations: [rotate, slide, bgColor]
@@ -48,9 +50,14 @@ export class ExpandableAdminSidebarSectionComponent extends AdminSidebarSectionC
    */
   expanded: Observable<boolean>;
 
-  constructor(@Inject('sectionDataProvider') menuSection, protected menuService: MenuService,
-              private variableService: CSSVariableService, protected injector: Injector) {
-    super(menuSection, menuService, injector);
+  constructor(
+    @Inject('sectionDataProvider') menuSection,
+    protected menuService: MenuService,
+    private variableService: CSSVariableService,
+    protected injector: Injector,
+    protected router: Router,
+  ) {
+    super(menuSection, menuService, injector, router);
   }
 
   /**

src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html

@@ -1,4 +1,4 @@
-<li class="nav-item dropdown"
+<div class="nav-item dropdown expandable-navbar-section"
     (keyup.enter)="activateSection($event)"
     (mouseenter)="activateSection($event)"
     (mouseleave)="deactivateSection($event)">
@@ -15,4 +15,4 @@
                     *ngComponentOutlet="(sectionMap$ | async).get(subSection.id).component; injector: (sectionMap$ | async).get(subSection.id).injector;"></ng-container>
         </ng-container>
     </ul>
-</li>
+</div>

src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts

@@ -49,7 +49,7 @@ describe('ExpandableNavbarSectionComponent', () => {
     describe('when the mouse enters the section header', () => {
       beforeEach(() => {
         spyOn(menuService, 'activateSection');
-        const sidebarToggler = fixture.debugElement.query(By.css('li.nav-item.dropdown'));
+        const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown'));
         sidebarToggler.triggerEventHandler('mouseenter', {
           preventDefault: () => {/**/
           }
@@ -64,7 +64,7 @@ describe('ExpandableNavbarSectionComponent', () => {
     describe('when the mouse leaves the section header', () => {
       beforeEach(() => {
         spyOn(menuService, 'deactivateSection');
-        const sidebarToggler = fixture.debugElement.query(By.css('li.nav-item.dropdown'));
+        const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown'));
         sidebarToggler.triggerEventHandler('mouseleave', {
           preventDefault: () => {/**/
           }
@@ -79,7 +79,7 @@ describe('ExpandableNavbarSectionComponent', () => {
     describe('when a click occurs on the section header', () => {
       beforeEach(() => {
         spyOn(menuService, 'toggleActiveSection');
-        const sidebarToggler = fixture.debugElement.query(By.css('li.nav-item.dropdown')).query(By.css('a'));
+        const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown > a'));
         sidebarToggler.triggerEventHandler('click', {
           preventDefault: () => {/**/
           }
@@ -122,7 +122,7 @@ describe('ExpandableNavbarSectionComponent', () => {
     describe('when the mouse enters the section header', () => {
       beforeEach(() => {
         spyOn(menuService, 'activateSection');
-        const sidebarToggler = fixture.debugElement.query(By.css('li.nav-item.dropdown'));
+        const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown > a'));
         sidebarToggler.triggerEventHandler('mouseenter', {
           preventDefault: () => {/**/
           }
@@ -137,7 +137,7 @@ describe('ExpandableNavbarSectionComponent', () => {
     describe('when the mouse leaves the section header', () => {
       beforeEach(() => {
         spyOn(menuService, 'deactivateSection');
-        const sidebarToggler = fixture.debugElement.query(By.css('li.nav-item.dropdown'));
+        const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown > a'));
         sidebarToggler.triggerEventHandler('mouseleave', {
           preventDefault: () => {/**/
           }
@@ -152,7 +152,7 @@ describe('ExpandableNavbarSectionComponent', () => {
     describe('when a click occurs on the section header link', () => {
       beforeEach(() => {
         spyOn(menuService, 'toggleActiveSection');
-        const sidebarToggler = fixture.debugElement.query(By.css('li.nav-item.dropdown')).query(By.css('a'));
+        const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown > a'));
         sidebarToggler.triggerEventHandler('click', {
           preventDefault: () => {/**/
           }

src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts

@@ -11,7 +11,8 @@ import { rendersSectionForMenu } from '../../shared/menu/menu-section.decorator'
  * Represents an expandable section in the navbar
  */
 @Component({
-  selector: 'ds-expandable-navbar-section',
+  /* tslint:disable:component-selector */
+  selector: 'li[ds-expandable-navbar-section]',
   templateUrl: './expandable-navbar-section.component.html',
   styleUrls: ['./expandable-navbar-section.component.scss'],
   animations: [slide]

src/app/navbar/navbar-section/navbar-section.component.html

@@ -1,4 +1,4 @@
-<li class="nav-item">
+<div class="nav-item navbar-section">
   <ng-container
           *ngComponentOutlet="(sectionMap$ | async).get(section.id).component; injector: (sectionMap$ | async).get(section.id).injector;"></ng-container>
-</li>
+</div>

src/app/navbar/navbar-section/navbar-section.component.ts

@@ -8,7 +8,8 @@ import { rendersSectionForMenu } from '../../shared/menu/menu-section.decorator'
  * Represents a non-expandable section in the navbar
  */
 @Component({
-  selector: 'ds-navbar-section',
+  /* tslint:disable:component-selector */
+  selector: 'li[ds-navbar-section]',
   templateUrl: './navbar-section.component.html',
   styleUrls: ['./navbar-section.component.scss']
 })

src/app/shared/menu/menu-item/link-menu-item.component.html

@@ -1 +1,10 @@
-<a href="javascript:void(0);" class="nav-item nav-link"  [ngClass]="{'disabled': !hasLink}" [routerLink]="getRouterLink()">{{item.text | translate}}</a>
+<a class="nav-item nav-link"
+   [ngClass]="{ 'disabled': !hasLink }"
+   [attr.aria-disabled]="!hasLink"
+   [title]="item.text | translate"
+   [routerLink]="getRouterLink()"
+   (click)="$event.stopPropagation()"
+   (keyup.space)="navigate($event)"
+   (keyup.enter)="navigate($event)"
+   href="javascript:void(0);"
+>{{item.text | translate}}</a>

src/app/shared/menu/menu-item/link-menu-item.component.spec.ts

@@ -5,6 +5,8 @@ import { By } from '@angular/platform-browser';
 import { LinkMenuItemComponent } from './link-menu-item.component';
 import { RouterLinkDirectiveStub } from '../../testing/router-link-directive.stub';
 import { environment } from '../../../../environments/environment';
+import { RouterStub } from '../../testing/router.stub';
+import { Router } from '@angular/router';
 
 describe('LinkMenuItemComponent', () => {
   let component: LinkMenuItemComponent;
@@ -25,6 +27,7 @@ describe('LinkMenuItemComponent', () => {
       declarations: [LinkMenuItemComponent, RouterLinkDirectiveStub],
       providers: [
         { provide: 'itemModelProvider', useValue: { text: text, link: link } },
+        { provide: Router, useValue: RouterStub },
       ],
       schemas: [NO_ERRORS_SCHEMA]
     })

src/app/shared/menu/menu-item/link-menu-item.component.ts

@@ -4,6 +4,7 @@ import { MenuItemType } from '../initial-menus-state';
 import { rendersMenuItemForType } from '../menu-item.decorator';
 import { isNotEmpty } from '../../empty.util';
 import { environment } from '../../../../environments/environment';
+import { Router } from '@angular/router';
 
 /**
  * Component that renders a menu section of type LINK
@@ -16,7 +17,10 @@ import { environment } from '../../../../environments/environment';
 export class LinkMenuItemComponent implements OnInit {
   item: LinkMenuItemModel;
   hasLink: boolean;
-  constructor(@Inject('itemModelProvider') item: LinkMenuItemModel) {
+  constructor(
+    @Inject('itemModelProvider') item: LinkMenuItemModel,
+    private router: Router,
+  ) {
     this.item = item;
   }
 
@@ -31,4 +35,12 @@ export class LinkMenuItemComponent implements OnInit {
     return undefined;
   }
 
+  navigate(event: any) {
+    event.preventDefault();
+    if (this.getRouterLink()) {
+      this.router.navigate([this.getRouterLink()]);
+    }
+    event.stopPropagation();
+  }
+
 }

src/app/shared/menu/menu-item/onclick-menu-item.component.html

@@ -1 +1,8 @@
-<a href="javascript:void(0);" class="nav-item nav-link" role="button" (click)="item.function()">{{item.text | translate}}</a>
+<a href="javascript:void(0);"
+   class="nav-item nav-link"
+   role="button"
+   [title]="item.text | translate"
+   (click)="activate($event)"
+   (keyup.space)="activate($event)"
+   (keyup.enter)="activate($event)"
+>{{item.text | translate}}</a>

src/app/shared/menu/menu-item/onclick-menu-item.component.spec.ts

@@ -44,8 +44,8 @@ describe('OnClickMenuItemComponent', () => {
     expect(textContent).toEqual(text);
   });
 
-  it('should contain call the function on the item when clicked', () => {
-    debugElement.query(By.css('a.nav-link')).triggerEventHandler('click', {});
+  it('should call the function on the item when clicked', () => {
+    debugElement.query(By.css('a.nav-link')).triggerEventHandler('click', new Event(('click')));
     expect(item.function).toHaveBeenCalled();
   });
 });

src/app/shared/menu/menu-item/onclick-menu-item.component.ts

@@ -17,4 +17,10 @@ export class OnClickMenuItemComponent {
   constructor(@Inject('itemModelProvider') item: OnClickMenuItemModel) {
     this.item = item;
   }
+
+  public activate(event: any) {
+    event.preventDefault();
+    this.item.function();
+    event.stopPropagation();
+  }
 }

src/themes/dspace/styles/_global-styles.scss

@@ -23,8 +23,8 @@
 }
 
 header {
-  ds-navbar-section > li,
-  ds-expandable-navbar-section > li {
+  li > .navbar-section,
+  li > .expandable-navbar-section {
     display: flex;
     flex-direction: column;
     justify-content: center;