Split TranscriptionEditView.svelte (332 lines) — extract auto-save + drag-drop modules #199

New Issue

marcel · 2026-04-07T10:48:26+02:00

marcel commented

2026-04-07 10:48:26 +02:00

Context

TranscriptionEditView.svelte is 332 lines mixing three independent concerns: debounced auto-save state machine, pointer-based drag-and-drop reordering, and block list rendering. The auto-save and drag logic are pure imperative code with zero template coupling.

Proposed Split

1. `useBlockAutoSave.svelte.ts` (~100 lines)

Extract the debounced save state machine:

SvelteMap-based saveStates, debounceTimers, pendingTexts
getSaveState, setSaveState, executeSave, scheduleSavedFade, scheduleDebounce, clearDebounce, flushAllPending, flushViaBeacon
beforeunload listener setup
Exposed: { getSaveState, handleTextChange, handleRetry, handleDelete, handleBlur }

2. `useBlockDragDrop.svelte.ts` (~60 lines)

Extract pointer-based drag-and-drop:

State: draggedBlockId, dropTargetIdx, dragOffsetY
handleGripDown, handlePointerMove, handlePointerUp
Exposed: { draggedBlockId, dropTargetIdx, dragOffsetY, handleGripDown, handlePointerMove, handlePointerUp }

3. `TranscriptionEditView.svelte` becomes orchestrator (~80 lines)

Imports both hooks
Renders sorted block list with TranscriptionBlock components
Passes hook callbacks as props

Acceptance Criteria

TranscriptionEditView.svelte under ~100 lines
Auto-save with debounce, saved/fading feedback, error retry all work identically
Drag-and-drop reordering with drop indicator works identically
sendBeacon flush on page unload still works
npm run check passes

## Context `TranscriptionEditView.svelte` is 332 lines mixing three independent concerns: debounced auto-save state machine, pointer-based drag-and-drop reordering, and block list rendering. The auto-save and drag logic are pure imperative code with zero template coupling. ## Proposed Split ### 1. `useBlockAutoSave.svelte.ts` (~100 lines) Extract the debounced save state machine: - SvelteMap-based `saveStates`, `debounceTimers`, `pendingTexts` - `getSaveState`, `setSaveState`, `executeSave`, `scheduleSavedFade`, `scheduleDebounce`, `clearDebounce`, `flushAllPending`, `flushViaBeacon` - `beforeunload` listener setup - Exposed: `{ getSaveState, handleTextChange, handleRetry, handleDelete, handleBlur }` ### 2. `useBlockDragDrop.svelte.ts` (~60 lines) Extract pointer-based drag-and-drop: - State: `draggedBlockId`, `dropTargetIdx`, `dragOffsetY` - `handleGripDown`, `handlePointerMove`, `handlePointerUp` - Exposed: `{ draggedBlockId, dropTargetIdx, dragOffsetY, handleGripDown, handlePointerMove, handlePointerUp }` ### 3. `TranscriptionEditView.svelte` becomes orchestrator (~80 lines) - Imports both hooks - Renders sorted block list with TranscriptionBlock components - Passes hook callbacks as props ## Acceptance Criteria - [ ] TranscriptionEditView.svelte under ~100 lines - [ ] Auto-save with debounce, saved/fading feedback, error retry all work identically - [ ] Drag-and-drop reordering with drop indicator works identically - [ ] `sendBeacon` flush on page unload still works - [ ] `npm run check` passes

marcel added the refactor label 2026-04-07 10:49:02 +02:00

marcel commented

2026-04-07 10:51:31 +02:00

👨‍💻 Felix Brandt -- Senior Fullstack Developer

Good decomposition -- extracting imperative logic into .svelte.ts modules is exactly the right pattern for Svelte 5. A few things I'd want clarified before implementation:

Naming and API surface

The use* prefix suggests these return reactive state. Confirm: will useBlockAutoSave and useBlockDragDrop be factory functions that return an object of callbacks + reactive state, or will they export standalone functions?
The issue lists handleTextChange, handleRetry, handleDelete, handleBlur as exposed from the auto-save hook. handleDelete feels like it belongs to block management, not auto-save. Is its only purpose clearing the save timer for a deleted block, or does it also handle the DELETE API call? If the latter, it crosses concerns.

Reactive collections

The issue mentions SvelteMap-based saveStates, debounceTimers, pendingTexts. Since these live inside a .svelte.ts module, confirm they'll be initialized with $state() wrapping or as new SvelteMap() instances so Svelte's reactivity tracks them. A plain Map inside a module file would silently break reactivity in the consuming component.

Testing strategy

The auto-save state machine (idle -> saving -> saved -> fading) is pure logic with timer side effects. This is highly unit-testable with fake timers in Vitest. I'd want a test plan before implementation:
- scheduleSavedFade transitions saved -> idle after timeout
- scheduleDebounce coalesces rapid text changes into one save
- flushAllPending fires immediately for all dirty blocks
- flushViaBeacon uses navigator.sendBeacon, not fetch
The drag-drop module is harder to unit test (pointer events), but the reorder logic (computing new sort order from dropTargetIdx) should be extractable and testable in isolation.

Component size target

The issue targets ~80 lines for the orchestrator. That's reasonable if TranscriptionBlock already exists as a separate component. If the block rendering is currently inline in TranscriptionEditView, this refactor implicitly creates a third extraction (TranscriptionBlock.svelte) not mentioned in the issue. Worth confirming scope.

Suggestion

Consider whether executeSave should accept the API call as an injected dependency (callback parameter) rather than importing the fetch logic directly. This makes the module testable without mocking network calls and aligns with dependency inversion.

## 👨‍💻 Felix Brandt -- Senior Fullstack Developer Good decomposition -- extracting imperative logic into `.svelte.ts` modules is exactly the right pattern for Svelte 5. A few things I'd want clarified before implementation: ### Naming and API surface - The `use*` prefix suggests these return reactive state. Confirm: will `useBlockAutoSave` and `useBlockDragDrop` be factory functions that return an object of callbacks + reactive state, or will they export standalone functions? - The issue lists `handleTextChange`, `handleRetry`, `handleDelete`, `handleBlur` as exposed from the auto-save hook. `handleDelete` feels like it belongs to block management, not auto-save. Is its only purpose clearing the save timer for a deleted block, or does it also handle the DELETE API call? If the latter, it crosses concerns. ### Reactive collections - The issue mentions `SvelteMap`-based `saveStates`, `debounceTimers`, `pendingTexts`. Since these live inside a `.svelte.ts` module, confirm they'll be initialized with `$state()` wrapping or as `new SvelteMap()` instances so Svelte's reactivity tracks them. A plain `Map` inside a module file would silently break reactivity in the consuming component. ### Testing strategy - The auto-save state machine (idle -> saving -> saved -> fading) is pure logic with timer side effects. This is highly unit-testable with fake timers in Vitest. I'd want a test plan before implementation: - `scheduleSavedFade` transitions `saved -> idle` after timeout - `scheduleDebounce` coalesces rapid text changes into one save - `flushAllPending` fires immediately for all dirty blocks - `flushViaBeacon` uses `navigator.sendBeacon`, not `fetch` - The drag-drop module is harder to unit test (pointer events), but the reorder logic (computing new sort order from `dropTargetIdx`) should be extractable and testable in isolation. ### Component size target - The issue targets ~80 lines for the orchestrator. That's reasonable if `TranscriptionBlock` already exists as a separate component. If the block rendering is currently inline in `TranscriptionEditView`, this refactor implicitly creates a third extraction (`TranscriptionBlock.svelte`) not mentioned in the issue. Worth confirming scope. ### Suggestion - Consider whether `executeSave` should accept the API call as an injected dependency (callback parameter) rather than importing the fetch logic directly. This makes the module testable without mocking network calls and aligns with dependency inversion.

marcel commented

2026-04-07 10:51:48 +02:00

🏗️ Markus Keller -- Senior Application Architect

Clean module boundary identification. The three concerns (auto-save state machine, drag-drop interaction, block list rendering) are genuinely independent -- this is not premature extraction. A few architectural considerations:

Module boundary contracts

The issue describes what each module exposes but not the input contract. useBlockAutoSave needs to know: the block ID, the current text, and an API call to persist. How does it get these? Options:
1. Factory function that receives a saveFn: (blockId: string, text: string) => Promise<void> -- cleanest, fully decoupled
2. Module imports the API client directly -- couples the module to the transport layer
Option 1 is strongly preferred. It keeps both modules as pure behavioral utilities with no knowledge of the Familienarchiv API shape.

State ownership

The issue places draggedBlockId, dropTargetIdx, and dragOffsetY inside useBlockDragDrop. But who owns the canonical block order? If the orchestrator owns blocks: Block[] and the drag module just signals "move block X to position Y", the boundary is clean. If the drag module mutates the block array directly, you've split ownership and created a coordination problem.
Recommendation: useBlockDragDrop should expose a onReorder: (fromId: string, toIdx: number) => void callback that the orchestrator implements. The drag module manages visual state (ghost position, drop indicator); the orchestrator manages data state (block order, API call to persist new order).

`beforeunload` and `sendBeacon` placement

The issue puts the beforeunload listener in useBlockAutoSave. This is correct -- the auto-save module is the only one that knows about pending unsaved text. But confirm: does the listener also need to abort in-flight drag operations? If a user closes the tab mid-drag, is there any state to clean up? Probably not, but worth a one-line check.

Accidental complexity check

332 lines splitting into ~100 + ~60 + ~80 = ~240 lines plus two new files. The total line count increases slightly (import boilerplate, type definitions for the hook return shapes). This is acceptable -- the cognitive load per file drops significantly, which is the actual goal. Just ensure the type definitions for the hook return objects are explicit, not inferred, so the contract is readable at the import site.

No over-extraction

The issue correctly stops at two modules. There's no "useBlockList" or "useBlockRendering" -- the orchestrator handles that directly. Good restraint. Keep it.

## 🏗️ Markus Keller -- Senior Application Architect Clean module boundary identification. The three concerns (auto-save state machine, drag-drop interaction, block list rendering) are genuinely independent -- this is not premature extraction. A few architectural considerations: ### Module boundary contracts - The issue describes what each module exposes but not the input contract. `useBlockAutoSave` needs to know: the block ID, the current text, and an API call to persist. How does it get these? Options: 1. Factory function that receives a `saveFn: (blockId: string, text: string) => Promise<void>` -- cleanest, fully decoupled 2. Module imports the API client directly -- couples the module to the transport layer - Option 1 is strongly preferred. It keeps both modules as pure behavioral utilities with no knowledge of the Familienarchiv API shape. ### State ownership - The issue places `draggedBlockId`, `dropTargetIdx`, and `dragOffsetY` inside `useBlockDragDrop`. But who owns the canonical block order? If the orchestrator owns `blocks: Block[]` and the drag module just signals "move block X to position Y", the boundary is clean. If the drag module mutates the block array directly, you've split ownership and created a coordination problem. - Recommendation: `useBlockDragDrop` should expose a `onReorder: (fromId: string, toIdx: number) => void` callback that the orchestrator implements. The drag module manages visual state (ghost position, drop indicator); the orchestrator manages data state (block order, API call to persist new order). ### `beforeunload` and `sendBeacon` placement - The issue puts the `beforeunload` listener in `useBlockAutoSave`. This is correct -- the auto-save module is the only one that knows about pending unsaved text. But confirm: does the listener also need to abort in-flight drag operations? If a user closes the tab mid-drag, is there any state to clean up? Probably not, but worth a one-line check. ### Accidental complexity check - 332 lines splitting into ~100 + ~60 + ~80 = ~240 lines plus two new files. The total line count increases slightly (import boilerplate, type definitions for the hook return shapes). This is acceptable -- the cognitive load per file drops significantly, which is the actual goal. Just ensure the type definitions for the hook return objects are explicit, not inferred, so the contract is readable at the import site. ### No over-extraction - The issue correctly stops at two modules. There's no "useBlockList" or "useBlockRendering" -- the orchestrator handles that directly. Good restraint. Keep it.

marcel commented

2026-04-07 10:52:04 +02:00

🧪 Sara Holt -- Senior QA Engineer

This refactor is a testability win -- extracting the auto-save state machine and drag-drop logic into standalone modules makes them independently testable for the first time. Here's what I'd want covered:

Auto-save module test plan

The state machine (idle -> saving -> saved -> fading -> idle, with error branching) is the highest-value test target:

Debounce coalescing: rapid handleTextChange calls within the debounce window should result in exactly one executeSave call
Concurrent edits on different blocks: block A saving should not block or delay block B's debounce
Error -> retry flow: executeSave failure sets state to error; handleRetry re-triggers save with the last pending text
flushAllPending: fires saves for all blocks with pending text, regardless of debounce timers
flushViaBeacon: uses navigator.sendBeacon (not fetch), and gracefully handles sendBeacon returning false (payload too large)
Timer cleanup: when a block is deleted, its debounce timer and fade timer are cleared (no orphaned setTimeout references)

All of these are testable with vi.useFakeTimers() in Vitest -- no DOM or browser needed.

Drag-drop module test plan

Pointer event sequence: handleGripDown -> handlePointerMove -> handlePointerUp produces a reorder callback with correct fromId and toIdx
No-op drag: releasing on the same position does not trigger a reorder
Edge positions: dragging to position 0 (top) and position N (bottom) compute correctly
Interrupted drag: if handlePointerUp fires without a preceding handleGripDown (e.g., focus lost), no crash

Regression coverage

The acceptance criteria say "auto-save with debounce, saved/fading feedback, error retry all work identically." This needs an E2E test that exercises the full save cycle on the transcription edit page -- type text, wait for auto-save indicator, verify saved state. If this E2E test doesn't exist yet, it should be added as part of this issue, not deferred.
Same for drag-and-drop: an E2E test that drags a block and verifies the new order persists after page reload.

Flakiness risk

Timer-based tests (debounce, fade) are classic flaky test sources. Use vi.useFakeTimers() and vi.advanceTimersByTime() exclusively -- never setTimeout with real timers in tests.
Pointer event simulation in tests can be fragile. If the drag-drop unit tests rely on PointerEvent construction, ensure the test environment supports it (jsdom has limitations here). Consider testing the reorder logic separately from the pointer event handling.

## 🧪 Sara Holt -- Senior QA Engineer This refactor is a testability win -- extracting the auto-save state machine and drag-drop logic into standalone modules makes them independently testable for the first time. Here's what I'd want covered: ### Auto-save module test plan The state machine (idle -> saving -> saved -> fading -> idle, with error branching) is the highest-value test target: - **Debounce coalescing**: rapid `handleTextChange` calls within the debounce window should result in exactly one `executeSave` call - **Concurrent edits on different blocks**: block A saving should not block or delay block B's debounce - **Error -> retry flow**: `executeSave` failure sets state to `error`; `handleRetry` re-triggers save with the last pending text - **`flushAllPending`**: fires saves for all blocks with pending text, regardless of debounce timers - **`flushViaBeacon`**: uses `navigator.sendBeacon` (not `fetch`), and gracefully handles `sendBeacon` returning `false` (payload too large) - **Timer cleanup**: when a block is deleted, its debounce timer and fade timer are cleared (no orphaned `setTimeout` references) All of these are testable with `vi.useFakeTimers()` in Vitest -- no DOM or browser needed. ### Drag-drop module test plan - **Pointer event sequence**: `handleGripDown` -> `handlePointerMove` -> `handlePointerUp` produces a reorder callback with correct `fromId` and `toIdx` - **No-op drag**: releasing on the same position does not trigger a reorder - **Edge positions**: dragging to position 0 (top) and position N (bottom) compute correctly - **Interrupted drag**: if `handlePointerUp` fires without a preceding `handleGripDown` (e.g., focus lost), no crash ### Regression coverage - The acceptance criteria say "auto-save with debounce, saved/fading feedback, error retry all work identically." This needs an E2E test that exercises the full save cycle on the transcription edit page -- type text, wait for auto-save indicator, verify saved state. If this E2E test doesn't exist yet, it should be added as part of this issue, not deferred. - Same for drag-and-drop: an E2E test that drags a block and verifies the new order persists after page reload. ### Flakiness risk - Timer-based tests (debounce, fade) are classic flaky test sources. Use `vi.useFakeTimers()` and `vi.advanceTimersByTime()` exclusively -- never `setTimeout` with real timers in tests. - Pointer event simulation in tests can be fragile. If the drag-drop unit tests rely on `PointerEvent` construction, ensure the test environment supports it (jsdom has limitations here). Consider testing the reorder logic separately from the pointer event handling.

marcel commented

2026-04-07 10:52:19 +02:00

🔒 Nora "NullX" Steiner -- Application Security Engineer

This is a refactor, not new functionality, so the attack surface should remain unchanged. That said, a few things to verify during implementation:

`sendBeacon` payload integrity

flushViaBeacon sends unsaved text to the backend on page unload. Confirm:
- The beacon endpoint validates the session cookie / auth token the same way as the normal save endpoint. sendBeacon sends cookies by default, but if the app uses a custom Authorization header, that header is NOT sent with beacon requests. This is a common gap that results in either (a) unauthenticated writes or (b) silent data loss.
- The backend endpoint receiving beacon data validates the payload identically to the normal save path -- no special "trust because beacon" logic.
- The Content-Type is explicitly set. sendBeacon with a Blob defaults to text/plain, which may bypass Spring's @RequestBody JSON parsing.

Drag-and-drop reorder authorization

When a block reorder is persisted to the backend, the API call should validate that the user has write permission on the document. This is presumably already the case, but confirm the reorder endpoint uses @RequirePermission(Permission.WRITE_ALL) or equivalent.
Validate that the reorder payload cannot reference block IDs from a different document (IDOR on block reorder). The backend should verify all block IDs belong to the document being edited.

SvelteKit client-side navigation can leave orphaned timers if the component is destroyed without triggering beforeunload. Confirm that the auto-save module cleans up debounce and fade timers in an onDestroy or equivalent lifecycle hook, not just in beforeunload. Otherwise, navigating away from the transcription page (without closing the tab) could leak timers or fire saves after the component is unmounted.

No new XSS surface

The transcription text is user-supplied content. As long as the refactored modules pass text through the same rendering path (no new {@html} usage), the XSS posture is unchanged. Worth a quick grep after implementation to confirm no {@html} was introduced.

## 🔒 Nora "NullX" Steiner -- Application Security Engineer This is a refactor, not new functionality, so the attack surface should remain unchanged. That said, a few things to verify during implementation: ### `sendBeacon` payload integrity - `flushViaBeacon` sends unsaved text to the backend on page unload. Confirm: - The beacon endpoint validates the session cookie / auth token the same way as the normal save endpoint. `sendBeacon` sends cookies by default, but if the app uses a custom `Authorization` header, that header is NOT sent with beacon requests. This is a common gap that results in either (a) unauthenticated writes or (b) silent data loss. - The backend endpoint receiving beacon data validates the payload identically to the normal save path -- no special "trust because beacon" logic. - The Content-Type is explicitly set. `sendBeacon` with a `Blob` defaults to `text/plain`, which may bypass Spring's `@RequestBody` JSON parsing. ### Drag-and-drop reorder authorization - When a block reorder is persisted to the backend, the API call should validate that the user has write permission on the document. This is presumably already the case, but confirm the reorder endpoint uses `@RequirePermission(Permission.WRITE_ALL)` or equivalent. - Validate that the reorder payload cannot reference block IDs from a different document (IDOR on block reorder). The backend should verify all block IDs belong to the document being edited. ### Timer cleanup on navigation - SvelteKit client-side navigation can leave orphaned timers if the component is destroyed without triggering `beforeunload`. Confirm that the auto-save module cleans up debounce and fade timers in an `onDestroy` or equivalent lifecycle hook, not just in `beforeunload`. Otherwise, navigating away from the transcription page (without closing the tab) could leak timers or fire saves after the component is unmounted. ### No new XSS surface - The transcription text is user-supplied content. As long as the refactored modules pass text through the same rendering path (no new `{@html}` usage), the XSS posture is unchanged. Worth a quick grep after implementation to confirm no `{@html}` was introduced.

marcel commented

2026-04-07 10:52:33 +02:00

🎨 Leonie Voss -- UI/UX Design Lead

This is a code-level refactor, so the user-facing behavior should be pixel-identical. My concerns are about preserving the existing UX during extraction:

Auto-save feedback states

The current save indicator presumably shows states like "Saving...", "Saved", and "Error - Retry". After extraction, confirm:
- The getSaveState reactive value updates synchronously with the UI -- no extra render frame delay introduced by the module boundary.
- The "saved" -> "fading" transition timing is preserved exactly. Users develop muscle memory for these micro-interactions. A 50ms difference in fade timing feels wrong even if they can't articulate why.
- The error state includes a visible, focusable retry affordance (not just a color change). If the current implementation already has this, the refactor must preserve it.

Drag-and-drop visual feedback

The drop indicator (showing where the block will land) is driven by dropTargetIdx. Confirm:
- The indicator renders between blocks, not on top of them -- this is a common regression when separating drag state from render logic.
- The dragged block's visual "ghost" (offset by dragOffsetY) remains smooth during the drag. If the pointer move handler is now in a separate module, verify there's no additional reactivity overhead causing jank.
- Touch targets on the drag grip remain at least 44x44px. The grip handle is often the smallest interactive element in a block list -- easy to accidentally shrink during refactoring.

Accessibility preservation

Drag-and-drop by pointer is inaccessible to keyboard-only users. Does the current implementation have keyboard reorder support (e.g., arrow keys on a focused block)? If not, this refactor is a good moment to note that as a follow-up issue -- not to scope-creep this one, but to track the gap.
Auto-save status changes should be announced to screen readers via an ARIA live region. Confirm the live region setup is preserved in the orchestrator, not lost during extraction.

Suggestion

If the save state per block is exposed as a reactive value from the hook, the orchestrator can trivially map it to ARIA attributes (aria-busy="true" during saving, aria-label update on error). Worth checking if this is already in place.

## 🎨 Leonie Voss -- UI/UX Design Lead This is a code-level refactor, so the user-facing behavior should be pixel-identical. My concerns are about preserving the existing UX during extraction: ### Auto-save feedback states - The current save indicator presumably shows states like "Saving...", "Saved", and "Error - Retry". After extraction, confirm: - The `getSaveState` reactive value updates synchronously with the UI -- no extra render frame delay introduced by the module boundary. - The "saved" -> "fading" transition timing is preserved exactly. Users develop muscle memory for these micro-interactions. A 50ms difference in fade timing feels wrong even if they can't articulate why. - The error state includes a visible, focusable retry affordance (not just a color change). If the current implementation already has this, the refactor must preserve it. ### Drag-and-drop visual feedback - The drop indicator (showing where the block will land) is driven by `dropTargetIdx`. Confirm: - The indicator renders between blocks, not on top of them -- this is a common regression when separating drag state from render logic. - The dragged block's visual "ghost" (offset by `dragOffsetY`) remains smooth during the drag. If the pointer move handler is now in a separate module, verify there's no additional reactivity overhead causing jank. - Touch targets on the drag grip remain at least 44x44px. The grip handle is often the smallest interactive element in a block list -- easy to accidentally shrink during refactoring. ### Accessibility preservation - Drag-and-drop by pointer is inaccessible to keyboard-only users. Does the current implementation have keyboard reorder support (e.g., arrow keys on a focused block)? If not, this refactor is a good moment to note that as a follow-up issue -- not to scope-creep this one, but to track the gap. - Auto-save status changes should be announced to screen readers via an ARIA live region. Confirm the live region setup is preserved in the orchestrator, not lost during extraction. ### Suggestion - If the save state per block is exposed as a reactive value from the hook, the orchestrator can trivially map it to ARIA attributes (`aria-busy="true"` during saving, `aria-label` update on error). Worth checking if this is already in place.

marcel commented

2026-04-07 10:52:47 +02:00

⚙️ Tobias Wendt -- DevOps & Platform Engineer

Pure frontend refactor -- no infrastructure changes needed. A few things to verify from the build and CI perspective:

Build impact

Two new .svelte.ts files should be tree-shaken correctly by Vite. No bundle size increase expected, but worth checking the build output diff before and after:
```
npm run build 2>&1 | grep -E '\.js\s+\d'
```
If the chunk containing TranscriptionEditView splits into multiple chunks, that's fine as long as they're loaded together (same route).

CI pipeline

npm run check (svelte-check) is listed in acceptance criteria -- good, that's already in the CI pipeline.
npm run lint should also pass. New .svelte.ts files need to follow the same ESLint + Prettier config. Confirm the lint glob pattern covers **/*.svelte.ts files (some older configs only match *.ts and *.svelte separately).

Hot module replacement (HMR)

.svelte.ts modules with reactive state ($state, SvelteMap) can behave unexpectedly during HMR in dev mode. If a developer edits useBlockAutoSave.svelte.ts, Vite may re-execute the module but the consuming component keeps its old references. This can cause "ghost timers" or duplicate beforeunload listeners during development.
Not a production issue, but worth documenting in a code comment if it's observed. The workaround is a full page reload after editing these modules.

No new dependencies

Confirm this refactor introduces no new npm packages. The issue description suggests pure extraction using existing Svelte 5 primitives (SvelteMap, $state, $derived). No additions to package.json expected.

Source map quality

After the split, verify that error stack traces in production still point to meaningful file names. useBlockAutoSave.svelte.ts:42 is more debuggable than a chunk hash. Vite's default source map config should handle this, but worth a quick check in the browser devtools after build.

## ⚙️ Tobias Wendt -- DevOps & Platform Engineer Pure frontend refactor -- no infrastructure changes needed. A few things to verify from the build and CI perspective: ### Build impact - Two new `.svelte.ts` files should be tree-shaken correctly by Vite. No bundle size increase expected, but worth checking the build output diff before and after: ```bash npm run build 2>&1 | grep -E '\.js\s+\d' ``` If the chunk containing `TranscriptionEditView` splits into multiple chunks, that's fine as long as they're loaded together (same route). ### CI pipeline - `npm run check` (svelte-check) is listed in acceptance criteria -- good, that's already in the CI pipeline. - `npm run lint` should also pass. New `.svelte.ts` files need to follow the same ESLint + Prettier config. Confirm the lint glob pattern covers `**/*.svelte.ts` files (some older configs only match `*.ts` and `*.svelte` separately). ### Hot module replacement (HMR) - `.svelte.ts` modules with reactive state (`$state`, `SvelteMap`) can behave unexpectedly during HMR in dev mode. If a developer edits `useBlockAutoSave.svelte.ts`, Vite may re-execute the module but the consuming component keeps its old references. This can cause "ghost timers" or duplicate `beforeunload` listeners during development. - Not a production issue, but worth documenting in a code comment if it's observed. The workaround is a full page reload after editing these modules. ### No new dependencies - Confirm this refactor introduces no new npm packages. The issue description suggests pure extraction using existing Svelte 5 primitives (`SvelteMap`, `$state`, `$derived`). No additions to `package.json` expected. ### Source map quality - After the split, verify that error stack traces in production still point to meaningful file names. `useBlockAutoSave.svelte.ts:42` is more debuggable than a chunk hash. Vite's default source map config should handle this, but worth a quick check in the browser devtools after build.

marcel commented

2026-04-07 11:06:35 +02:00

Response to all persona reviews

I read all six comments against the actual source code. Here are concrete answers organized by persona.

Felix Brandt (Senior Fullstack Developer)

Factory function vs standalone exports: Factory function is the right call. Both modules need initialization parameters (useBlockAutoSave needs documentId and a saveFn; useBlockDragDrop needs the sorted blocks list and a reorderFn). They should return an object of callbacks + reactive state. Not standalone exports.

handleDelete crossing concerns: Confirmed it crosses concerns. TranscriptionEditView.svelte:130-135 shows handleDelete does three things: (1) clears debounce timer, (2) clears pending text and save state, (3) calls onDeleteBlock. Items 1-2 are auto-save cleanup; item 3 is block management owned by the parent. After extraction, handleDelete in the orchestrator should call autoSave.clearBlock(blockId) (cleanup method on the auto-save module) then call onDeleteBlock(blockId). The auto-save module should NOT own the delete API call.

SvelteMap reactivity: Confirmed. Lines 39-41 already use new SvelteMap() (imported from svelte/reactivity), not plain Map. This will work correctly inside .svelte.ts modules -- Svelte 5's reactivity system tracks SvelteMap reads/writes across module boundaries.

executeSave dependency injection: Agreed. Currently executeSave (line 53) delegates to onSaveBlock which is already a prop callback. After extraction, the factory function should accept saveFn: (blockId: string, text: string) => Promise<void> as a parameter. This is already the shape of onSaveBlock in the current Props (line 17). Clean 1:1 mapping.

TranscriptionBlock scope: TranscriptionBlock.svelte already exists as a separate component (272 lines). No implicit third extraction needed. The orchestrator just passes hook-derived props to it.

Markus Keller (Senior Application Architect)

Module input contracts:

useBlockAutoSave(options: { documentId: string, saveFn: (blockId: string, text: string) => Promise<void> }) -- returns { getSaveState, handleTextChange, handleRetry, clearBlock, handleBlur, flushViaBeacon, destroy }
useBlockDragDrop(options: { getSortedBlocks: () => Block[], onReorder: (newOrder: string[]) => void }) -- returns { draggedBlockId, dropTargetIdx, dragOffsetY, handleGripDown, handlePointerMove, handlePointerUp }

Option 1 (injected saveFn) is what we'll use. The current code already uses this pattern -- onSaveBlock prop at line 17.

State ownership for block order: Confirmed the orchestrator should own block order. Currently reorder() at line 137-154 mutates blocks directly AND makes the API call. After extraction, useBlockDragDrop should only manage visual state (draggedBlockId, dropTargetIdx, dragOffsetY) and call onReorder(newOrder: string[]) on drop. The orchestrator implements onReorder to call the API and update the block array. The drag module never touches the data array.

beforeunload + drag cleanup: No drag state needs cleanup on unload. The drag state (draggedBlockId, dropTargetIdx, dragOffsetY) is purely visual -- no server-side effect, no pending data. Closing the tab mid-drag is harmless.

Line count increase: Agreed this is acceptable. The goal is cognitive load per file, not total line count.

Sara Holt (Senior QA Engineer)

Existing tests: Zero. No test files exist matching *Transcription*test* or *transcription*test* anywhere in frontend/src/. This is greenfield.

Test plan for auto-save module: All six scenarios listed are testable with vi.useFakeTimers(). The factory function pattern makes this straightforward -- inject a mock saveFn, call handleTextChange, advance timers, assert saveFn call count and getSaveState transitions. Specific timings from the code:

Debounce: 1500ms (TranscriptionEditView.svelte:87)
Saved fade delay: 2000ms (line 70) + 300ms transition (line 77)

Debounce coalescing test approach:

const saveFn = vi.fn().mockResolvedValue(undefined);
const { handleTextChange } = useBlockAutoSave({ documentId: 'x', saveFn });
handleTextChange('block-1', 'a');
vi.advanceTimersByTime(500);
handleTextChange('block-1', 'ab');
vi.advanceTimersByTime(500);
handleTextChange('block-1', 'abc');
vi.advanceTimersByTime(1500);
expect(saveFn).toHaveBeenCalledOnce();
expect(saveFn).toHaveBeenCalledWith('block-1', 'abc');

Drag-drop reorder logic: The index calculation at lines 216-220 (splice/insertAt adjustment) should be extracted into a pure function and tested separately. Pointer event simulation is unnecessary for unit tests.

E2E tests: Agree these should be created as part of this issue, not deferred. The acceptance criteria require behavioral parity -- the only reliable way to verify that is an E2E test.

Flakiness: vi.useFakeTimers() exclusively. No real timers in tests.

Nora "NullX" Steiner (Application Security Engineer)

sendBeacon auth -- THIS IS A REAL BUG (pre-existing, not introduced by this refactor):

The app uses Basic Auth via Authorization header, not cookie-based sessions (SecurityConfig.java:46 explicitly disables CSRF with a comment explaining this). sendBeacon at TranscriptionEditView.svelte:232-238 calls navigator.sendBeacon(url, blob) directly from the browser. sendBeacon cannot set custom headers -- it only sends cookies.

In dev mode: the Vite proxy (vite.config.ts:23-29) extracts auth_token from cookies and injects the Authorization header, so it works.
In production: sendBeacon('/api/...') hits the SvelteKit Node server, but sendBeacon is NOT a SvelteKit fetch -- it bypasses handleFetch entirely. There is no catch-all /api SvelteKit route for transcription block updates. The beacon request will get a 404 or hit the SvelteKit fallback.

Result: flushViaBeacon silently loses data in production. This should be filed as a separate bug. The fix options:

Replace sendBeacon with fetch(..., { keepalive: true }) routed through a SvelteKit server endpoint that injects auth
Add a SvelteKit catch-all API proxy route at src/routes/api/[...path]/+server.ts

Content-Type: The current code correctly sets type: 'application/json' on the Blob (line 236), so Spring's @RequestBody parsing would work IF the request reaches the backend.

CSRF on reorder endpoint: CSRF is globally disabled (SecurityConfig.java:46). The reorder endpoint at TranscriptionBlockController.java:75-76 is protected by @RequirePermission(Permission.WRITE_ALL). No CSRF concern.

IDOR on reorder: TranscriptionService.reorderBlocks() (line 110-117) calls getBlock(documentId, blockIds.get(i)) for each block ID, which uses findByIdAndDocumentId(blockId, documentId) (line 43). Block IDs from a different document will throw TRANSCRIPTION_BLOCK_NOT_FOUND. IDOR is properly prevented.

Timer cleanup on SvelteKit navigation: The $effect cleanup at lines 241-254 runs when the component is destroyed (SvelteKit navigation), clearing the beforeunload listener and all debounce timers. However, scheduleSavedFade at lines 69-80 creates setTimeout calls that are NOT tracked or cleaned up on destroy. These are fire-and-forget timers that could execute after navigation, calling setSaveState on a destroyed SvelteMap. This is a minor leak -- it won't crash (the map still exists in memory until GC), but it's sloppy. The extracted module should track fade timers and clear them in destroy().

No {@html} in transcription components: Confirmed. Grep for {@html} in TranscriptionEditView.svelte and TranscriptionBlock.svelte returns zero matches. No XSS surface change.

Leonie Voss (UI/UX Design Lead)

Save indicator timing: The timings are hardcoded constants: 1500ms debounce, 2000ms "saved" display, 300ms fade-out (TranscriptionEditView.svelte:70-78,87). After extraction these become constants in useBlockAutoSave.svelte.ts. No reactivity overhead is added by the module boundary -- getSaveState returns from a SvelteMap.get() which is synchronous. No extra render frame delay.

Drop indicator positioning: The drop indicator renders between blocks via {#if dropTargetIdx === i} at line 267-269, and after the last block at line 300-302. This logic stays in the orchestrator template, not in the drag module. The module only exposes reactive dropTargetIdx state. No risk of rendering on top of blocks.

Drag grip touch targets: The drag grip at TranscriptionBlock.svelte:137-143 is a div with no explicit width/height set -- it relies on the text content (the braille character). This is likely under 44x44px. However, on mobile the grip is hidden (md:block at line 138) and replaced by arrow buttons (md:hidden at lines 127, 146) which are h-7 w-7 (28x28px) -- also under the 44px minimum. This is a pre-existing a11y issue, not introduced by this refactor. Worth a follow-up issue.

Keyboard reorder: Currently supported via the mobile arrow buttons (onMoveUp/onMoveDown at TranscriptionBlock.svelte:130-131,149-150), which are <button> elements and therefore keyboard-focusable. These are always present in the DOM (just hidden on desktop via md:hidden). Keyboard users on desktop cannot access them. A follow-up issue for keyboard reorder on desktop (e.g., Shift+ArrowUp/Down on focused block) would be appropriate.

ARIA live regions: None exist. No aria-live, role="status", or role="alert" on the save state indicator (TranscriptionBlock.svelte:209-231). The extracted getSaveState reactive value makes it easy to add aria-live="polite" on the status container. Should be a follow-up issue, not scope-creep on this refactor.

Tobias Wendt (DevOps & Platform Engineer)

Bundle impact: The two new .svelte.ts files will be in the same chunk as TranscriptionEditView.svelte since they're direct imports on the same route. Vite won't split them into separate chunks. No lazy-loading boundary crossed. Net bundle size change: negligible (import boilerplate only).

Lint coverage for .svelte.ts: Confirmed covered. eslint.config.js:35 includes **/*.svelte.ts in its file glob. No config change needed.

HMR ghost timer risk: Real concern. When Vite HMR re-executes useBlockAutoSave.svelte.ts, the old module instance's timers survive but the cleanup function from the old $effect may not fire if the consuming component wasn't re-created. Mitigation: the factory function pattern helps here -- if the component re-renders and calls the factory again, the old instance's destroy() is called by the component's $effect cleanup. Adding a brief code comment noting "full page reload recommended after editing this module in dev" is reasonable.

No new dependencies: Confirmed. Only svelte/reactivity (SvelteMap) and standard Svelte 5 runes. No package.json changes.

Source maps: Vite preserves original file names in source maps by default with adapter-node. useBlockAutoSave.svelte.ts:42 will appear correctly in stack traces.

Summary of action items for implementation

Use factory function pattern for both modules with injected callbacks
handleDelete stays in orchestrator; auto-save module exposes clearBlock(blockId) for cleanup
Drag module exposes visual state only; orchestrator owns block order and API calls
Track fade timers in auto-save module and clear them in destroy() (fixes minor timer leak)
File bug for sendBeacon silent data loss in production (pre-existing, blocks this refactor's acceptance criteria since "sendBeacon flush on page unload still works" cannot pass in prod)
Unit tests for auto-save state machine with vi.useFakeTimers() -- include in this issue
Unit test for reorder index calculation as pure function
E2E test for save cycle and drag reorder -- include in this issue
Follow-up issues (not this PR): mobile touch target sizes, desktop keyboard reorder, ARIA live regions on save status

## Response to all persona reviews I read all six comments against the actual source code. Here are concrete answers organized by persona. --- ### Felix Brandt (Senior Fullstack Developer) **Factory function vs standalone exports:** Factory function is the right call. Both modules need initialization parameters (`useBlockAutoSave` needs `documentId` and a `saveFn`; `useBlockDragDrop` needs the sorted blocks list and a `reorderFn`). They should return an object of callbacks + reactive state. Not standalone exports. **`handleDelete` crossing concerns:** Confirmed it crosses concerns. `TranscriptionEditView.svelte:130-135` shows `handleDelete` does three things: (1) clears debounce timer, (2) clears pending text and save state, (3) calls `onDeleteBlock`. Items 1-2 are auto-save cleanup; item 3 is block management owned by the parent. After extraction, `handleDelete` in the orchestrator should call `autoSave.clearBlock(blockId)` (cleanup method on the auto-save module) then call `onDeleteBlock(blockId)`. The auto-save module should NOT own the delete API call. **`SvelteMap` reactivity:** Confirmed. Lines 39-41 already use `new SvelteMap()` (imported from `svelte/reactivity`), not plain `Map`. This will work correctly inside `.svelte.ts` modules -- Svelte 5's reactivity system tracks `SvelteMap` reads/writes across module boundaries. **`executeSave` dependency injection:** Agreed. Currently `executeSave` (line 53) delegates to `onSaveBlock` which is already a prop callback. After extraction, the factory function should accept `saveFn: (blockId: string, text: string) => Promise<void>` as a parameter. This is already the shape of `onSaveBlock` in the current Props (line 17). Clean 1:1 mapping. **`TranscriptionBlock` scope:** `TranscriptionBlock.svelte` already exists as a separate component (272 lines). No implicit third extraction needed. The orchestrator just passes hook-derived props to it. --- ### Markus Keller (Senior Application Architect) **Module input contracts:** - `useBlockAutoSave(options: { documentId: string, saveFn: (blockId: string, text: string) => Promise<void> })` -- returns `{ getSaveState, handleTextChange, handleRetry, clearBlock, handleBlur, flushViaBeacon, destroy }` - `useBlockDragDrop(options: { getSortedBlocks: () => Block[], onReorder: (newOrder: string[]) => void })` -- returns `{ draggedBlockId, dropTargetIdx, dragOffsetY, handleGripDown, handlePointerMove, handlePointerUp }` Option 1 (injected `saveFn`) is what we'll use. The current code already uses this pattern -- `onSaveBlock` prop at line 17. **State ownership for block order:** Confirmed the orchestrator should own block order. Currently `reorder()` at line 137-154 mutates `blocks` directly AND makes the API call. After extraction, `useBlockDragDrop` should only manage visual state (`draggedBlockId`, `dropTargetIdx`, `dragOffsetY`) and call `onReorder(newOrder: string[])` on drop. The orchestrator implements `onReorder` to call the API and update the block array. The drag module never touches the data array. **`beforeunload` + drag cleanup:** No drag state needs cleanup on unload. The drag state (`draggedBlockId`, `dropTargetIdx`, `dragOffsetY`) is purely visual -- no server-side effect, no pending data. Closing the tab mid-drag is harmless. **Line count increase:** Agreed this is acceptable. The goal is cognitive load per file, not total line count. --- ### Sara Holt (Senior QA Engineer) **Existing tests:** Zero. No test files exist matching `*Transcription*test*` or `*transcription*test*` anywhere in `frontend/src/`. This is greenfield. **Test plan for auto-save module:** All six scenarios listed are testable with `vi.useFakeTimers()`. The factory function pattern makes this straightforward -- inject a mock `saveFn`, call `handleTextChange`, advance timers, assert `saveFn` call count and `getSaveState` transitions. Specific timings from the code: - Debounce: 1500ms (`TranscriptionEditView.svelte:87`) - Saved fade delay: 2000ms (line 70) + 300ms transition (line 77) **Debounce coalescing test approach:** ```ts const saveFn = vi.fn().mockResolvedValue(undefined); const { handleTextChange } = useBlockAutoSave({ documentId: 'x', saveFn }); handleTextChange('block-1', 'a'); vi.advanceTimersByTime(500); handleTextChange('block-1', 'ab'); vi.advanceTimersByTime(500); handleTextChange('block-1', 'abc'); vi.advanceTimersByTime(1500); expect(saveFn).toHaveBeenCalledOnce(); expect(saveFn).toHaveBeenCalledWith('block-1', 'abc'); ``` **Drag-drop reorder logic:** The index calculation at lines 216-220 (`splice`/`insertAt` adjustment) should be extracted into a pure function and tested separately. Pointer event simulation is unnecessary for unit tests. **E2E tests:** Agree these should be created as part of this issue, not deferred. The acceptance criteria require behavioral parity -- the only reliable way to verify that is an E2E test. **Flakiness:** `vi.useFakeTimers()` exclusively. No real timers in tests. --- ### Nora "NullX" Steiner (Application Security Engineer) **`sendBeacon` auth -- THIS IS A REAL BUG (pre-existing, not introduced by this refactor):** The app uses Basic Auth via `Authorization` header, not cookie-based sessions (`SecurityConfig.java:46` explicitly disables CSRF with a comment explaining this). `sendBeacon` at `TranscriptionEditView.svelte:232-238` calls `navigator.sendBeacon(url, blob)` directly from the browser. `sendBeacon` cannot set custom headers -- it only sends cookies. - In **dev mode**: the Vite proxy (`vite.config.ts:23-29`) extracts `auth_token` from cookies and injects the `Authorization` header, so it works. - In **production**: `sendBeacon('/api/...')` hits the SvelteKit Node server, but `sendBeacon` is NOT a SvelteKit `fetch` -- it bypasses `handleFetch` entirely. There is no catch-all `/api` SvelteKit route for transcription block updates. The beacon request will get a 404 or hit the SvelteKit fallback. **Result: `flushViaBeacon` silently loses data in production.** This should be filed as a separate bug. The fix options: 1. Replace `sendBeacon` with `fetch(..., { keepalive: true })` routed through a SvelteKit server endpoint that injects auth 2. Add a SvelteKit catch-all API proxy route at `src/routes/api/[...path]/+server.ts` **Content-Type:** The current code correctly sets `type: 'application/json'` on the Blob (line 236), so Spring's `@RequestBody` parsing would work IF the request reaches the backend. **CSRF on reorder endpoint:** CSRF is globally disabled (`SecurityConfig.java:46`). The reorder endpoint at `TranscriptionBlockController.java:75-76` is protected by `@RequirePermission(Permission.WRITE_ALL)`. No CSRF concern. **IDOR on reorder:** `TranscriptionService.reorderBlocks()` (line 110-117) calls `getBlock(documentId, blockIds.get(i))` for each block ID, which uses `findByIdAndDocumentId(blockId, documentId)` (line 43). Block IDs from a different document will throw `TRANSCRIPTION_BLOCK_NOT_FOUND`. IDOR is properly prevented. **Timer cleanup on SvelteKit navigation:** The `$effect` cleanup at lines 241-254 runs when the component is destroyed (SvelteKit navigation), clearing the `beforeunload` listener and all debounce timers. However, `scheduleSavedFade` at lines 69-80 creates `setTimeout` calls that are NOT tracked or cleaned up on destroy. These are fire-and-forget timers that could execute after navigation, calling `setSaveState` on a destroyed `SvelteMap`. This is a minor leak -- it won't crash (the map still exists in memory until GC), but it's sloppy. The extracted module should track fade timers and clear them in `destroy()`. **No `{@html}` in transcription components:** Confirmed. Grep for `{@html}` in `TranscriptionEditView.svelte` and `TranscriptionBlock.svelte` returns zero matches. No XSS surface change. --- ### Leonie Voss (UI/UX Design Lead) **Save indicator timing:** The timings are hardcoded constants: 1500ms debounce, 2000ms "saved" display, 300ms fade-out (`TranscriptionEditView.svelte:70-78,87`). After extraction these become constants in `useBlockAutoSave.svelte.ts`. No reactivity overhead is added by the module boundary -- `getSaveState` returns from a `SvelteMap.get()` which is synchronous. No extra render frame delay. **Drop indicator positioning:** The drop indicator renders between blocks via `{#if dropTargetIdx === i}` at line 267-269, and after the last block at line 300-302. This logic stays in the orchestrator template, not in the drag module. The module only exposes reactive `dropTargetIdx` state. No risk of rendering on top of blocks. **Drag grip touch targets:** The drag grip at `TranscriptionBlock.svelte:137-143` is a `div` with no explicit width/height set -- it relies on the text content (the braille character). This is likely under 44x44px. However, on mobile the grip is `hidden` (`md:block` at line 138) and replaced by arrow buttons (`md:hidden` at lines 127, 146) which are `h-7 w-7` (28x28px) -- also under the 44px minimum. This is a pre-existing a11y issue, not introduced by this refactor. Worth a follow-up issue. **Keyboard reorder:** Currently supported via the mobile arrow buttons (`onMoveUp`/`onMoveDown` at `TranscriptionBlock.svelte:130-131,149-150`), which are `<button>` elements and therefore keyboard-focusable. These are always present in the DOM (just hidden on desktop via `md:hidden`). Keyboard users on desktop cannot access them. A follow-up issue for keyboard reorder on desktop (e.g., Shift+ArrowUp/Down on focused block) would be appropriate. **ARIA live regions:** None exist. No `aria-live`, `role="status"`, or `role="alert"` on the save state indicator (`TranscriptionBlock.svelte:209-231`). The extracted `getSaveState` reactive value makes it easy to add `aria-live="polite"` on the status container. Should be a follow-up issue, not scope-creep on this refactor. --- ### Tobias Wendt (DevOps & Platform Engineer) **Bundle impact:** The two new `.svelte.ts` files will be in the same chunk as `TranscriptionEditView.svelte` since they're direct imports on the same route. Vite won't split them into separate chunks. No lazy-loading boundary crossed. Net bundle size change: negligible (import boilerplate only). **Lint coverage for `.svelte.ts`:** Confirmed covered. `eslint.config.js:35` includes `**/*.svelte.ts` in its file glob. No config change needed. **HMR ghost timer risk:** Real concern. When Vite HMR re-executes `useBlockAutoSave.svelte.ts`, the old module instance's timers survive but the cleanup function from the old `$effect` may not fire if the consuming component wasn't re-created. Mitigation: the factory function pattern helps here -- if the component re-renders and calls the factory again, the old instance's `destroy()` is called by the component's `$effect` cleanup. Adding a brief code comment noting "full page reload recommended after editing this module in dev" is reasonable. **No new dependencies:** Confirmed. Only `svelte/reactivity` (`SvelteMap`) and standard Svelte 5 runes. No `package.json` changes. **Source maps:** Vite preserves original file names in source maps by default with `adapter-node`. `useBlockAutoSave.svelte.ts:42` will appear correctly in stack traces. --- ### Summary of action items for implementation 1. Use factory function pattern for both modules with injected callbacks 2. `handleDelete` stays in orchestrator; auto-save module exposes `clearBlock(blockId)` for cleanup 3. Drag module exposes visual state only; orchestrator owns block order and API calls 4. Track fade timers in auto-save module and clear them in `destroy()` (fixes minor timer leak) 5. **File bug for `sendBeacon` silent data loss in production** (pre-existing, blocks this refactor's acceptance criteria since "sendBeacon flush on page unload still works" cannot pass in prod) 6. Unit tests for auto-save state machine with `vi.useFakeTimers()` -- include in this issue 7. Unit test for reorder index calculation as pure function 8. E2E test for save cycle and drag reorder -- include in this issue 9. Follow-up issues (not this PR): mobile touch target sizes, desktop keyboard reorder, ARIA live regions on save status

marcel referenced this issue from a commit

2026-04-15 14:58:02 +02:00

refactor(transcription): extract useBlockAutoSave and useBlockDragDrop from TranscriptionEditView (#199)

marcel referenced this issue

2026-04-15 15:00:31 +02:00

refactor(frontend): utility dedup, component splits, dead code removal (#193–#200) #241

marcel closed this issue

2026-04-15 21:02:12 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#199

Split TranscriptionEditView.svelte (332 lines) — extract auto-save + drag-drop modules #199

Context

Proposed Split

1. useBlockAutoSave.svelte.ts (~100 lines)

2. useBlockDragDrop.svelte.ts (~60 lines)

3. TranscriptionEditView.svelte becomes orchestrator (~80 lines)

Acceptance Criteria

👨‍💻 Felix Brandt -- Senior Fullstack Developer

Naming and API surface

Reactive collections

Testing strategy

Component size target

Suggestion

🏗️ Markus Keller -- Senior Application Architect

Module boundary contracts

State ownership

beforeunload and sendBeacon placement

Accidental complexity check

No over-extraction

🧪 Sara Holt -- Senior QA Engineer

Auto-save module test plan

Drag-drop module test plan

Regression coverage

Flakiness risk

🔒 Nora "NullX" Steiner -- Application Security Engineer

sendBeacon payload integrity

Drag-and-drop reorder authorization

Timer cleanup on navigation

No new XSS surface

🎨 Leonie Voss -- UI/UX Design Lead

Auto-save feedback states

Drag-and-drop visual feedback

Accessibility preservation

Suggestion

⚙️ Tobias Wendt -- DevOps & Platform Engineer

Build impact

CI pipeline

Hot module replacement (HMR)

No new dependencies

Source map quality

Response to all persona reviews

Felix Brandt (Senior Fullstack Developer)

Markus Keller (Senior Application Architect)

Sara Holt (Senior QA Engineer)

Nora "NullX" Steiner (Application Security Engineer)

Leonie Voss (UI/UX Design Lead)

Tobias Wendt (DevOps & Platform Engineer)

Summary of action items for implementation

Split `TranscriptionEditView.svelte` (332 lines) — extract auto-save + drag-drop modules #199

1. `useBlockAutoSave.svelte.ts` (~100 lines)

2. `useBlockDragDrop.svelte.ts` (~60 lines)

3. `TranscriptionEditView.svelte` becomes orchestrator (~80 lines)

`beforeunload` and `sendBeacon` placement

`sendBeacon` payload integrity