bug: notification deep-link does not scroll to comment on document detail page #299

Merged
marcel merged 8 commits from feat/issue-276-notification-deep-link-scroll into main 2026-04-21 15:06:02 +02:00

8 Commits

Author SHA1 Message Date
Marcel
3bf0b38c42 polish(document-detail): address review concerns on onMount deep-link wiring
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 2m40s
CI / OCR Service Tests (pull_request) Successful in 30s
CI / Backend Unit Tests (pull_request) Failing after 2m44s
CI / Unit & Component Tests (push) Failing after 2m41s
CI / OCR Service Tests (push) Successful in 37s
CI / Backend Unit Tests (push) Failing after 2m55s
Three small refinements from Felix's review cycle 1:

- replaceState(page.url.pathname, page.state ?? {}) — defend against
  first-navigation cases where page.state can be undefined.
- Extract the inline tick + requestAnimationFrame into a named
  waitForPanelRender() helper; intent is now readable from onMount.
- Attach .catch() to the fire-and-forget scrollToCommentFromQuery
  promise so any helper throw surfaces via console.error instead
  of silently disappearing.

No behavior change on the happy path. All existing tests stay green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 14:10:46 +02:00
Marcel
567faee3cc test(e2e): notification deep-link scrolls to target comment
Some checks failed
CI / Unit & Component Tests (push) Failing after 2m43s
CI / OCR Service Tests (push) Successful in 36s
CI / Backend Unit Tests (push) Failing after 3m0s
CI / Unit & Component Tests (pull_request) Failing after 2m46s
CI / OCR Service Tests (pull_request) Successful in 31s
CI / Backend Unit Tests (pull_request) Failing after 2m45s
Seeds a document, transcription block, and block comment via API,
then visits /documents/{id}?commentId=X&annotationId=Y and asserts
the page enters transcribe mode, the comment article becomes visible,
and the URL query params are stripped. Runs at 320px and 1440px so
the collapsed PDF strip clipping on mobile is caught. An axe-core
pass guards the new tabindex + focus-visible ring against a11y
regressions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 13:44:45 +02:00
Marcel
e22265f5bc feat(document-detail): wire notification deep-link scroll in onMount
After navHeight setup, call scrollToCommentFromQuery with the page
URL and callbacks into the component's local state (transcribeMode,
activeAnnotationId, flashAnnotationId) plus SvelteKit's replaceState
to strip the consumed query params.

afterTick awaits both Svelte's tick() and one requestAnimationFrame,
mirroring the existing handleAnnotationClick timing so the annotation
panel has rendered before scrollIntoView fires.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 13:42:55 +02:00
Marcel
20ae85f879 feat(comment): expose comment id + focus ring on CommentMessage wrapper
Notification deep-link scroll targets #comment-{id}. Add the id to
the article wrapper along with tabindex="-1" so scrollIntoView +
.focus({preventScroll:true}) can land screen-reader and keyboard
focus on the specific comment. A focus-visible ring appears only
for keyboard users so mouse clicks don't trigger a visible outline.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 13:32:04 +02:00
Marcel
251eb9c3fc feat(frontend): add scrollToCommentFromQuery helper for notification deep-link
Pure function that reads commentId + annotationId from the page URL,
enters transcribe mode if needed, activates the block's annotation,
scrolls the target comment into view, focuses it for screen readers,
fires the existing annotation flash, and strips the params via the
injected callback.

All side effects go through callbacks so the helper is unit-testable
without mounting the page or a DOM (only scrollIntoView/focus are
called on the injected element). Eight tests cover both absent params,
happy path, transcribe-mode activation, missing DOM target, reduced
motion, flash trigger, and URL strip.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 13:29:52 +02:00
Marcel
bc69e8ff1e refactor(comment): drop dead document and annotation comment APIs
Only block comments are surfaced by the frontend now. The document-level
and annotation-level comment endpoints and service methods existed but
had no consumer. Remove them along with their repository queries and
test coverage so the surface area matches the actual feature set.

Shared edit, delete, and block reply endpoints stay. postBlockComment
now carries the authorName/mention/audit behaviors previously tested
through the dropped postComment method, so those behaviors remain
covered by the block-scoped test suite.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 13:27:10 +02:00
Marcel
13732ab96b fix(db): V51 backfills annotation_id on block comments and notifications
Previously issued block-comment notifications were stored with
annotation_id=NULL because CommentService.postBlockComment did not
populate DocumentComment.annotationId. Now that the code fix is in
place, existing rows need to be filled in so legacy notifications
can also carry the query param that the frontend deep-link flow
expects.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 13:13:13 +02:00
Marcel
4658852281 fix(comment): populate annotationId on block comments from the block
postBlockComment now looks up the block via TranscriptionService and
sets comment.annotationId from block.getAnnotationId(). This closes
the upstream root cause of issue #276, where notifications for block
comments were stored with annotationId=null, breaking the notification
deep-link flow on the document detail page.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-21 13:09:30 +02:00