fix(documents): tag click no longer navigates to document detail page
Nesting the tag <button> inside the row's <a href="…"> made the browser treat any click on the button as a click on the anchor, sending the user to the document detail page even though the tag handler called goto() with the tag-filter URL. e.stopPropagation() doesn't cancel the anchor's default navigation. Refactor to the stretched-link pattern: the row-wide anchor sits as an overlay (`absolute inset-0 z-0`) and the content wrapper sits above it (`relative z-10` + `pointer-events-none`). Tag buttons re-enable pointer events with `pointer-events-auto`, so they're true siblings of the anchor and receive their own clicks. Empty content areas pass through to the anchor for whole-row navigation. The vitest-browser client project doesn't load Tailwind CSS, so the z-index has no effect there and Playwright's coordinate-based click hits the anchor instead of the button. Trigger the click directly on the button DOM element in the unit test (with a comment explaining the test-env constraint); the actual user-facing behavior is verified via playwright against the running dev server. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -36,8 +36,8 @@ const hasMore = $derived(item.contributors.length >= 4);
|
||||
|
||||
function tagClass(matched: boolean): string {
|
||||
return matched
|
||||
? 'inline-flex items-center gap-1 rounded px-2 py-1.5 text-xs font-bold tracking-widest uppercase bg-primary text-primary-fg transition-colors'
|
||||
: 'inline-flex items-center gap-1 rounded px-2 py-1.5 text-xs font-bold tracking-widest uppercase bg-muted text-ink hover:bg-primary hover:text-primary-fg transition-colors';
|
||||
? 'pointer-events-auto inline-flex items-center gap-1 rounded px-2 py-1.5 text-xs font-bold tracking-widest uppercase bg-primary text-primary-fg transition-colors'
|
||||
: 'pointer-events-auto inline-flex items-center gap-1 rounded px-2 py-1.5 text-xs font-bold tracking-widest uppercase bg-muted text-ink hover:bg-primary hover:text-primary-fg transition-colors';
|
||||
}
|
||||
|
||||
function safeTagColor(color: string | null | undefined): string {
|
||||
@@ -45,8 +45,15 @@ function safeTagColor(color: string | null | undefined): string {
|
||||
}
|
||||
</script>
|
||||
|
||||
<li class="group transition-colors duration-200 hover:bg-muted/50">
|
||||
<a href="/documents/{doc.id}" class="block px-4 py-4 sm:py-5">
|
||||
<li class="group relative transition-colors duration-200 hover:bg-muted/50">
|
||||
<!--
|
||||
Stretched-link pattern: the row-wide anchor sits as an overlay so it
|
||||
isn't a parent of interior interactive controls (tag buttons). Nesting
|
||||
<button> inside <a> is invalid HTML and was causing tag clicks to also
|
||||
fire the row navigation in real browsers.
|
||||
-->
|
||||
<a href="/documents/{doc.id}" aria-label={titleText} class="absolute inset-0 z-0 block"></a>
|
||||
<div class="pointer-events-none relative z-10 px-4 py-4 sm:py-5">
|
||||
<div class="flex gap-3 sm:gap-5">
|
||||
<!-- Thumbnail tile -->
|
||||
<DocumentThumbnail doc={doc} size="lg" />
|
||||
@@ -124,7 +131,7 @@ function safeTagColor(color: string | null | undefined): string {
|
||||
<button
|
||||
type="button"
|
||||
class={tagClass(matchedTagIds.has(tag.id))}
|
||||
onclick={(e) => { e.stopPropagation(); goto('/documents?tag=' + encodeURIComponent(tag.name)); }}
|
||||
onclick={() => goto('/documents?tag=' + encodeURIComponent(tag.name))}
|
||||
>
|
||||
{#if tag.color}
|
||||
<span
|
||||
@@ -202,5 +209,5 @@ function safeTagColor(color: string | null | undefined): string {
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</a>
|
||||
</div>
|
||||
</li>
|
||||
|
||||
@@ -1,12 +1,16 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
import { cleanup, render } from 'vitest-browser-svelte';
|
||||
import { page } from 'vitest/browser';
|
||||
import { goto } from '$app/navigation';
|
||||
import DocumentRow from './DocumentRow.svelte';
|
||||
import type { components } from '$lib/generated/api';
|
||||
|
||||
vi.mock('$app/navigation', () => ({ goto: vi.fn() }));
|
||||
|
||||
afterEach(() => cleanup());
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.mocked(goto).mockClear();
|
||||
});
|
||||
|
||||
type DocumentSearchItem = components['schemas']['DocumentSearchItem'];
|
||||
|
||||
@@ -229,7 +233,6 @@ describe('DocumentRow – tags', () => {
|
||||
});
|
||||
|
||||
it('navigates to /documents?tag=… on tag click', async () => {
|
||||
const { goto } = await import('$app/navigation');
|
||||
const item = makeItem({
|
||||
document: {
|
||||
...makeItem().document,
|
||||
@@ -237,8 +240,15 @@ describe('DocumentRow – tags', () => {
|
||||
}
|
||||
});
|
||||
render(DocumentRow, { item });
|
||||
await page.getByRole('button', { name: 'Urlaub & Reise' }).click();
|
||||
expect(goto).toHaveBeenCalledWith('/documents?tag=Urlaub%20%26%20Reise');
|
||||
// Tailwind CSS isn't loaded in the vitest-browser client project, so the
|
||||
// `z-10` that elevates the content wrapper above the stretched-link
|
||||
// overlay anchor has no effect here — Playwright's coordinate-based
|
||||
// click would hit the anchor instead of the tag button. Fire the click
|
||||
// directly on the button to verify the handler logic.
|
||||
document.querySelector<HTMLButtonElement>('button')?.click();
|
||||
await expect
|
||||
.poll(() => vi.mocked(goto).mock.calls[0]?.[0])
|
||||
.toBe('/documents?tag=Urlaub%20%26%20Reise');
|
||||
});
|
||||
|
||||
it('tag click does not navigate to the document detail page', async () => {
|
||||
|
||||
Reference in New Issue
Block a user