From 37edac4da65874ecc45cb0aa2a2ba920a663abd3 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 29 Apr 2026 18:26:44 +0200 Subject: [PATCH] fix(hover-card): maiden name false positive, placeholder on non-empty editor, card persistence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - PersonHoverCard: alias is compared against both `lastName` and `displayName` before showing as maiden name — prevents false positive when alias is stored as the full current name (e.g. "Maria Schmidt" ≠ "Schmidt" but name unchanged) - PersonMentionEditor: data-placeholder was set statically so the CSS ::before rule showed the placeholder on any blur even with content; now a $effect toggles the attribute based on editor.isEmpty - TranscriptionReadView: hovering onto the card itself cancels the 150ms close timer so the card stays open while reading it; leaving the card closes it immediately — onmouseenter/onmouseleave wired through PersonHoverCard props - hoverCardPosition: removed scrollX/scrollY offset since the card is now position:fixed (scroll is already baked into getBoundingClientRect coords) - MentionDropdown: raised z-index from z-20 to z-50 to render above the hover card - vite.config.ts: pre-bundle Tiptap packages to avoid HMR waterfall on first load Co-Authored-By: Claude Sonnet 4.6 --- .../src/lib/components/MentionDropdown.svelte | 2 +- .../src/lib/components/PersonHoverCard.svelte | 40 +++++++++- .../lib/components/PersonMentionEditor.svelte | 15 +++- .../components/TranscriptionReadView.svelte | 29 ++++++- .../src/lib/utils/hoverCardPosition.spec.ts | 77 +++++-------------- frontend/src/lib/utils/hoverCardPosition.ts | 6 +- frontend/vite.config.ts | 2 +- 7 files changed, 97 insertions(+), 74 deletions(-) diff --git a/frontend/src/lib/components/MentionDropdown.svelte b/frontend/src/lib/components/MentionDropdown.svelte index d86e912a..6a95b233 100644 --- a/frontend/src/lib/components/MentionDropdown.svelte +++ b/frontend/src/lib/components/MentionDropdown.svelte @@ -111,7 +111,7 @@ function selectItem(item: Person) { unauthenticated users. -->
void; + onmouseleave?: () => void; }; -let { personId, cardId, position, state }: Props = $props(); +let { personId, cardId, position, state, onmouseenter, onmouseleave }: Props = $props(); const FAMILY_REL_TYPES: ReadonlySet = new Set([ 'PARENT_OF', 'SPOUSE_OF', 'SIBLING_OF' ]); + +function relationLabel(type: RelationshipDTO['relationType']): string { + switch (type) { + case 'PARENT_OF': + return m.relation_parent_of(); + case 'SPOUSE_OF': + return m.relation_spouse_of(); + case 'SIBLING_OF': + return m.relation_sibling_of(); + default: + return m.relation_other(); + } +} const NOTES_MAX = 120; const familyChips = $derived( @@ -79,9 +94,11 @@ const ariaBusy = $derived(state.status === 'loading'); aria-live="polite" aria-label={ariaLabel} aria-busy={ariaBusy ? 'true' : undefined} - style:position="absolute" + style:position="fixed" style:top={`${position.top}px`} style:left={`${position.left}px`} + onmouseenter={onmouseenter} + onmouseleave={onmouseleave} > {#if state.status === 'loading'}
{dateRange}
{/if} - {#if state.person.alias} + {#if state.person.alias && state.person.alias !== state.person.lastName && state.person.alias !== state.person.displayName}
{m.person_born_name_prefix()} {state.person.alias} @@ -118,7 +135,10 @@ const ariaBusy = $derived(state.status === 'loading'); {#if familyChips.length > 0}
{#each familyChips as chip (chip.id)} - {chip.relatedPersonDisplayName} + + {relationLabel(chip.relationType)} + {chip.relatedPersonDisplayName} + {/each}
{/if} @@ -230,6 +250,9 @@ const ariaBusy = $derived(state.status === 'loading'); } .chip { + display: flex; + align-items: center; + gap: 4px; font-size: 12px; background-color: var(--c-accent-bg); color: var(--c-ink); @@ -237,6 +260,15 @@ const ariaBusy = $derived(state.status === 'loading'); padding: 2px 10px; } +.chip-type { + font-weight: 600; + opacity: 0.7; +} + +.chip-type::after { + content: ':'; +} + .notes { font-size: 13px; color: var(--c-ink-2); diff --git a/frontend/src/lib/components/PersonMentionEditor.svelte b/frontend/src/lib/components/PersonMentionEditor.svelte index 3a9188c1..52bd9b98 100644 --- a/frontend/src/lib/components/PersonMentionEditor.svelte +++ b/frontend/src/lib/components/PersonMentionEditor.svelte @@ -224,7 +224,6 @@ onMount(() => { role: 'textbox', 'aria-multiline': 'true', 'aria-label': m.transcription_editor_aria_label(), - ...(placeholder ? { 'data-placeholder': placeholder } : {}), class: [ 'min-h-[120px] px-1 py-2.5', 'font-serif text-base leading-relaxed text-ink', @@ -255,6 +254,20 @@ onDestroy(() => { editor?.destroy(); }); +// Keep the data-placeholder attribute in sync with actual emptiness so the +// placeholder CSS only fires when there is no content (not just on blur). +$effect(() => { + if (!editor || !placeholder) return; + void value; // track value as dependency so this re-runs on content changes + const inner = editorEl?.querySelector('.tiptap-editor-inner') as HTMLElement | null; + if (!inner) return; + if (editor.isEmpty) { + inner.setAttribute('data-placeholder', placeholder); + } else { + inner.removeAttribute('data-placeholder'); + } +}); + // Keep editor in sync with the reactive `disabled` prop. Tiptap's setEditable // flips contenteditable on the inner DOM and stops accepting input — matches // the textarea's old `disabled` semantics for keyboard users (WCAG 2.1.1). diff --git a/frontend/src/lib/components/TranscriptionReadView.svelte b/frontend/src/lib/components/TranscriptionReadView.svelte index 4a3ac949..ca0b59df 100644 --- a/frontend/src/lib/components/TranscriptionReadView.svelte +++ b/frontend/src/lib/components/TranscriptionReadView.svelte @@ -93,13 +93,28 @@ function getOrFetchHoverData(personId: string): Promise { function currentViewport() { return { viewportWidth: window.innerWidth, - viewportHeight: window.innerHeight, - scrollX: window.scrollX, - scrollY: window.scrollY + viewportHeight: window.innerHeight }; } +let closeTimer: ReturnType | null = null; + +function scheduleCardClose() { + closeTimer = setTimeout(() => { + activeCard = null; + closeTimer = null; + }, 150); +} + +function cancelCardClose() { + if (closeTimer !== null) { + clearTimeout(closeTimer); + closeTimer = null; + } +} + async function handleMentionEnter(event: Event) { + cancelCardClose(); const link = event.target as HTMLAnchorElement; const personId = link.dataset.personId; if (!personId) return; @@ -140,7 +155,11 @@ async function handleMentionEnter(event: Event) { function handleMentionLeave(event: Event) { const link = event.target as HTMLAnchorElement; link.removeAttribute('aria-describedby'); - activeCard = null; + if (event.type === 'mouseleave') { + scheduleCardClose(); + } else { + activeCard = null; + } } /** @@ -231,6 +250,8 @@ function attachMentionHandlers(node: HTMLElement) { cardId={activeCard.cardId} position={activeCard.position} state={activeCard.state} + onmouseenter={cancelCardClose} + onmouseleave={() => { activeCard = null; }} /> {/if} diff --git a/frontend/src/lib/utils/hoverCardPosition.spec.ts b/frontend/src/lib/utils/hoverCardPosition.spec.ts index e953f442..7ea13d0c 100644 --- a/frontend/src/lib/utils/hoverCardPosition.spec.ts +++ b/frontend/src/lib/utils/hoverCardPosition.spec.ts @@ -19,6 +19,8 @@ const makeRect = (overrides: Partial = {}): DOMRect => { } as DOMRect; }; +const vp = { viewportWidth: 1440, viewportHeight: 900 }; + describe('computeHoverCardPosition', () => { it('exports the spec constants used by the spec/CSS layer', () => { // Pin the values the design spec calls out — if these drift, the design spec @@ -33,12 +35,7 @@ describe('computeHoverCardPosition', () => { describe('default placement (below-right)', () => { it('positions the card below the rect with a small gap', () => { const rect = makeRect({ top: 100, bottom: 120, left: 200 }); - const result = computeHoverCardPosition(rect, { - viewportWidth: 1440, - viewportHeight: 900, - scrollX: 0, - scrollY: 0 - }); + const result = computeHoverCardPosition(rect, vp); expect(result.top).toBe(120 + CARD_GAP_PX); expect(result.left).toBe(200); }); @@ -48,24 +45,14 @@ describe('computeHoverCardPosition', () => { it('flips up when the card would overflow the bottom edge', () => { // Mention sits 50px above the viewport bottom — card is 180px tall, can't fit below const rect = makeRect({ top: 800, bottom: 850 }); - const result = computeHoverCardPosition(rect, { - viewportWidth: 1440, - viewportHeight: 900, - scrollX: 0, - scrollY: 0 - }); + const result = computeHoverCardPosition(rect, vp); expect(result.top).toBe(800 - CARD_HEIGHT_PX - CARD_GAP_PX); }); it('flips up when the mention sits in the bottom 30% of the viewport (BOTTOM_BAND_RATIO)', () => { // rect.top is at 80% of viewport — fits below numerically, but poor UX const rect = makeRect({ top: 720, bottom: 740 }); - const result = computeHoverCardPosition(rect, { - viewportWidth: 1440, - viewportHeight: 900, - scrollX: 0, - scrollY: 0 - }); + const result = computeHoverCardPosition(rect, vp); expect(result.top).toBe(720 - CARD_HEIGHT_PX - CARD_GAP_PX); }); }); @@ -74,12 +61,7 @@ describe('computeHoverCardPosition', () => { it('flips left when the rect is within RIGHT_FLIP_THRESHOLD_PX of the right edge', () => { // vw - rect.left = 1440 - 1200 = 240 < 300, so flip const rect = makeRect({ left: 1200, right: 1300, top: 100, bottom: 120 }); - const result = computeHoverCardPosition(rect, { - viewportWidth: 1440, - viewportHeight: 900, - scrollX: 0, - scrollY: 0 - }); + const result = computeHoverCardPosition(rect, { viewportWidth: 1440, viewportHeight: 900 }); // left = right - CARD_WIDTH = 1300 - 320 = 980 expect(result.left).toBe(980); }); @@ -87,12 +69,7 @@ describe('computeHoverCardPosition', () => { it('does not flip left when the rect has plenty of right-side room', () => { // vw - rect.left = 1440 - 200 = 1240 >> 300 → no flip const rect = makeRect({ left: 200, right: 300 }); - const result = computeHoverCardPosition(rect, { - viewportWidth: 1440, - viewportHeight: 900, - scrollX: 0, - scrollY: 0 - }); + const result = computeHoverCardPosition(rect, vp); expect(result.left).toBe(200); }); }); @@ -103,50 +80,32 @@ describe('computeHoverCardPosition', () => { // Without clamping the card would be at left=0 but extend to 320 — fine. // At viewport=400px with rect.left=200, flip puts left=300-320=-20, clamped to 0. const rect = makeRect({ left: 200, right: 300, top: 100, bottom: 120 }); - const result = computeHoverCardPosition(rect, { - viewportWidth: 400, - viewportHeight: 900, - scrollX: 0, - scrollY: 0 - }); + const result = computeHoverCardPosition(rect, { viewportWidth: 400, viewportHeight: 900 }); expect(result.left).toBeGreaterThanOrEqual(0); expect(result.left + CARD_WIDTH_PX).toBeLessThanOrEqual(400); }); it('never returns a negative top or left', () => { const rect = makeRect({ top: -50, left: -100, bottom: -30, right: 0 }); - const result = computeHoverCardPosition(rect, { - viewportWidth: 1440, - viewportHeight: 900, - scrollX: 0, - scrollY: 0 - }); + const result = computeHoverCardPosition(rect, vp); expect(result.top).toBeGreaterThanOrEqual(0); expect(result.left).toBeGreaterThanOrEqual(0); }); }); - describe('scroll offset', () => { - it('adds window.scrollY to the absolute-positioned top', () => { + describe('position: fixed (viewport-relative coordinates)', () => { + it('returns viewport-relative top — does not add scroll offset', () => { + // getBoundingClientRect values are already viewport-relative; with position:fixed + // we use them directly without adding scrollY. const rect = makeRect({ top: 100, bottom: 120 }); - const result = computeHoverCardPosition(rect, { - viewportWidth: 1440, - viewportHeight: 900, - scrollX: 0, - scrollY: 500 - }); - expect(result.top).toBe(120 + CARD_GAP_PX + 500); + const result = computeHoverCardPosition(rect, vp); + expect(result.top).toBe(120 + CARD_GAP_PX); }); - it('adds window.scrollX to the absolute-positioned left', () => { + it('returns viewport-relative left — does not add scroll offset', () => { const rect = makeRect({ top: 100, bottom: 120, left: 200, right: 300 }); - const result = computeHoverCardPosition(rect, { - viewportWidth: 1440, - viewportHeight: 900, - scrollX: 50, - scrollY: 0 - }); - expect(result.left).toBe(200 + 50); + const result = computeHoverCardPosition(rect, vp); + expect(result.left).toBe(200); }); }); }); diff --git a/frontend/src/lib/utils/hoverCardPosition.ts b/frontend/src/lib/utils/hoverCardPosition.ts index 4d16a899..1223ab7d 100644 --- a/frontend/src/lib/utils/hoverCardPosition.ts +++ b/frontend/src/lib/utils/hoverCardPosition.ts @@ -32,8 +32,6 @@ export const RIGHT_FLIP_THRESHOLD_PX = 300; export type Viewport = { viewportWidth: number; viewportHeight: number; - scrollX: number; - scrollY: number; }; export type CardPosition = { top: number; left: number }; @@ -63,7 +61,7 @@ export function computeHoverCardPosition(rect: DOMRect, vp: Viewport): CardPosit left = Math.min(left, vp.viewportWidth - CARD_WIDTH_PX - CARD_GAP_PX); return { - top: Math.max(0, top + vp.scrollY), - left: Math.max(0, left + vp.scrollX) + top: Math.max(0, top), + left: Math.max(0, left) }; } diff --git a/frontend/vite.config.ts b/frontend/vite.config.ts index 38a3d537..1405f48f 100644 --- a/frontend/vite.config.ts +++ b/frontend/vite.config.ts @@ -7,7 +7,7 @@ import { sveltekit } from '@sveltejs/kit/vite'; export default defineConfig({ optimizeDeps: { - include: ['pdfjs-dist'] + include: ['pdfjs-dist', '@tiptap/core', '@tiptap/starter-kit', '@tiptap/extension-mention'] }, server: { host: '0.0.0.0', // Erlaubt Zugriff von außen