From 72ab664936926a62c82b1ec46a0c51dc663991d8 Mon Sep 17 00:00:00 2001 From: Ben <45583362+ben-basten@users.noreply.github.com> Date: Tue, 27 Aug 2024 18:13:17 -0400 Subject: [PATCH] feat(web): announce notifications to screen readers (#12071) --- e2e/src/web/specs/photo-viewer.e2e-spec.ts | 2 +- .../context-menu/context-menu.svelte | 2 +- .../shared-components/loading-spinner.svelte | 1 + .../__tests__/notification-card.spec.ts | 23 +++++++++++++++++++ .../__tests__/notification-list.spec.ts | 6 +++-- .../notification/notification-card.svelte | 4 ++++ .../notification/notification-list.svelte | 22 ++++++++++-------- 7 files changed, 46 insertions(+), 14 deletions(-) diff --git a/e2e/src/web/specs/photo-viewer.e2e-spec.ts b/e2e/src/web/specs/photo-viewer.e2e-spec.ts index bc3f6843ca750..09340e98cbfb3 100644 --- a/e2e/src/web/specs/photo-viewer.e2e-spec.ts +++ b/e2e/src/web/specs/photo-viewer.e2e-spec.ts @@ -33,7 +33,7 @@ test.describe('Photo Viewer', () => { await page.waitForLoadState('load'); // this is the spinner await page.waitForSelector('svg[role=status]'); - await expect(page.getByRole('status')).toBeVisible(); + await expect(page.getByTestId('loading-spinner')).toBeVisible(); }); test('loads high resolution photo when zoomed', async ({ page }) => { diff --git a/web/src/lib/components/shared-components/context-menu/context-menu.svelte b/web/src/lib/components/shared-components/context-menu/context-menu.svelte index c6975fdc195e3..8f5ebfa2cfedc 100644 --- a/web/src/lib/components/shared-components/context-menu/context-menu.svelte +++ b/web/src/lib/components/shared-components/context-menu/context-menu.svelte @@ -50,7 +50,7 @@ bind:this={menuElement} class:max-h-[100vh]={isVisible} class:max-h-0={!isVisible} - class="flex flex-col transition-all duration-[250ms] ease-in-out" + class="flex flex-col transition-all duration-[250ms] ease-in-out outline-none" role="menu" tabindex="-1" > diff --git a/web/src/lib/components/shared-components/loading-spinner.svelte b/web/src/lib/components/shared-components/loading-spinner.svelte index 7835e17310234..48626a50f485a 100644 --- a/web/src/lib/components/shared-components/loading-spinner.svelte +++ b/web/src/lib/components/shared-components/loading-spinner.svelte @@ -11,6 +11,7 @@ viewBox="0 0 100 101" fill="none" xmlns="http://www.w3.org/2000/svg" + data-testid="loading-spinner" > { expect(sut.getByTestId('message')).toHaveTextContent('Notification message'); }); + it('makes all buttons non-focusable and hidden from screen readers', () => { + sut = render(NotificationCard, { + notification: { + id: 1234, + message: 'Notification message', + timeout: 1000, + type: NotificationType.Info, + action: { type: 'discard' }, + button: { + text: 'button', + onClick: vi.fn(), + }, + }, + }); + const buttons = sut.container.querySelectorAll('button'); + + expect(buttons).toHaveLength(2); + for (const button of buttons) { + expect(button.getAttribute('tabindex')).toBe('-1'); + expect(button.getAttribute('aria-hidden')).toBe('true'); + } + }); + it('shows title and renders component', () => { sut = render(NotificationCard, { notification: { diff --git a/web/src/lib/components/shared-components/notification/__tests__/notification-list.spec.ts b/web/src/lib/components/shared-components/notification/__tests__/notification-list.spec.ts index 44634d6b20038..669b7d75bd855 100644 --- a/web/src/lib/components/shared-components/notification/__tests__/notification-list.spec.ts +++ b/web/src/lib/components/shared-components/notification/__tests__/notification-list.spec.ts @@ -9,8 +9,6 @@ function _getNotificationListElement(sut: RenderResult): HTMLA } describe('NotificationList component', () => { - const sut: RenderResult = render(NotificationList); - beforeAll(() => { // https://testing-library.com/docs/svelte-testing-library/faq#why-arent-transition-events-running vi.stubGlobal('requestAnimationFrame', (fn: FrameRequestCallback) => { @@ -23,6 +21,10 @@ describe('NotificationList component', () => { }); it('shows a notification when added and closes it automatically after the delay timeout', async () => { + const sut: RenderResult = render(NotificationList); + const status = await sut.findAllByRole('status'); + + expect(status).toHaveLength(1); expect(_getNotificationListElement(sut)).not.toBeInTheDocument(); notificationController.show({ diff --git a/web/src/lib/components/shared-components/notification/notification-card.svelte b/web/src/lib/components/shared-components/notification/notification-card.svelte index aac0823bf563b..61e710a1707d2 100644 --- a/web/src/lib/components/shared-components/notification/notification-card.svelte +++ b/web/src/lib/components/shared-components/notification/notification-card.svelte @@ -91,6 +91,8 @@ size="20" padding="2" on:click={discard} + aria-hidden="true" + tabindex={-1} /> @@ -108,6 +110,8 @@ type="button" class="{buttonStyle[notification.type]} rounded px-3 pt-1.5 pb-1 transition-all duration-200" on:click={handleButtonClick} + aria-hidden="true" + tabindex={-1} > {notification.button.text} diff --git a/web/src/lib/components/shared-components/notification/notification-list.svelte b/web/src/lib/components/shared-components/notification/notification-list.svelte index d94ff5c14dd97..c7c54be26720c 100644 --- a/web/src/lib/components/shared-components/notification/notification-list.svelte +++ b/web/src/lib/components/shared-components/notification/notification-list.svelte @@ -1,7 +1,7 @@ -{#if $notificationList.length > 0} -
- {#each $notificationList as notification (notification.id)} -
- -
- {/each} -
-{/if} +
+ {#if $notificationList.length > 0} +
+ {#each $notificationList as notification (notification.id)} +
+ +
+ {/each} +
+ {/if} +