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 bc3f6843ca..09340e98cb 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 c6975fdc19..8f5ebfa2cf 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 7835e17310..48626a50f4 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"
>
<path
d="M100 50.5908C100 78.2051 77.6142 100.591 50 100.591C22.3858 100.591 0 78.2051 0 50.5908C0 22.9766 22.3858 0.59082 50 0.59082C77.6142 0.59082 100 22.9766 100 50.5908ZM9.08144 50.5908C9.08144 73.1895 27.4013 91.5094 50 91.5094C72.5987 91.5094 90.9186 73.1895 90.9186 50.5908C90.9186 27.9921 72.5987 9.67226 50 9.67226C27.4013 9.67226 9.08144 27.9921 9.08144 50.5908Z"
diff --git a/web/src/lib/components/shared-components/notification/__tests__/notification-card.spec.ts b/web/src/lib/components/shared-components/notification/__tests__/notification-card.spec.ts
index bed072f5b7..2d92e77377 100644
--- a/web/src/lib/components/shared-components/notification/__tests__/notification-card.spec.ts
+++ b/web/src/lib/components/shared-components/notification/__tests__/notification-card.spec.ts
@@ -39,6 +39,29 @@ describe('NotificationCard component', () => {
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 44634d6b20..669b7d75bd 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<NotificationList>): HTMLA
}
describe('NotificationList component', () => {
- const sut: RenderResult<NotificationList> = 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<NotificationList> = 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 aac0823bf5..61e710a170 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}
/>
</div>
@@ -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}
</button>
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 d94ff5c14d..c7c54be267 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 @@
<script lang="ts">
import { notificationController } from './notification';
import { fade } from 'svelte/transition';
-
+ import { t } from 'svelte-i18n';
import NotificationCard from './notification-card.svelte';
import { flip } from 'svelte/animate';
import { quintOut } from 'svelte/easing';
@@ -9,12 +9,14 @@
const { notificationList } = notificationController;
</script>
-{#if $notificationList.length > 0}
- <section transition:fade={{ duration: 250 }} id="notification-list" class="fixed right-5 top-[80px] z-[99999999]">
- {#each $notificationList as notification (notification.id)}
- <div animate:flip={{ duration: 250, easing: quintOut }}>
- <NotificationCard {notification} />
- </div>
- {/each}
- </section>
-{/if}
+<div role="status" aria-relevant="additions" aria-label={$t('notifications')}>
+ {#if $notificationList.length > 0}
+ <section transition:fade={{ duration: 250 }} id="notification-list" class="fixed right-5 top-[80px] z-[99999999]">
+ {#each $notificationList as notification (notification.id)}
+ <div animate:flip={{ duration: 250, easing: quintOut }}>
+ <NotificationCard {notification} />
+ </div>
+ {/each}
+ </section>
+ {/if}
+</div>