Merge pull request #3919 from DSpace/backport-3581-to-dspace-8_x

[Port dspace-8_x] Made expandable navbar section more keyboard accessible
This commit is contained in:
Tim Donohue
2025-01-29 16:40:22 -06:00
committed by GitHub
7 changed files with 398 additions and 70 deletions

View File

@@ -84,7 +84,7 @@ export class ExpandableAdminSidebarSectionComponent extends AdminSidebarSectionC
this.sidebarActiveBg$ = this.variableService.getVariable('--ds-admin-sidebar-active-bg');
this.isSidebarCollapsed$ = this.menuService.isMenuCollapsed(this.menuID);
this.isSidebarPreviewCollapsed$ = this.menuService.isMenuPreviewCollapsed(this.menuID);
this.isExpanded$ = combineLatestObservable([this.active, this.isSidebarCollapsed$, this.isSidebarPreviewCollapsed$]).pipe(
this.isExpanded$ = combineLatestObservable([this.active$, this.isSidebarCollapsed$, this.isSidebarPreviewCollapsed$]).pipe(
map(([active, sidebarCollapsed, sidebarPreviewCollapsed]) => (active && (!sidebarCollapsed || !sidebarPreviewCollapsed))),
);
}

View File

@@ -1,35 +1,37 @@
<div class="ds-menu-item-wrapper text-md-center"
[id]="'expandable-navbar-section-' + section.id"
(mouseenter)="onMouseEnter($event, isActive)"
(mouseleave)="onMouseLeave($event, isActive)"
data-test="navbar-section-wrapper"
*ngVar="(active | async) as isActive">
(mouseenter)="onMouseEnter($event)"
(mouseleave)="onMouseLeave($event)"
data-test="navbar-section-wrapper">
<a href="javascript:void(0);" routerLinkActive="active"
role="menuitem"
(keyup.enter)="toggleSection($event)"
(keyup.space)="toggleSection($event)"
(click)="toggleSection($event)"
(keydown.space)="$event.preventDefault()"
(keydown)="keyDown($event)"
aria-haspopup="menu"
data-test="navbar-section-toggler"
[attr.aria-expanded]="isActive"
[attr.aria-controls]="expandableNavbarSectionId(section.id)"
[attr.aria-expanded]="(active$ | async).valueOf()"
[attr.aria-controls]="expandableNavbarSectionId()"
class="d-flex flex-row flex-nowrap align-items-center gapx-1 ds-menu-toggler-wrapper"
[class.disabled]="section.model?.disabled">
<span class="flex-fill">
<ng-container
*ngComponentOutlet="(sectionMap$ | async).get(section.id).component; injector: (sectionMap$ | async).get(section.id).injector;"></ng-container>
<!-- <span class="sr-only">{{'nav.expandable-navbar-section-suffix' | translate}}</span>-->
</span>
<i class="fas fa-caret-down fa-xs toggle-menu-icon" aria-hidden="true"></i>
</a>
<div @slide *ngIf="isActive" (click)="deactivateSection($event)"
[id]="expandableNavbarSectionId(section.id)"
<div *ngIf="(active$ | async).valueOf() === true" (click)="deactivateSection($event)"
[id]="expandableNavbarSectionId()"
[dsHoverOutsideOfParentSelector]="'#expandable-navbar-section-' + section.id"
(dsHoverOutside)="deactivateSection($event, false)"
role="menu"
class="dropdown-menu show nav-dropdown-menu m-0 shadow-none border-top-0 px-3 px-md-0 pt-0 pt-md-1">
<div @slide role="presentation">
<div *ngFor="let subSection of (subSections$ | async)" class="text-nowrap" role="presentation">
<ng-container
*ngComponentOutlet="(sectionMap$ | async).get(subSection.id).component; injector: (sectionMap$ | async).get(subSection.id).injector;"></ng-container>
</div>
</div>
</div>
</div>

View File

@@ -1,6 +1,11 @@
import { Component } from '@angular/core';
import {
Component,
DebugElement,
} from '@angular/core';
import {
ComponentFixture,
fakeAsync,
flush,
TestBed,
waitForAsync,
} from '@angular/core/testing';
@@ -10,9 +15,11 @@ import { of as observableOf } from 'rxjs';
import { HostWindowService } from '../../shared/host-window.service';
import { MenuService } from '../../shared/menu/menu.service';
import { LinkMenuItemModel } from '../../shared/menu/menu-item/models/link.model';
import { MenuSection } from '../../shared/menu/menu-section.model';
import { HostWindowServiceStub } from '../../shared/testing/host-window-service.stub';
import { MenuServiceStub } from '../../shared/testing/menu-service.stub';
import { VarDirective } from '../../shared/utils/var.directive';
import { HoverOutsideDirective } from '../../shared/utils/hover-outside.directive';
import { ExpandableNavbarSectionComponent } from './expandable-navbar-section.component';
describe('ExpandableNavbarSectionComponent', () => {
@@ -23,11 +30,17 @@ describe('ExpandableNavbarSectionComponent', () => {
describe('on larger screens', () => {
beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
imports: [NoopAnimationsModule, ExpandableNavbarSectionComponent, TestComponent, VarDirective],
imports: [
ExpandableNavbarSectionComponent,
HoverOutsideDirective,
NoopAnimationsModule,
TestComponent,
],
providers: [
{ provide: 'sectionDataProvider', useValue: {} },
{ provide: MenuService, useValue: menuService },
{ provide: HostWindowService, useValue: new HostWindowServiceStub(800) },
TestComponent,
],
}).compileComponents();
}));
@@ -41,10 +54,6 @@ describe('ExpandableNavbarSectionComponent', () => {
fixture.detectChanges();
});
it('should create', () => {
expect(component).toBeTruthy();
});
describe('when the mouse enters the section header (while inactive)', () => {
beforeEach(() => {
spyOn(component, 'onMouseEnter').and.callThrough();
@@ -141,6 +150,8 @@ describe('ExpandableNavbarSectionComponent', () => {
});
describe('when spacebar is pressed on section header (while inactive)', () => {
let sidebarToggler: DebugElement;
beforeEach(() => {
spyOn(component, 'toggleSection').and.callThrough();
spyOn(menuService, 'toggleActiveSection');
@@ -149,15 +160,27 @@ describe('ExpandableNavbarSectionComponent', () => {
component.ngOnInit();
fixture.detectChanges();
const sidebarToggler = fixture.debugElement.query(By.css('[data-test="navbar-section-toggler"]'));
// dispatch the (keyup.space) action used in our component HTML
sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: ' ' }));
sidebarToggler = fixture.debugElement.query(By.css('[data-test="navbar-section-toggler"]'));
});
it('should call toggleSection on the menuService', () => {
// dispatch the (keyup.space) action used in our component HTML
sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { code: 'Space', key: ' ' }));
expect(component.toggleSection).toHaveBeenCalled();
expect(menuService.toggleActiveSection).toHaveBeenCalled();
});
// Should not do anything in order to work correctly with NVDA: https://www.nvaccess.org/
it('should not do anything on keydown space', () => {
const event: Event = new KeyboardEvent('keydown', { code: 'Space', key: ' ' });
spyOn(event, 'preventDefault').and.callThrough();
// dispatch the (keyup.space) action used in our component HTML
sidebarToggler.nativeElement.dispatchEvent(event);
expect(event.preventDefault).toHaveBeenCalled();
});
});
describe('when spacebar is pressed on section header (while active)', () => {
@@ -179,12 +202,116 @@ describe('ExpandableNavbarSectionComponent', () => {
expect(menuService.toggleActiveSection).toHaveBeenCalled();
});
});
describe('when enter is pressed on section header (while inactive)', () => {
let sidebarToggler: DebugElement;
beforeEach(() => {
spyOn(component, 'toggleSection').and.callThrough();
spyOn(menuService, 'toggleActiveSection');
// Make sure section is 'inactive'. Requires calling ngOnInit() to update component 'active' property.
spyOn(menuService, 'isSectionActive').and.returnValue(observableOf(false));
component.ngOnInit();
fixture.detectChanges();
sidebarToggler = fixture.debugElement.query(By.css('[data-test="navbar-section-toggler"]'));
});
// Should not do anything in order to work correctly with NVDA: https://www.nvaccess.org/
it('should not do anything on keydown space', () => {
const event: Event = new KeyboardEvent('keydown', { code: 'Enter' });
spyOn(event, 'preventDefault').and.callThrough();
// dispatch the (keyup.space) action used in our component HTML
sidebarToggler.nativeElement.dispatchEvent(event);
expect(event.preventDefault).toHaveBeenCalled();
});
});
describe('when arrow down is pressed on section header', () => {
it('should call activateSection', () => {
spyOn(component, 'activateSection').and.callThrough();
const sidebarToggler: DebugElement = fixture.debugElement.query(By.css('[data-test="navbar-section-toggler"]'));
// dispatch the (keydown.ArrowDown) action used in our component HTML
sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { code: 'ArrowDown' }));
expect(component.focusOnFirstChildSection).toBe(true);
expect(component.activateSection).toHaveBeenCalled();
});
});
describe('when tab is pressed on section header', () => {
it('should call deactivateSection', () => {
spyOn(component, 'deactivateSection').and.callThrough();
const sidebarToggler: DebugElement = fixture.debugElement.query(By.css('[data-test="navbar-section-toggler"]'));
// dispatch the (keydown.ArrowDown) action used in our component HTML
sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { code: 'Tab' }));
expect(component.deactivateSection).toHaveBeenCalled();
});
});
describe('navigateDropdown', () => {
beforeEach(fakeAsync(() => {
jasmine.getEnv().allowRespy(true);
spyOn(menuService, 'getSubSectionsByParentID').and.returnValue(observableOf([
Object.assign(new MenuSection(), {
id: 'subSection1',
model: Object.assign(new LinkMenuItemModel(), {
type: 'TEST_LINK',
}),
parentId: component.section.id,
}),
Object.assign(new MenuSection(), {
id: 'subSection2',
model: Object.assign(new LinkMenuItemModel(), {
type: 'TEST_LINK',
}),
parentId: component.section.id,
}),
]));
component.ngOnInit();
flush();
fixture.detectChanges();
component.focusOnFirstChildSection = true;
component.active$.next(true);
fixture.detectChanges();
}));
it('should close the modal on Tab', () => {
spyOn(menuService, 'deactivateSection').and.callThrough();
const firstSubsection: DebugElement = fixture.debugElement.queryAll(By.css('.dropdown-menu a[role="menuitem"]'))[0];
firstSubsection.nativeElement.focus();
firstSubsection.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { code: 'Tab' }));
expect(menuService.deactivateSection).toHaveBeenCalled();
});
it('should close the modal on Escape', () => {
spyOn(menuService, 'deactivateSection').and.callThrough();
const firstSubsection: DebugElement = fixture.debugElement.queryAll(By.css('.dropdown-menu a[role="menuitem"]'))[0];
firstSubsection.nativeElement.focus();
firstSubsection.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { code: 'Escape' }));
expect(menuService.deactivateSection).toHaveBeenCalled();
});
});
});
describe('on smaller, mobile screens', () => {
beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
imports: [NoopAnimationsModule, ExpandableNavbarSectionComponent, TestComponent, VarDirective],
imports: [
ExpandableNavbarSectionComponent,
HoverOutsideDirective,
NoopAnimationsModule,
TestComponent,
],
providers: [
{ provide: 'sectionDataProvider', useValue: {} },
{ provide: MenuService, useValue: menuService },
@@ -253,7 +380,9 @@ describe('ExpandableNavbarSectionComponent', () => {
// declare a test component
@Component({
selector: 'ds-test-cmp',
template: ``,
template: `
<a role="menuitem">link</a>
`,
standalone: true,
})
class TestComponent {

View File

@@ -5,10 +5,12 @@ import {
NgIf,
} from '@angular/common';
import {
AfterViewChecked,
Component,
HostListener,
Inject,
Injector,
OnDestroy,
OnInit,
} from '@angular/core';
import { RouterLinkActive } from '@angular/router';
@@ -19,7 +21,8 @@ import { slide } from '../../shared/animations/slide';
import { HostWindowService } from '../../shared/host-window.service';
import { MenuService } from '../../shared/menu/menu.service';
import { MenuID } from '../../shared/menu/menu-id.model';
import { VarDirective } from '../../shared/utils/var.directive';
import { MenuSection } from '../../shared/menu/menu-section.model';
import { HoverOutsideDirective } from '../../shared/utils/hover-outside.directive';
import { NavbarSectionComponent } from '../navbar-section/navbar-section.component';
/**
@@ -31,9 +34,17 @@ import { NavbarSectionComponent } from '../navbar-section/navbar-section.compone
styleUrls: ['./expandable-navbar-section.component.scss'],
animations: [slide],
standalone: true,
imports: [VarDirective, RouterLinkActive, NgComponentOutlet, NgIf, NgFor, AsyncPipe],
imports: [
AsyncPipe,
HoverOutsideDirective,
NgComponentOutlet,
NgFor,
NgIf,
RouterLinkActive,
],
})
export class ExpandableNavbarSectionComponent extends NavbarSectionComponent implements OnInit {
export class ExpandableNavbarSectionComponent extends NavbarSectionComponent implements AfterViewChecked, OnInit, OnDestroy {
/**
* This section resides in the Public Navbar
*/
@@ -44,6 +55,11 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp
*/
mouseEntered = false;
/**
* Whether the section was expanded
*/
focusOnFirstChildSection = false;
/**
* True if screen size was small before a resize event
*/
@@ -54,6 +70,18 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp
*/
isMobile$: Observable<boolean>;
/**
* Boolean used to add the event listeners to the items in the expandable menu when expanded. This is done for
* performance reasons, there is currently an *ngIf on the menu to prevent the {@link HoverOutsideDirective} to tank
* performance when not expanded.
*/
addArrowEventListeners = false;
/**
* List of current dropdown items who have event listeners
*/
private dropdownItems: NodeListOf<HTMLElement>;
@HostListener('window:resize', ['$event'])
onResize() {
this.isMobile$.pipe(
@@ -68,29 +96,80 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp
});
}
constructor(@Inject('sectionDataProvider') menuSection,
constructor(
@Inject('sectionDataProvider') public section: MenuSection,
protected menuService: MenuService,
protected injector: Injector,
private windowService: HostWindowService,
protected windowService: HostWindowService,
) {
super(menuSection, menuService, injector);
super(section, menuService, injector);
this.isMobile$ = this.windowService.isMobile();
}
ngOnInit() {
super.ngOnInit();
this.subs.push(this.active$.subscribe((active: boolean) => {
if (active === true) {
this.addArrowEventListeners = true;
} else {
this.focusOnFirstChildSection = undefined;
this.unsubscribeFromEventListeners();
}
}));
}
ngAfterViewChecked(): void {
if (this.addArrowEventListeners) {
this.dropdownItems = document.querySelectorAll(`#${this.expandableNavbarSectionId()} *[role="menuitem"]`);
this.dropdownItems.forEach((item: HTMLElement) => {
item.addEventListener('keydown', this.navigateDropdown.bind(this));
});
if (this.focusOnFirstChildSection && this.dropdownItems.length > 0) {
this.dropdownItems.item(0).focus();
}
this.addArrowEventListeners = false;
}
}
ngOnDestroy(): void {
super.ngOnDestroy();
this.unsubscribeFromEventListeners();
}
/**
* Activate this section if it's currently inactive, deactivate it when it's currently active.
* Also saves whether this toggle was performed by a keyboard event (non-click event) in order to know if thi first
* item should be focussed when activating a section.
*
* @param {Event} event The user event that triggered this method
*/
override toggleSection(event: Event): void {
this.focusOnFirstChildSection = event.type !== 'click';
super.toggleSection(event);
}
/**
* Removes all the current event listeners on the dropdown items (called when the menu is closed & on component
* destruction)
*/
unsubscribeFromEventListeners(): void {
if (this.dropdownItems) {
this.dropdownItems.forEach((item: HTMLElement) => {
item.removeEventListener('keydown', this.navigateDropdown.bind(this));
});
this.dropdownItems = undefined;
}
}
/**
* When the mouse enters the section toggler activate the menu section
* @param $event
* @param isActive
*/
onMouseEnter($event: Event, isActive: boolean) {
onMouseEnter($event: Event): void {
this.isMobile$.pipe(
first(),
).subscribe((isMobile) => {
if (!isMobile && !isActive && !this.mouseEntered) {
if (!isMobile && !this.active$.value && !this.mouseEntered) {
this.activateSection($event);
}
this.mouseEntered = true;
@@ -100,13 +179,12 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp
/**
* When the mouse leaves the section toggler deactivate the menu section
* @param $event
* @param isActive
*/
onMouseLeave($event: Event, isActive: boolean) {
onMouseLeave($event: Event): void {
this.isMobile$.pipe(
first(),
).subscribe((isMobile) => {
if (!isMobile && isActive && this.mouseEntered) {
if (!isMobile && this.active$.value && this.mouseEntered) {
this.deactivateSection($event);
}
this.mouseEntered = false;
@@ -115,9 +193,60 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp
/**
* returns the ID of the DOM element representing the navbar section
* @param sectionId
*/
expandableNavbarSectionId(sectionId: string) {
return `expandable-navbar-section-${sectionId}-dropdown`;
expandableNavbarSectionId(): string {
return `expandable-navbar-section-${this.section.id}-dropdown`;
}
/**
* Handles the navigation between the menu items
*
* @param event
*/
navigateDropdown(event: KeyboardEvent): void {
if (event.code === 'Tab') {
this.deactivateSection(event, false);
return;
} else if (event.code === 'Escape') {
this.deactivateSection(event, false);
(document.querySelector(`a[aria-controls="${this.expandableNavbarSectionId()}"]`) as HTMLElement)?.focus();
return;
}
event.preventDefault();
event.stopPropagation();
const items: NodeListOf<Element> = document.querySelectorAll(`#${this.expandableNavbarSectionId()} *[role="menuitem"]`);
if (items.length === 0) {
return;
}
const currentIndex: number = Array.from(items).findIndex((item: Element) => item === event.target);
if (event.key === 'ArrowDown') {
(items[(currentIndex + 1) % items.length] as HTMLElement).focus();
} else if (event.key === 'ArrowUp') {
(items[(currentIndex - 1 + items.length) % items.length] as HTMLElement).focus();
}
}
/**
* Handles all the keydown events on the dropdown toggle
*
* @param event
*/
keyDown(event: KeyboardEvent): void {
switch (event.code) {
// Works for both Tab & Shift Tab
case 'Tab':
this.deactivateSection(event, false);
break;
case 'ArrowDown':
this.focusOnFirstChildSection = true;
this.activateSection(event);
break;
case 'Space':
case 'Enter':
event.preventDefault();
break;
}
}
}

View File

@@ -37,9 +37,9 @@ import { MenuSection } from '../menu-section.model';
export class MenuSectionComponent implements OnInit, OnDestroy {
/**
* Observable that emits whether or not this section is currently active
* {@link BehaviorSubject} containing the current state to whether this section is currently active
*/
active: Observable<boolean>;
active$: BehaviorSubject<boolean> = new BehaviorSubject(false);
/**
* The ID of the menu this section resides in
@@ -72,7 +72,11 @@ export class MenuSectionComponent implements OnInit, OnDestroy {
* Set initial values for instance variables
*/
ngOnInit(): void {
this.active = this.menuService.isSectionActive(this.menuID, this.section.id).pipe(distinctUntilChanged());
this.subs.push(this.menuService.isSectionActive(this.menuID, this.section.id).pipe(distinctUntilChanged()).subscribe((isActive: boolean) => {
if (this.active$.value !== isActive) {
this.active$.next(isActive);
}
}));
this.initializeInjectorData();
}
@@ -90,9 +94,12 @@ export class MenuSectionComponent implements OnInit, OnDestroy {
/**
* Activate this section
* @param {Event} event The user event that triggered this method
* @param skipEvent Weather the event should still be triggered after deactivating the section or not
*/
activateSection(event: Event) {
activateSection(event: Event, skipEvent = true): void {
if (skipEvent) {
event.preventDefault();
}
if (!this.section.model?.disabled) {
this.menuService.activateSection(this.menuID, this.section.id);
}
@@ -100,10 +107,14 @@ export class MenuSectionComponent implements OnInit, OnDestroy {
/**
* Deactivate this section
*
* @param {Event} event The user event that triggered this method
* @param skipEvent Weather the event should still be triggered after deactivating the section or not
*/
deactivateSection(event: Event) {
deactivateSection(event: Event, skipEvent = true): void {
if (skipEvent) {
event.preventDefault();
}
this.menuService.deactivateSection(this.menuID, this.section.id);
}

View File

@@ -0,0 +1,53 @@
import {
Directive,
ElementRef,
EventEmitter,
HostListener,
Input,
Output,
} from '@angular/core';
/**
* Directive to detect when the user hovers outside the element the directive was put on
*
* **Performance Consideration**: it's probably not good for performance to use this excessively (on
* {@link ExpandableNavbarSectionComponent} for example, a workaround for this problem was to add an `*ngIf` to prevent
* this Directive from always being active)
*/
@Directive({
selector: '[dsHoverOutside]',
standalone: true,
})
export class HoverOutsideDirective {
/**
* Emits null when the user hovers outside of the element
*/
@Output()
public dsHoverOutside = new EventEmitter();
/**
* CSS selector for the parent element to monitor. If set, the directive will use this
* selector to determine if the hover event originated within the selected parent element.
* If left unset, the directive will monitor mouseover hover events for the element it is applied to.
*/
@Input()
public dsHoverOutsideOfParentSelector: string;
constructor(
private elementRef: ElementRef,
) {
}
@HostListener('document:mouseover', ['$event'])
public onMouseOver(event: MouseEvent): void {
const targetElement: HTMLElement = event.target as HTMLElement;
const element: Element = document.querySelector(this.dsHoverOutsideOfParentSelector);
const hoveredInside = (element ? new ElementRef(element) : this.elementRef).nativeElement.contains(targetElement);
if (!hoveredInside) {
this.dsHoverOutside.emit(null);
}
}
}

View File

@@ -9,11 +9,8 @@ import { RouterLinkActive } from '@angular/router';
import { ExpandableNavbarSectionComponent as BaseComponent } from '../../../../../app/navbar/expandable-navbar-section/expandable-navbar-section.component';
import { slide } from '../../../../../app/shared/animations/slide';
import { VarDirective } from '../../../../../app/shared/utils/var.directive';
import { HoverOutsideDirective } from '../../../../../app/shared/utils/hover-outside.directive';
/**
* Represents an expandable section in the navbar
*/
@Component({
selector: 'ds-themed-expandable-navbar-section',
// templateUrl: './expandable-navbar-section.component.html',
@@ -22,7 +19,14 @@ import { VarDirective } from '../../../../../app/shared/utils/var.directive';
styleUrls: ['../../../../../app/navbar/expandable-navbar-section/expandable-navbar-section.component.scss'],
animations: [slide],
standalone: true,
imports: [VarDirective, RouterLinkActive, NgComponentOutlet, NgIf, NgFor, AsyncPipe],
imports: [
AsyncPipe,
HoverOutsideDirective,
NgComponentOutlet,
NgFor,
NgIf,
RouterLinkActive,
],
})
export class ExpandableNavbarSectionComponent extends BaseComponent {
}