refactor(notification): replace callback props with form actions in Dropdown and Bell

NotificationDropdown now wraps each row in a <form action="/aktivitaeten?/dismiss-notification">
and the mark-all control in <form action="/aktivitaeten?/mark-all-read">, wired via use:enhance
for optimistic UI. Props renamed onMarkRead/onMarkAllRead → optimisticMarkRead/optimisticMarkAllRead
to match the simplified store API. NotificationBell passes the store helpers directly; handleMarkRead
is removed.

Test mocks updated: $app/forms enhance mock fires SubmitFunction synchronously on form submit so
callback assertions work without a real HTTP round-trip.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-05-19 23:15:56 +02:00
committed by marcel
parent cdd5bfa318
commit 3d3c111c2b
4 changed files with 218 additions and 167 deletions

View File

@@ -1,10 +1,8 @@
<script lang="ts">
import { onMount, onDestroy } from 'svelte';
import { goto } from '$app/navigation';
import { m } from '$lib/paraglide/messages.js';
import { clickOutside } from '$lib/shared/actions/clickOutside';
import { notificationStore } from '$lib/notification/notifications.svelte';
import { buildCommentHref } from '$lib/shared/discussion/commentDeepLink';
import NotificationDropdown from './NotificationDropdown.svelte';
let open = $state(false);
@@ -30,17 +28,6 @@ function closeDropdown() {
bellButtonEl?.focus();
}
async function handleMarkRead(notification: Parameters<typeof stream.markRead>[0]) {
await stream.markRead(notification);
const url = buildCommentHref(
notification.documentId,
notification.referenceId,
notification.annotationId
);
closeDropdown();
goto(url);
}
function handleKeydown(event: KeyboardEvent) {
if (event.key === 'Escape' && open) {
event.stopPropagation();
@@ -113,8 +100,8 @@ onDestroy(() => {
{#if open}
<NotificationDropdown
notifications={stream.notifications}
onMarkRead={handleMarkRead}
onMarkAllRead={stream.markAllRead}
optimisticMarkRead={stream.optimisticMarkRead}
optimisticMarkAllRead={stream.optimisticMarkAllRead}
onClose={closeDropdown}
/>
{/if}

View File

@@ -3,10 +3,18 @@ import { cleanup, render } from 'vitest-browser-svelte';
import type { NotificationItem } from '$lib/notification/notifications';
import NotificationBell from './NotificationBell.svelte';
const gotoMock = vi.hoisted(() => vi.fn());
vi.mock('$app/navigation', () => ({ goto: gotoMock, beforeNavigate: vi.fn() }));
vi.mock('$app/navigation', () => ({ goto: vi.fn(), beforeNavigate: vi.fn() }));
vi.mock('$app/forms', () => ({
enhance(node: HTMLFormElement, submit?: (opts: { formData: FormData }) => unknown) {
const handler = (e: Event) => {
e.preventDefault();
submit?.({ formData: new FormData(node) } as never);
};
node.addEventListener('submit', handler);
return { destroy: () => node.removeEventListener('submit', handler) };
}
}));
const mockMarkRead = vi.hoisted(() => vi.fn().mockResolvedValue(undefined));
const mockNotificationList = vi.hoisted((): { value: NotificationItem[] } => ({ value: [] }));
vi.mock('$lib/notification/notifications.svelte', () => ({
@@ -17,18 +25,17 @@ vi.mock('$lib/notification/notifications.svelte', () => ({
get unreadCount() {
return mockNotificationList.value.length;
},
markRead: mockMarkRead,
optimisticMarkRead: vi.fn(),
optimisticMarkAllRead: vi.fn(),
fetchNotifications: vi.fn().mockResolvedValue(undefined),
init: vi.fn(),
destroy: vi.fn(),
markAllRead: vi.fn()
destroy: vi.fn()
}
}));
afterEach(() => {
cleanup();
gotoMock.mockClear();
mockMarkRead.mockClear();
vi.clearAllMocks();
mockNotificationList.value = [];
});
@@ -45,16 +52,6 @@ const makeNotification = (overrides: Partial<NotificationItem> = {}): Notificati
...overrides
});
async function openDropdownAndClickFirstNotification() {
const bellButton = document.querySelector<HTMLButtonElement>('button[aria-haspopup="true"]')!;
bellButton.click();
await vi.waitFor(() => {
expect(document.querySelector('[role="dialog"]')).not.toBeNull();
});
const notifButton = document.querySelector<HTMLButtonElement>('[role="list"] button')!;
notifButton.click();
}
describe('NotificationBell — cursor and tooltip', () => {
it('bell button has cursor-pointer class', async () => {
render(NotificationBell);
@@ -82,29 +79,3 @@ describe('NotificationBell — cursor and tooltip', () => {
expect(btn.getAttribute('aria-label')).toBe(btn.getAttribute('title'));
});
});
describe('NotificationBell', () => {
it('handleMarkRead navigates to URL including annotationId when notification has annotationId', async () => {
mockNotificationList.value = [makeNotification({ annotationId: 'annot-1' })];
render(NotificationBell);
await openDropdownAndClickFirstNotification();
await vi.waitFor(() => {
expect(gotoMock).toHaveBeenCalledWith(
'/documents/doc-1?commentId=ref-1&annotationId=annot-1'
);
});
});
it('handleMarkRead navigates to commentId-only URL when annotationId is absent', async () => {
mockNotificationList.value = [makeNotification({ annotationId: null })];
render(NotificationBell);
await openDropdownAndClickFirstNotification();
await vi.waitFor(() => {
expect(gotoMock).toHaveBeenCalledWith('/documents/doc-1?commentId=ref-1');
});
});
});

View File

@@ -1,17 +1,19 @@
<script lang="ts">
import { goto } from '$app/navigation';
import { enhance } from '$app/forms';
import { m } from '$lib/paraglide/messages.js';
import { relativeTime } from '$lib/shared/utils/time';
import { buildCommentHref } from '$lib/shared/discussion/commentDeepLink';
import type { NotificationItem } from '$lib/notification/notifications.svelte';
type Props = {
notifications: NotificationItem[];
onMarkRead: (notification: NotificationItem) => void;
onMarkAllRead: () => void;
optimisticMarkRead: (id: string) => void;
optimisticMarkAllRead: () => void;
onClose: () => void;
};
let { notifications, onMarkRead, onMarkAllRead, onClose }: Props = $props();
let { notifications, optimisticMarkRead, optimisticMarkAllRead, onClose }: Props = $props();
function handleViewAll() {
onClose(); // close first — avoids stale dropdown during navigation transition
@@ -31,13 +33,23 @@ function handleViewAll() {
{m.notification_bell_label()}
</span>
{#if notifications.length > 0}
<button
type="button"
onclick={onMarkAllRead}
class="text-xs font-medium text-ink-3 transition-colors hover:text-ink"
<form
action="/aktivitaeten?/mark-all-read"
method="POST"
use:enhance={() => {
optimisticMarkAllRead();
return async ({ update }) => {
await update({ reset: false, invalidateAll: false });
};
}}
>
{m.notification_mark_all_read()}
</button>
<button
type="submit"
class="text-xs font-medium text-ink-3 transition-colors hover:text-ink"
>
{m.notification_mark_all_read()}
</button>
</form>
{/if}
</div>
@@ -66,67 +78,87 @@ function handleViewAll() {
<ul role="list" class="max-h-[24rem] overflow-y-auto">
{#each notifications as notification (notification.id)}
<li>
<button
type="button"
onclick={() => onMarkRead(notification)}
class="flex w-full cursor-pointer items-start gap-3 border-b border-line px-4 py-3 text-left last:border-b-0 hover:bg-canvas
{!notification.read ? 'bg-accent-bg/20' : ''}"
<form
action="/aktivitaeten?/dismiss-notification"
method="POST"
class="contents"
use:enhance={() => {
optimisticMarkRead(notification.id);
onClose();
goto(
buildCommentHref(
notification.documentId,
notification.referenceId,
notification.annotationId
)
);
return async ({ update }) => {
await update({ reset: false, invalidateAll: false });
};
}}
>
<!-- Type icon -->
<span class="mt-0.5 shrink-0 text-ink-3" aria-hidden="true">
{#if notification.type === 'REPLY'}
<!-- Reply icon -->
<svg
xmlns="http://www.w3.org/2000/svg"
class="h-4 w-4"
fill="none"
viewBox="0 0 24 24"
stroke="currentColor"
stroke-width="2"
>
<path
stroke-linecap="round"
stroke-linejoin="round"
d="M3 10h10a8 8 0 018 8v2M3 10l6 6m-6-6l6-6"
/>
</svg>
{:else}
<!-- Mention icon -->
<svg
xmlns="http://www.w3.org/2000/svg"
class="h-4 w-4"
fill="none"
viewBox="0 0 24 24"
stroke="currentColor"
stroke-width="2"
>
<path
stroke-linecap="round"
stroke-linejoin="round"
d="M16 12a4 4 0 10-8 0 4 4 0 008 0zm0 0v1.5a2.5 2.5 0 005 0V12a9 9 0 10-9 9m4.5-1.206a8.959 8.959 0 01-4.5 1.207"
/>
</svg>
<input type="hidden" name="notificationId" value={notification.id} />
<button
type="submit"
class="flex w-full cursor-pointer items-start gap-3 border-b border-line px-4 py-3.5 text-left last:border-b-0 hover:bg-canvas
{!notification.read ? 'bg-accent-bg/20' : ''}"
>
<!-- Type icon -->
<span class="mt-0.5 shrink-0 text-ink-3" aria-hidden="true">
{#if notification.type === 'REPLY'}
<!-- Reply icon -->
<svg
xmlns="http://www.w3.org/2000/svg"
class="h-4 w-4"
fill="none"
viewBox="0 0 24 24"
stroke="currentColor"
stroke-width="2"
>
<path
stroke-linecap="round"
stroke-linejoin="round"
d="M3 10h10a8 8 0 018 8v2M3 10l6 6m-6-6l6-6"
/>
</svg>
{:else}
<!-- Mention icon -->
<svg
xmlns="http://www.w3.org/2000/svg"
class="h-4 w-4"
fill="none"
viewBox="0 0 24 24"
stroke="currentColor"
stroke-width="2"
>
<path
stroke-linecap="round"
stroke-linejoin="round"
d="M16 12a4 4 0 10-8 0 4 4 0 008 0zm0 0v1.5a2.5 2.5 0 005 0V12a9 9 0 10-9 9m4.5-1.206a8.959 8.959 0 01-4.5 1.207"
/>
</svg>
{/if}
</span>
<!-- Text + time -->
<div class="min-w-0 flex-1">
<p class="text-sm leading-snug text-ink">
{notification.type === 'REPLY'
? m.notification_type_reply({ actor: notification.actorName })
: m.notification_type_mention({ actor: notification.actorName })}
</p>
<p class="mt-1 text-xs text-ink-3">{relativeTime(notification.createdAt)}</p>
</div>
<!-- Unread dot -->
{#if !notification.read}
<span
class="mt-1.5 h-2 w-2 shrink-0 rounded-full bg-primary"
aria-label={m.notification_unread()}
></span>
{/if}
</span>
<!-- Text + time -->
<div class="min-w-0 flex-1">
<p class="text-sm leading-snug text-ink">
{notification.type === 'REPLY'
? m.notification_type_reply({ actor: notification.actorName })
: m.notification_type_mention({ actor: notification.actorName })}
</p>
<p class="mt-1 text-xs text-ink-3">{relativeTime(notification.createdAt)}</p>
</div>
<!-- Unread dot -->
{#if !notification.read}
<span
class="mt-1.5 h-2 w-2 shrink-0 rounded-full bg-primary"
aria-label={m.notification_unread()}
></span>
{/if}
</button>
</button>
</form>
</li>
{/each}
</ul>

View File

@@ -6,6 +6,19 @@ import NotificationDropdown from './NotificationDropdown.svelte';
vi.mock('$app/navigation', () => ({ goto: vi.fn() }));
// Invoke the SubmitFunction synchronously on form submit so tests can assert
// that optimisticMarkRead / optimisticMarkAllRead are called.
vi.mock('$app/forms', () => ({
enhance(node: HTMLFormElement, submit?: (opts: { formData: FormData }) => unknown) {
const handler = (e: Event) => {
e.preventDefault();
submit?.({ formData: new FormData(node) } as never);
};
node.addEventListener('submit', handler);
return { destroy: () => node.removeEventListener('submit', handler) };
}
}));
afterEach(() => {
cleanup();
vi.clearAllMocks();
@@ -29,8 +42,8 @@ describe('NotificationDropdown', () => {
render(NotificationDropdown, {
props: {
notifications: [],
onMarkRead: () => {},
onMarkAllRead: () => {},
optimisticMarkRead: () => {},
optimisticMarkAllRead: () => {},
onClose: () => {}
}
});
@@ -42,8 +55,8 @@ describe('NotificationDropdown', () => {
render(NotificationDropdown, {
props: {
notifications: [],
onMarkRead: () => {},
onMarkAllRead: () => {},
optimisticMarkRead: () => {},
optimisticMarkAllRead: () => {},
onClose: () => {}
}
});
@@ -55,8 +68,8 @@ describe('NotificationDropdown', () => {
render(NotificationDropdown, {
props: {
notifications: [],
onMarkRead: () => {},
onMarkAllRead: () => {},
optimisticMarkRead: () => {},
optimisticMarkAllRead: () => {},
onClose: () => {}
}
});
@@ -70,8 +83,8 @@ describe('NotificationDropdown', () => {
render(NotificationDropdown, {
props: {
notifications: [makeNotification()],
onMarkRead: () => {},
onMarkAllRead: () => {},
optimisticMarkRead: () => {},
optimisticMarkAllRead: () => {},
onClose: () => {}
}
});
@@ -83,8 +96,8 @@ describe('NotificationDropdown', () => {
render(NotificationDropdown, {
props: {
notifications: [makeNotification({ type: 'REPLY', actorName: 'Bert' })],
onMarkRead: () => {},
onMarkAllRead: () => {},
optimisticMarkRead: () => {},
optimisticMarkAllRead: () => {},
onClose: () => {}
}
});
@@ -98,8 +111,8 @@ describe('NotificationDropdown', () => {
render(NotificationDropdown, {
props: {
notifications: [makeNotification({ type: 'MENTION', actorName: 'Clara' })],
onMarkRead: () => {},
onMarkAllRead: () => {},
optimisticMarkRead: () => {},
optimisticMarkAllRead: () => {},
onClose: () => {}
}
});
@@ -116,8 +129,8 @@ describe('NotificationDropdown', () => {
makeNotification({ id: 'n1', read: false }),
makeNotification({ id: 'n2', read: true })
],
onMarkRead: () => {},
onMarkAllRead: () => {},
optimisticMarkRead: () => {},
optimisticMarkAllRead: () => {},
onClose: () => {}
}
});
@@ -126,37 +139,83 @@ describe('NotificationDropdown', () => {
expect(unreadDots.length).toBe(1);
});
it('calls onMarkRead with the notification when an item is clicked', async () => {
const onMarkRead = vi.fn();
it('each notification row is wrapped in a form posting to the dismiss action', async () => {
render(NotificationDropdown, {
props: {
notifications: [makeNotification({ id: 'n42' })],
optimisticMarkRead: () => {},
optimisticMarkAllRead: () => {},
onClose: () => {}
}
});
const form = document.querySelector('form[action="/aktivitaeten?/dismiss-notification"]');
expect(form).not.toBeNull();
expect(form?.getAttribute('method')).toBe('POST');
});
it('the dismiss form has a hidden notificationId input with the notification id', async () => {
render(NotificationDropdown, {
props: {
notifications: [makeNotification({ id: 'n42' })],
optimisticMarkRead: () => {},
optimisticMarkAllRead: () => {},
onClose: () => {}
}
});
const input = document.querySelector<HTMLInputElement>(
'form[action="/aktivitaeten?/dismiss-notification"] input[name="notificationId"]'
);
expect(input?.value).toBe('n42');
});
it('calls optimisticMarkRead with the notification id when a row is submitted', async () => {
const optimisticMarkRead = vi.fn();
const n = makeNotification({ id: 'n42', actorName: 'Anna' });
render(NotificationDropdown, {
props: {
notifications: [n],
onMarkRead,
onMarkAllRead: () => {},
optimisticMarkRead,
optimisticMarkAllRead: () => {},
onClose: () => {}
}
});
await page.getByRole('button', { name: /Anna hat auf deinen/i }).click();
expect(onMarkRead).toHaveBeenCalledWith(n);
expect(optimisticMarkRead).toHaveBeenCalledWith('n42');
});
it('calls onMarkAllRead when the mark-all-read button is clicked', async () => {
const onMarkAllRead = vi.fn();
it('the mark-all-read control is a form posting to the mark-all-read action', async () => {
render(NotificationDropdown, {
props: {
notifications: [makeNotification()],
onMarkRead: () => {},
onMarkAllRead,
optimisticMarkRead: () => {},
optimisticMarkAllRead: () => {},
onClose: () => {}
}
});
const form = document.querySelector('form[action="/aktivitaeten?/mark-all-read"]');
expect(form).not.toBeNull();
expect(form?.getAttribute('method')).toBe('POST');
});
it('calls optimisticMarkAllRead when the mark-all-read button is submitted', async () => {
const optimisticMarkAllRead = vi.fn();
render(NotificationDropdown, {
props: {
notifications: [makeNotification()],
optimisticMarkRead: () => {},
optimisticMarkAllRead,
onClose: () => {}
}
});
await page.getByRole('button', { name: /alle gelesen/i }).click();
expect(onMarkAllRead).toHaveBeenCalledOnce();
expect(optimisticMarkAllRead).toHaveBeenCalledOnce();
});
it('calls onClose when the view-all button is clicked', async () => {
@@ -164,8 +223,8 @@ describe('NotificationDropdown', () => {
render(NotificationDropdown, {
props: {
notifications: [],
onMarkRead: () => {},
onMarkAllRead: () => {},
optimisticMarkRead: () => {},
optimisticMarkAllRead: () => {},
onClose
}
});
@@ -179,8 +238,8 @@ describe('NotificationDropdown', () => {
render(NotificationDropdown, {
props: {
notifications: [],
onMarkRead: () => {},
onMarkAllRead: () => {},
optimisticMarkRead: () => {},
optimisticMarkAllRead: () => {},
onClose: () => {}
}
});
@@ -193,12 +252,15 @@ describe('NotificationDropdown', () => {
it('calls onClose before navigating to /aktivitaeten', async () => {
const callOrder: string[] = [];
const onClose = vi.fn(() => callOrder.push('close'));
vi.mocked(goto).mockImplementation(() => callOrder.push('goto'));
vi.mocked(goto).mockImplementation(() => {
callOrder.push('goto');
return Promise.resolve();
});
render(NotificationDropdown, {
props: {
notifications: [],
onMarkRead: () => {},
onMarkAllRead: () => {},
optimisticMarkRead: () => {},
optimisticMarkAllRead: () => {},
onClose
}
});
@@ -212,8 +274,8 @@ describe('NotificationDropdown', () => {
render(NotificationDropdown, {
props: {
notifications: [makeNotification({ id: 'm1', type: 'MENTION', actorName: 'Anna' })],
onMarkRead: () => {},
onMarkAllRead: () => {},
optimisticMarkRead: () => {},
optimisticMarkAllRead: () => {},
onClose: () => {}
}
});
@@ -225,8 +287,8 @@ describe('NotificationDropdown', () => {
render(NotificationDropdown, {
props: {
notifications: [makeNotification({ id: 'r1', type: 'REPLY', actorName: 'Bert' })],
onMarkRead: () => {},
onMarkAllRead: () => {},
optimisticMarkRead: () => {},
optimisticMarkAllRead: () => {},
onClose: () => {}
}
});
@@ -242,14 +304,13 @@ describe('NotificationDropdown', () => {
makeNotification({ id: 'n1', actorName: 'First' }),
makeNotification({ id: 'n2', actorName: 'Second' })
],
onMarkRead: () => {},
onMarkAllRead: () => {},
optimisticMarkRead: () => {},
optimisticMarkAllRead: () => {},
onClose: () => {}
}
});
const items = document.querySelectorAll('button[type="button"]');
// At least 2 items + mark-all button
expect(items.length).toBeGreaterThanOrEqual(2);
const forms = document.querySelectorAll('form[action="/aktivitaeten?/dismiss-notification"]');
expect(forms.length).toBe(2);
});
});