Skip to content

Commit d12b23b

Browse files
alexandrevryghemJoran De Braekeleer
alexandrevryghem
authored and
Joran De Braekeleer
committed
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
1 parent bf585e7 commit d12b23b

File tree

9 files changed

+77
-63
lines changed

9 files changed

+77
-63
lines changed

‎cypress/e2e/header.cy.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,24 +15,24 @@ describe('Header', () => {
1515
cy.visit('/');
1616

1717
// Click the language switcher (globe) in header
18-
cy.get('a[data-test="lang-switch"]').click();
18+
cy.get('button[data-test="lang-switch"]').click();
1919
// Click on the "Deusch" language in dropdown
20-
cy.get('#language-menu-list li').contains('Deutsch').click();
20+
cy.get('#language-menu-list div[role="option"]').contains('Deutsch').click();
2121

2222
// HTML "lang" attribute should switch to "de"
2323
cy.get('html').invoke('attr', 'lang').should('eq', 'de');
2424

2525
// Login menu should now be in German
26-
cy.get('a[data-test="login-menu"]').contains('Anmelden');
26+
cy.get('[data-test="login-menu"]').contains('Anmelden');
2727

2828
// Change back to English from language switcher
29-
cy.get('a[data-test="lang-switch"]').click();
30-
cy.get('#language-menu-list li').contains('English').click();
29+
cy.get('button[data-test="lang-switch"]').click();
30+
cy.get('#language-menu-list div[role="option"]').contains('English').click();
3131

3232
// HTML "lang" attribute should switch to "en"
3333
cy.get('html').invoke('attr', 'lang').should('eq', 'en');
3434

3535
// Login menu should now be in English
36-
cy.get('a[data-test="login-menu"]').contains('Log In');
36+
cy.get('[data-test="login-menu"]').contains('Log In');
3737
});
3838
});

‎src/app/header/header.component.scss‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
}
2424
}
2525

26-
.navbar {
26+
.navbar, div[role="toolbar"] {
2727
display: flex;
2828
gap: calc(var(--bs-spacer) / 3);
2929
align-items: center;

‎src/app/navbar/navbar.component.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
<ng-container *ngIf="(isMobile$ | async) && (isAuthenticated$ | async)">
1010
<ds-themed-user-menu [inExpandableNavbar]="true"></ds-themed-user-menu>
1111
</ng-container>
12-
<div class="navbar-nav align-items-md-center mr-auto shadow-none gapx-3">
12+
<div class="navbar-nav align-items-md-center mr-auto shadow-none gapx-3" role="menubar">
1313
<ng-container *ngFor="let section of (sections | async)">
1414
<ng-container
1515
*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‎

Lines changed: 56 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,54 +1,63 @@
1-
<div class="navbar-nav mr-auto" *ngIf="!(isMobile$ | async); else mobileButtons" data-test="auth-nav">
2-
<div *ngIf="!(isAuthenticated | async) && (showAuth | async)"
3-
class="nav-item"
4-
(click)="$event.stopPropagation();">
5-
<div ngbDropdown #loginDrop="ngbDropdown" display="dynamic" placement="bottom-right" class="d-inline-block" @fadeInOut>
6-
<a href="javascript:void(0);" class="dropdownLogin px-0.5" [attr.aria-label]="'nav.login' |translate"
7-
(click)="$event.preventDefault()" [attr.data-test]="'login-menu' | dsBrowserOnly"
8-
role="menuitem"
9-
tabindex="0"
10-
aria-haspopup="menu"
11-
aria-controls="loginDropdownMenu"
12-
[attr.aria-expanded]="loginDrop.isOpen()"
13-
ngbDropdownToggle>{{ 'nav.login' | translate }}</a>
14-
<div id="loginDropdownMenu" [ngClass]="{'pl-3 pr-3': (loading | async)}" ngbDropdownMenu
15-
role="menu"
16-
[attr.aria-label]="'nav.login' | translate">
17-
<ds-themed-log-in
18-
[isStandalonePage]="false"></ds-themed-log-in>
1+
<div *ngVar="(isAuthenticated$ | async) as isAuthenticated">
2+
<div class="navbar-nav mr-auto" *ngIf="!(isMobile$ | async); else mobileButtons" data-test="auth-nav">
3+
<div *ngVar="(showAuth$ | async) as showAuth">
4+
<div *ngIf="!isAuthenticated && showAuth"
5+
class="nav-item"
6+
(click)="$event.stopPropagation();">
7+
<div ngbDropdown #loginDrop="ngbDropdown" display="dynamic" placement="bottom-right" class="d-inline-block"
8+
@fadeInOut>
9+
<button class="dropdownLogin btn btn-link px-0" [attr.aria-label]="'nav.login' |translate"
10+
(click)="$event.preventDefault()" [attr.data-test]="'login-menu' | dsBrowserOnly"
11+
role="button"
12+
tabindex="0"
13+
aria-haspopup="menu"
14+
aria-controls="loginDropdownMenu"
15+
[attr.aria-expanded]="loginDrop.isOpen()"
16+
ngbDropdownToggle>
17+
{{ 'nav.login' | translate }}
18+
</button>
19+
<div id="loginDropdownMenu" [ngClass]="{'pl-3 pr-3': (loading | async)}" ngbDropdownMenu
20+
role="dialog"
21+
aria-modal="true"
22+
[attr.aria-label]="'nav.login' | translate">
23+
<ds-themed-log-in
24+
[isStandalonePage]="false"></ds-themed-log-in>
25+
</div>
26+
</div>
1927
</div>
20-
</div>
21-
</div>
22-
<div *ngIf="(isAuthenticated | async) && (showAuth | async)" class="nav-item">
23-
<div ngbDropdown display="dynamic" placement="bottom-right" class="d-inline-block" @fadeInOut>
24-
<a href="javascript:void(0);"
25-
role="menuitem"
26-
tabindex="0"
27-
[attr.aria-label]="'nav.user-profile-menu-and-logout' | translate"
28-
aria-controls="user-menu-dropdown"
29-
(click)="$event.preventDefault()" [title]="'nav.user-profile-menu-and-logout' | translate"
30-
class="dropdownLogout px-1"
31-
[attr.data-test]="'user-menu' | dsBrowserOnly"
32-
ngbDropdownToggle>
33-
<i class="fas fa-user-circle fa-lg fa-fw"></i></a>
34-
<div id="logoutDropdownMenu" ngbDropdownMenu>
35-
<ds-themed-user-menu [inExpandableNavbar]="false"></ds-themed-user-menu>
28+
<div *ngIf="isAuthenticated && showAuth" class="nav-item">
29+
<div ngbDropdown display="dynamic" placement="bottom-right" class="d-inline-block" @fadeInOut>
30+
<button
31+
role="button"
32+
tabindex="0"
33+
[attr.aria-label]="'nav.user-profile-menu-and-logout' | translate"
34+
aria-controls="user-menu-dropdown"
35+
(click)="$event.preventDefault()" [title]="'nav.user-profile-menu-and-logout' | translate"
36+
class="dropdownLogout btn btn-link px-0"
37+
[attr.data-test]="'user-menu' | dsBrowserOnly"
38+
ngbDropdownToggle>
39+
<i class="fas fa-user-circle fa-lg fa-fw"></i></button>
40+
<div id="logoutDropdownMenu" ngbDropdownMenu>
41+
<ds-user-menu [inExpandableNavbar]="false"></ds-user-menu>
42+
</div>
43+
</div>
3644
</div>
3745
</div>
3846
</div>
47+
<ng-template #mobileButtons>
48+
<div data-test="auth-nav">
49+
<a *ngIf="!isAuthenticated" routerLink="/login" routerLinkActive="active" class="loginLink px-0.5" role="link"
50+
tabindex="0">
51+
{{ 'nav.login' | translate }}<span class="sr-only">(current)</span>
52+
</a>
53+
<a *ngIf="isAuthenticated" role="link" [attr.aria-label]="'nav.logout' |translate"
54+
[title]="'nav.logout' | translate" routerLink="/logout" routerLinkActive="active" class="logoutLink px-1"
55+
tabindex="0">
56+
<i class="fas fa-sign-out-alt fa-lg fa-fw"></i>
57+
<span class="sr-only">(current)</span>
58+
</a>
59+
</div>
60+
</ng-template>
3961
</div>
4062

41-
42-
<ng-template #mobileButtons>
43-
<div data-test="auth-nav">
44-
<a *ngIf="!(isAuthenticated | async)" routerLink="/login" routerLinkActive="active" class="loginLink px-0.5" role="button" tabindex="0">
45-
{{ 'nav.login' | translate }}<span class="sr-only">(current)</span>
46-
</a>
47-
<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">
48-
<i class="fas fa-sign-out-alt fa-lg fa-fw"></i>
49-
<span class="sr-only">(current)</span>
50-
</a>
51-
</div>
52-
</ng-template>
53-
5463
<!-- 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‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,7 @@
2828
box-shadow: unset;
2929
}
3030
}
31+
32+
.dropdown-toggle::after {
33+
margin-left: 0;
34+
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export class AuthNavMenuComponent implements OnInit {
2424
* Whether user is authenticated.
2525
* @type {Observable<string>}
2626
*/
27-
public isAuthenticated: Observable<boolean>;
27+
public isAuthenticated$: Observable<boolean>;
2828

2929
/**
3030
* True if the authentication is loading.
@@ -34,7 +34,7 @@ export class AuthNavMenuComponent implements OnInit {
3434

3535
public isMobile$: Observable<boolean>;
3636

37-
public showAuth = observableOf(false);
37+
public showAuth$ = observableOf(false);
3838

3939
public user: Observable<EPerson>;
4040

@@ -49,14 +49,14 @@ export class AuthNavMenuComponent implements OnInit {
4949

5050
ngOnInit(): void {
5151
// set isAuthenticated
52-
this.isAuthenticated = this.store.pipe(select(isAuthenticated));
52+
this.isAuthenticated$ = this.store.pipe(select(isAuthenticated));
5353

5454
// set loading
5555
this.loading = this.store.pipe(select(isAuthenticationLoading));
5656

5757
this.user = this.authService.getAuthenticatedUserFromStore();
5858

59-
this.showAuth = this.store.pipe(
59+
this.showAuth$ = this.store.pipe(
6060
select(routerStateSelector),
6161
filter((router: RouterReducerState) => isNotUndefined(router) && isNotUndefined(router.state)),
6262
map((router: RouterReducerState) => (!router.state.url.startsWith(LOGIN_ROUTE)

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,15 @@
33
[attr.aria-label]="'nav.language' |translate"
44
aria-controls="language-menu-list"
55
aria-haspopup="menu"
6-
[title]="'nav.language' | translate" class="dropdown-toggle btn btn-link px-1"
6+
[title]="'nav.language' | translate" class="dropdown-toggle btn btn-link px-0"
77
(click)="$event.preventDefault()" data-toggle="dropdown" ngbDropdownToggle
88
data-test="lang-switch"
99
tabindex="0">
1010
<i class="fas fa-globe-asia fa-lg fa-fw"></i>
1111
</button>
1212
<div ngbDropdownMenu class="dropdown-menu" role="listbox" [attr.aria-label]="'nav.language' |translate" id="language-menu-list">
1313
<div class="dropdown-item" tabindex="0" role="option" #langSelect *ngFor="let lang of translate.getLangs()"
14+
[lang]="lang"
1415
(keyup.enter)="useLang(lang)"
1516
(click)="useLang(lang)"
1617
[attr.aria-selected]="lang === translate.currentLang"

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@
2828
</form>
2929

3030
<div class="mt-2">
31-
<a class="dropdown-item" *ngIf="canRegister$ | async" [routerLink]="[getRegisterRoute()]" [attr.data-test]="'register' | dsBrowserOnly" role="menuitem" tabindex="0">
31+
<a class="dropdown-item" *ngIf="canRegister$ | async" [routerLink]="[getRegisterRoute()]" [attr.data-test]="'register' | dsBrowserOnly" tabindex="0">
3232
{{ 'login.form.new-user' | translate }}
3333
</a>
34-
<a class="dropdown-item" [routerLink]="[getForgotRoute()]" [attr.data-test]="'forgot' | dsBrowserOnly" role="menuitem" tabindex="0">
34+
<a class="dropdown-item" [routerLink]="[getForgotRoute()]" [attr.data-test]="'forgot' | dsBrowserOnly" tabindex="0">
3535
{{ 'login.form.forgot-password' | translate }}
3636
</a>
3737
</div>

‎src/themes/dspace/app/header/header.component.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
<!-- Search bar and other menus -->
1616
<div id="header-right" class="h-100 d-flex flex-row flex-nowrap justify-content-end align-items-center gapx-1 ml-auto">
1717
<ds-themed-search-navbar></ds-themed-search-navbar>
18-
<div role="menubar" class="h-100 d-flex flex-row flex-nowrap align-items-center gapx-1">
18+
<div role="toolbar" class="h-100 d-flex flex-row flex-nowrap align-items-center gapx-1">
1919
<ds-themed-lang-switch></ds-themed-lang-switch>
2020
<ds-context-help-toggle></ds-context-help-toggle>
2121
<ds-impersonate-navbar></ds-impersonate-navbar>

0 commit comments

Comments
 (0)