Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(components/popovers): screen readers can navigate to popover contents #2672

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .eslintrc-overrides.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@
"@angular-eslint/template/no-any": ["error"],
"@angular-eslint/template/no-call-expression": ["warn"],
"@angular-eslint/template/no-distracting-elements": ["warn"],
"@angular-eslint/template/no-inline-styles": ["warn"],
"@angular-eslint/template/no-inline-styles": [
"warn",
{ "allowBindToStyle": true, "allowNgStyle": true }
Blackbaud-SteveBrush marked this conversation as resolved.
Show resolved Hide resolved
],
"@angular-eslint/template/no-interpolation-in-attributes": ["warn"],
"@angular-eslint/template/no-positive-tabindex": ["warn"],
"@angular-eslint/template/prefer-control-flow": ["warn"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,41 @@
type="button"
[skyPopover]="myPopover0"
>
Popover demo on click
Open popover with interactive content
</button>

<button
Blackbaud-SteveBrush marked this conversation as resolved.
Show resolved Hide resolved
class="sky-btn sky-btn-link"
class="sky-btn sky-btn-primary sky-margin-inline-default"
type="button"
[skyPopover]="myPopover0"
[skyPopoverTrigger]="'mouseenter'"
[skyPopover]="myPopover1"
>
Popover demo on hover
Only text
</button>

<sky-popover popoverTitle="Playground popover" #myPopover0>
The content of a popover can be text, HTML, or Angular components.
</sky-popover>
<p>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Quisque ut velit
a urna fermentum fermentum. Quisque sed lectus sit amet nibh tempus
fermentum ac eget lorem. Mauris lorem nisl, finibus ut turpis vitae,
venenatis faucibus nibh. Phasellus laoreet elit ac sagittis tincidunt. Sed
finibus, sem nec convallis condimentum, nulla odio mattis sem, venenatis
rhoncus neque nisi quis quam. Ut quis aliquet eros. Fusce quis mauris
tellus. Ut pharetra mi sed nisi pharetra, sit amet bibendum leo cursus.
Maecenas bibendum risus vestibulum nisl sagittis, vitae fermentum nibh
facilisis. Nunc luctus vehicula ex ac aliquam. Suspendisse sodales iaculis
nibh id condimentum.
</p>

<p>
<button type="button">Some action</button>
</p>
</sky-page-content>
</sky-page>

<sky-popover #myPopover0 popoverTitle="Playground popover">
The content of a popover can be text, HTML, or Angular components.
<button type="button">Subscribe</button>
</sky-popover>

<sky-popover #myPopover1>
The content of a popover can be text, HTML, or Angular components.
</sky-popover>
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { Component } from '@angular/core';

import { SkyPopoverModule } from '../popover.module';

/**
* Fixture used to test accessibility features.
*/
@Component({
imports: [SkyPopoverModule],
selector: 'sky-popover-test',
standalone: true,
template: `
<button data-sky-id="triggerEl" type="button" [skyPopover]="popover1">
What's this?
</button>
<sky-popover #popover1> Some help message. </sky-popover>
`,
})
export class PopoverA11yTestComponent {}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<div
#popoverRef
class="sky-popover-container sky-popover-max-height"
[@.disabled]="!enableAnimations"
[@skyPopoverAnimation]="animationState"
Expand All @@ -10,7 +11,6 @@
]"
(@skyPopoverAnimation.done)="onAnimationEvent($event)"
(@skyPopoverAnimation.start)="onAnimationEvent($event)"
#popoverRef
>
<div
class="sky-popover"
Expand All @@ -20,40 +20,34 @@
'sky-elevation-4': 'modern'
}"
>
<header
*ngIf="popoverTitle"
class="sky-popover-header"
[skyThemeClass]="{
'sky-padding-even-default': 'default',
'sky-padding-even-lg sky-margin-stacked-lg': 'modern'
}"
>
<h1
*ngIf="popoverTitle"
class="sky-popover-title"
@if (popoverTitle) {
<div
Blackbaud-SteveBrush marked this conversation as resolved.
Show resolved Hide resolved
class="sky-popover-header"
[skyThemeClass]="{
'sky-font-heading-4': 'default',
'sky-font-emphasized': 'modern'
'sky-padding-even-default': 'default',
'sky-padding-even-lg sky-margin-stacked-lg': 'modern'
}"
>
{{ popoverTitle }}
</h1>
</header>
<h4 class="sky-popover-title sky-font-heading-4">
Blackbaud-SteveBrush marked this conversation as resolved.
Show resolved Hide resolved
{{ popoverTitle }}
</h4>
</div>
}
<div
class="sky-popover-body"
[skyThemeClass]="{
'sky-padding-even-default': 'default',
'sky-padding-even-lg': 'modern'
}"
>
<ng-container #contentTarget></ng-container>
<ng-container #contentTarget />
</div>
<span
#arrowRef
aria-hidden="true"
class="sky-popover-arrow"
[style.left.px]="arrowLeft"
[style.top.px]="arrowTop"
#arrowRef
></span>
</div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from '@angular/core';
import {
SKY_STACKING_CONTEXT,
SkyIdService,
SkyOverlayInstance,
SkyOverlayService,
SkyStackingContext,
Expand All @@ -28,8 +29,6 @@ import { SkyPopoverAlignment } from './types/popover-alignment';
import { SkyPopoverPlacement } from './types/popover-placement';
import { SkyPopoverType } from './types/popover-type';

let nextId = 0;

@Component({
selector: 'sky-popover',
templateUrl: './popover.component.html',
Expand Down Expand Up @@ -123,7 +122,7 @@ export class SkyPopoverComponent implements OnDestroy {

public isMouseEnter = false;

public popoverId = `sky-popover-${nextId++}`;
public popoverId: string;

@ViewChild('templateRef', {
read: TemplateRef,
Expand Down Expand Up @@ -161,6 +160,8 @@ export class SkyPopoverComponent implements OnDestroy {
) {
this.#overlayService = overlayService;
this.#zIndex = stackingContext?.zIndex;

this.popoverId = inject(SkyIdService).generateId();
}

public ngOnDestroy(): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import {
inject,
tick,
} from '@angular/core/testing';
import { SkyAppTestUtility, expect } from '@skyux-sdk/testing';
import { NoopAnimationsModule } from '@angular/platform-browser/animations';
import { SkyAppTestUtility, expect, expectAsync } from '@skyux-sdk/testing';
import {
SKY_STACKING_CONTEXT,
SkyAffixAutoFitContext,
Expand All @@ -23,6 +24,7 @@ import {

import { BehaviorSubject, Subject } from 'rxjs';

import { PopoverA11yTestComponent } from './fixtures/popover-a11y.component.fixture';
import { PopoverFixtureComponent } from './fixtures/popover.component.fixture';
import { PopoverFixturesModule } from './fixtures/popover.module.fixture';
import { SkyPopoverAdapterService } from './popover-adapter.service';
Expand Down Expand Up @@ -1082,3 +1084,76 @@ describe('Popover directive', () => {
}));
});
});

describe('Popover directive accessibility', () => {
function getPopoverEl(): HTMLElement | null {
return document.querySelector<HTMLElement>('sky-popover-content');
}

/**
* Asserts the trigger button is accessible.
*/
async function expectAccessible(
buttonEl: HTMLButtonElement | null,
attrs: { ariaExpanded: string },
): Promise<void> {
const pointerEl = buttonEl?.nextElementSibling;
const popoverEl = getPopoverEl();
const popoverId = popoverEl?.id ?? null;
const ariaControls = buttonEl?.getAttribute('aria-controls');
const ariaOwns = pointerEl?.getAttribute('aria-owns');

expect(buttonEl?.getAttribute('aria-expanded')).toEqual(attrs.ariaExpanded);
expect(pointerEl).toExist();

if (attrs.ariaExpanded === 'true') {
expect(popoverEl).toExist();
expect(popoverId).toBeDefined();
expect(ariaControls).toEqual(popoverId);
expect(ariaOwns).toEqual(popoverId);
} else {
expect(popoverEl).toBeNull();
expect(ariaControls).toBeNull();
expect(ariaOwns).toBeNull();
}

await expectAsync(document.body).toBeAccessible({
rules: {
region: {
enabled: false,
},
},
});
}

it('should be accessible', async () => {
TestBed.configureTestingModule({
imports: [PopoverA11yTestComponent, NoopAnimationsModule],
});

const fixture = TestBed.createComponent(PopoverA11yTestComponent);

fixture.detectChanges();

const btn = (
fixture.nativeElement as HTMLElement
).querySelector<HTMLButtonElement>('button[data-sky-id="triggerEl"]');

// Open the popover.
btn?.click();
fixture.detectChanges();

await expectAccessible(btn, {
ariaExpanded: 'true',
});

// Close the popover.
btn?.click();
fixture.detectChanges();
await fixture.whenStable();

await expectAccessible(btn, {
ariaExpanded: 'false',
});
});
});
Loading
Loading