fix(chronik): split Für-dich row markup — Dismiss is sibling of link, not nested
HTML5 forbids interactive content (<button>, <a>, <input>...) as descendants of <a>. The original <a href=…><button>✓</button></a> markup triggered two concrete bugs flagged by Felix, Nora, and Leonie in PR #288 review: - Browsers inconsistently route the nested click: on some engines the stopPropagation() still bubbles, and the user navigates into the document instead of dismissing. - The senior audience (60+) tap-selects with a slight drag, and the OS treats the interaction as anchor vs. button inconsistently — a reproducible usability failure Leonie has seen in testing before. Refactor to the Option-C layout from issue #285 comment #3573: outer <li> flex container, <a> wrapping avatar + body + time, <button> as a sibling. Independent focus stops, invalid-HTML gone, no behavioural regression. A new spec locks the invariant: `dismiss.closest('a')` must be null. Part of #285, address PR #288 review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -18,7 +18,6 @@ function verb(type: NotificationItem['type'], actor: string): string {
|
||||
}
|
||||
|
||||
function href(n: NotificationItem): string {
|
||||
// Link to the specific comment/reference within the document.
|
||||
return `/documents/${n.documentId}?commentId=${n.referenceId}`;
|
||||
}
|
||||
</script>
|
||||
@@ -78,10 +77,12 @@ function href(n: NotificationItem): string {
|
||||
|
||||
<ul role="list" class="flex flex-col gap-2">
|
||||
{#each unread as n (n.id)}
|
||||
<li>
|
||||
<li
|
||||
class="fade-in group flex items-start gap-3 rounded-sm p-2 transition-colors hover:bg-canvas"
|
||||
>
|
||||
<a
|
||||
href={href(n)}
|
||||
class="fade-in group flex items-start gap-3 rounded-sm p-2 transition-colors hover:bg-canvas focus-visible:ring-2 focus-visible:ring-focus-ring focus-visible:outline-none"
|
||||
class="flex min-w-0 flex-1 items-start gap-3 rounded-sm focus-visible:ring-2 focus-visible:ring-focus-ring focus-visible:outline-none"
|
||||
>
|
||||
<span
|
||||
aria-hidden="true"
|
||||
@@ -97,30 +98,26 @@ function href(n: NotificationItem): string {
|
||||
{relativeTime(n.createdAt)}
|
||||
</p>
|
||||
</div>
|
||||
<button
|
||||
type="button"
|
||||
data-testid="chronik-fuerdich-dismiss"
|
||||
aria-label={m.chronik_mark_read_aria()}
|
||||
onclick={(e) => {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
onMarkRead(n);
|
||||
}}
|
||||
class="mt-0.5 shrink-0 rounded-sm p-1 text-ink-3 transition-colors hover:bg-muted hover:text-ink focus-visible:ring-2 focus-visible:ring-focus-ring focus-visible:outline-none"
|
||||
>
|
||||
<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"
|
||||
aria-hidden="true"
|
||||
>
|
||||
<path stroke-linecap="round" stroke-linejoin="round" d="M6 18L18 6M6 6l12 12" />
|
||||
</svg>
|
||||
</button>
|
||||
</a>
|
||||
<button
|
||||
type="button"
|
||||
data-testid="chronik-fuerdich-dismiss"
|
||||
aria-label={m.chronik_mark_read_aria()}
|
||||
onclick={() => onMarkRead(n)}
|
||||
class="mt-0.5 shrink-0 rounded-sm p-1 text-ink-3 transition-colors hover:bg-muted hover:text-ink focus-visible:ring-2 focus-visible:ring-focus-ring focus-visible:outline-none"
|
||||
>
|
||||
<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"
|
||||
aria-hidden="true"
|
||||
>
|
||||
<path stroke-linecap="round" stroke-linejoin="round" d="M6 18L18 6M6 6l12 12" />
|
||||
</svg>
|
||||
</button>
|
||||
</li>
|
||||
{/each}
|
||||
</ul>
|
||||
|
||||
@@ -113,4 +113,17 @@ describe('ChronikFuerDichBox', () => {
|
||||
expect(onMarkRead).toHaveBeenCalledTimes(1);
|
||||
expect(onMarkRead.mock.calls[0][0]).toEqual(n);
|
||||
});
|
||||
|
||||
it('Dismiss button is a sibling of the document link, never nested inside <a>', async () => {
|
||||
render(ChronikFuerDichBox, {
|
||||
unread: [notif({ id: 'x' })],
|
||||
onMarkRead: vi.fn(),
|
||||
onMarkAllRead: vi.fn()
|
||||
});
|
||||
const dismiss = document.querySelector('[data-testid="chronik-fuerdich-dismiss"]');
|
||||
expect(dismiss).not.toBeNull();
|
||||
// HTML spec forbids interactive content descendants of <a>.
|
||||
// Prevents the senior-audience tap-drag bug flagged by Leonie.
|
||||
expect(dismiss?.closest('a')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user