feat: auto-open transcription panel when navigating from mission-control cards #377
Reference in New Issue
Block a user
Delete Branch "feat/issue-376-auto-open-transcription-panel"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #376
Summary
Mission-control card links in the "Text markieren" and "Text transkribieren" columns now append
?task=transcribeto the document URL. On the document page,onMountreads this param and — before the comment deep-link handler runs — opens the transcription panel, scrolls the close button into view, moves focus to it, then strips the param from the URL viareplaceState. Users navigating from a mission-control card now land directly in the panel without any additional interaction.👨💻 Felix Brandt — Senior Fullstack Developer
Verdict: ⚠️ Approved with concerns
Blockers
None.
Suggestions
+page.svelte—tick().then()has no.catch(): The immediately adjacentscrollToCommentFromQuery(...).catch((e) => console.error(...))sets a clear precedent for handling promise rejections. WhilescrollIntoView,focus, andreplaceStateare unlikely to throw, the.then()chain is unhandled. A silent failure here would be invisible. Add.catch((e) => console.error('task deep-link failed', e))to match the pattern.What I checked
onMountkept synchronous (noasynckeyword) — correct, avoids breaking Svelte's cleanup detection ✅tick().then()sequence:transcribeMode = true→ tick → scroll/focus → replaceState — correct order ✅scrollToCommentFromQuerygetsnew URL(page.url)beforereplaceStateruns, but that's fine — it only readscommentId, nottask✅🏛️ Markus Keller — Application Architect
Verdict: ✅ Approved
What I checked
onMount, entirely separate fromscrollToCommentFromQuery— the utility's single responsibility is preserved ✅scrollToCommentFromQuery, sotranscribeModeis alreadytrueif both params are somehow present ✅replaceState(page.url.pathname, ...)is correct:page.url.pathnameis the path without query string — this is the correct strip ✅No architectural concerns.
🔒 Nora "NullX" Steiner — Security Engineer
Verdict: ✅ Approved
What I checked
page.url.searchParams.get('task') === 'transcribe'— exact string match, never rendered to DOM ✅onMount(client-side only), never forwarded to the backend ✅replaceStatestrips cleanly: usespage.url.pathname(path only, no query), so the param does not linger in referrer headers on subsequent navigations ✅+page.server.tsis completely unaffected ✅Zero security concerns.
🧪 Sara Holt — QA Engineer
Verdict: ✅ Approved
What I checked
SegmentationColumn.svelte.spec.ts: link assertion updated from/documents/abc-123to/documents/abc-123?task=transcribe✅TranscriptionColumn.svelte.spec.ts: link assertion updated from/documents/xyz-456to/documents/xyz-456?task=transcribe✅Observation
The
tick().then()in+page.sveltehas no.catch(). If the browser throws duringscrollIntoVieworreplaceState(edge case, e.g. detached DOM during fast navigation), the failure would be silent. Felix flagged this too — worth a one-liner fix.🎨 Leonie Voss — UX Designer & Accessibility Strategist
Verdict: ✅ Approved
What I checked
closeBtn?.focus({ preventScroll: true })afterscrollIntoView— keyboard and screen reader users land on the close button with focus, which is immediately actionable ✅prefersReducedMotionrespected:behavior: prefersReducedMotion ? 'instant' : 'smooth'— vestibular-sensitive users get instant positioning ✅replaceState(page.url.pathname, ...)cleans the address bar — copy-pasted document URLs won't surprise recipients with an auto-open panel ✅block: 'nearest': correct choice — scrolls the minimum necessary to bring the panel header into view, avoids jarring jumps if the panel is already partially visible ✅Minor observation
The close button (
[data-testid="panel-close"]) is the correct focus target for immediate orientation. A future enhancement (not this PR) would be to also setaria-expandedon the panel trigger button so the open/closed state is programmatically announced — this is an existing gap, not introduced here.🚀 Tobias Wendt — DevOps & Platform Engineer
Verdict: ✅ Approved
What I checked
npm run checkpasses on the changed file,npm run lintpasses (Prettier + ESLint) ✅?task=transcribecontinue to work exactly as before — zero rollback risk ✅onMountis client-side only — SSR renders the closed-panel state and hydration opens it; no hydration mismatch ✅Nothing to flag from my side.
Review Cycle 1 — Changes
Addressed
tick().then()missing.catch()— added.catch((e) => console.error('task deep-link failed', e))to match the existing deep-link scroll pattern — commita81a6e72Deferred to new issues
(none)
Re-running review cycle 2.
👨💻 Felix Brandt — Senior Fullstack Developer
Verdict: ✅ Approved
Cycle 1 concern addressed —
tick().then().catch()now matches the pattern of thescrollToCommentFromQuerychain directly below it. No remaining blockers.What I checked
.catch((e) => console.error('task deep-link failed', e))is in place — commita81a6e72✅tick()ordering:transcribeMode = trueis set synchronously before thetick()call, so the panel DOM is guaranteed to exist when.then()fires ✅closeBtn:closeBtn?.scrollIntoView(...)andcloseBtn?.focus(...)fail silently if the panel didn't render — correct defensive choice since it's an optional UX enhancement ✅'links to /documents/{id}?task=transcribe'is exact and sentence-readable ✅prefersReducedMotionas$derived: evaluated at render time fromwindow.matchMedia, consistent with existing usage ✅replaceState(page.url.pathname, page.state ?? {}): strips the query param from history cleanly —pathnameexcludes search, correct ✅Suggestion (non-blocking)
The
+page.svelteonMounttask-param branch has no direct vitest-browser test — only the column link URLs are tested at the component level. Given the "component tests only" decision from the discussion phase, this is acceptable. If E2E coverage is ever added for the document page, a test assertingtranscribeModeactivates on?task=transcribewould round out the pyramid.🏛️ Markus Keller — Application Architect
Verdict: ✅ Approved
Clean, minimal solution. No architectural concerns.
What I checked
?task=transcribelogic lives entirely in the frontend route component'sonMount. It reads a URL param, sets local state, and strips the param. No backend change, no new service, no new dependency ✅?task=transcribeis a well-established query-param deep-link pattern. UsingreplaceStateto clean the param from history is correct — the param is ephemeral intent, not persistent state ✅scrollToCommentFromQuery: the task block runs before the comment deep-link handler, sotranscribeModeis alreadytruewhen the comment handler evaluates it. Correct execution order ✅SegmentationColumn,TranscriptionColumn, and+page.svelteonly. All within the same SvelteKit route. No service boundaries crossed ✅This is the simplest solution that solves the onboarding problem. Nothing to add.
🔒 Nora "NullX" Steiner — Application Security Engineer
Verdict: ✅ Approved
No security concerns in this diff.
What I checked
page.url.searchParams.get('task')is read and compared to the literal string'transcribe'. If it matches, a boolean flag is set. The param value is never reflected into the DOM, logged, or sent to the API. No XSS vector ✅replaceState(page.url.pathname, page.state ?? {})uses the current pathname — a server-controlled value — not the user-supplied query string. Clean ✅document.querySelector('[data-testid="panel-close"]'): purely a DOM traversal for focus management. No input is read from this element, no user content is rendered ✅Nothing to flag here.
🧪 Sara Holt — QA Engineer & Test Strategist
Verdict: ✅ Approved
Test coverage is appropriate for the agreed scope. No blockers.
What I checked
SegmentationColumn.svelte.spec.tsandTranscriptionColumn.svelte.spec.tsassert the new?task=transcribehref value withgetByRole('link')+toHaveAttribute('href', ...). Behavior-based, not internal-state-based ✅makeDoc()with overrides is already in both files — no repeated builder construction ✅'links to /documents/{id}?task=transcribe'reads as a complete behavior description ✅.catch()is present ✅Observation (non-blocking)
The
onMounttask-param activation path in+page.sveltehas no dedicated component test — verified by the discussion decision to limit to component-level tests for this PR. The column tests confirm the correct URL is generated; the activation behavior itself would require either a route-level integration test or a Playwright E2E test. Either is reasonable as a follow-up if confidence is needed there.🎨 Leonie Voss — UX Designer & Accessibility Strategist
Verdict: ✅ Approved
Focus management and motion handling are done correctly. No accessibility concerns.
What I checked
prefersReducedMotionrespected:scrollIntoView({ behavior: prefersReducedMotion ? 'instant' : 'smooth' })— users with vestibular sensitivity get no scroll animation ✅block: 'nearest': scrolls only if the close button is outside the viewport, doesn't force a jarring scroll if it's already visible ✅closeBtn?.focus({ preventScroll: true })— keyboard users and screen reader users land directly in the panel without having to tab through the document content ✅data-testid="panel-close"]maps to a button witharia-label={m.transcription_panel_close()}— screen readers will announce the correct label when focus lands ✅replaceState: the address bar is clean after navigation — no?task=transcribepolluting share links or bookmarks ✅min-h-[44px]on the<a>elements — senior-friendly (60+ users using the mission-control view) ✅The onboarding flow is now: click from mission-control → land in document → panel opens → focus is on the close button → clear exit path. That's the right mental model for a first-time user.
🛠️ Tobias Wendt — DevOps & Platform Engineer
Verdict: ✅ Approved
No infrastructure, CI, or deployment impact.
What I checked
npm run test(Vitest) andnpm run check(svelte-check) gates cover the changed files ✅?task=transcribefeature is pure client-side URL routing. No environment variables, no feature flags, no backend profile changes ✅Clean frontend-only change. Nothing to do on the infra side.