diff --git a/packages/annotation/src/App.mermaid.test.tsx b/packages/annotation/src/App.mermaid.test.tsx index f9bff5f..cf3f2a7 100644 --- a/packages/annotation/src/App.mermaid.test.tsx +++ b/packages/annotation/src/App.mermaid.test.tsx @@ -6,10 +6,12 @@ import { afterEach, describe, expect, it } from 'vitest'; import { userEvent as browserUserEvent } from 'vitest/browser'; import { annotatedMarkdownTestIds } from './AnnotatedMarkdown.tsx'; import { annotationDraftCommentComposerTestIds } from './AnnotationDraftCommentComposer.tsx'; +import { annotationThreadCardTestIds } from './AnnotationThreadCard.tsx'; import { appTestIds } from './App.tsx'; +import { commentsSidebarTestIds } from './CommentsSidebar.tsx'; import { elementBlockAttrs } from './element/elementBlock.ts'; import { mermaidAttrs } from './element/mermaid/MermaidBlock.tsx'; -import { annotateAndSubmit, renderApp, saveAnnotation } from './testHelpers/index.tsx'; +import { annotateAndSubmit, drag, renderApp, saveAnnotation } from './testHelpers/index.tsx'; // End-to-end through the real `mermaid` dependency: a plan with a fenced mermaid block flows through // AnnotatedMarkdown → MermaidBlock (renders the SVG and tags its nodes/edges) @@ -165,6 +167,32 @@ describe('App — Mermaid diagram annotation', () => { expect(annotated.hovered).toBe(annotated.resting); }); + it('orders the sidebar by document position when a diagram comment precedes a text comment', async () => { + const user = userEvent.setup(); + renderApp({ initialPayload: { contentKind: 'plan', content: MERMAID_PLAN.content } }); + + // Comment on the prose below the diagram, then on a node in the diagram above it. The sidebar + // must list the diagram comment first — it comes first in the document. + const paragraph = await screen.findByText('Annotate a node or edge above.'); + drag({ target: paragraph.firstChild as Text, from: 0, to: 'Annotate'.length }); + await user.type( + await screen.findByTestId(annotationDraftCommentComposerTestIds.textarea), + 'Text comment below the diagram', + ); + await user.click(screen.getByTestId(annotationDraftCommentComposerTestIds.saveButton)); + await waitFor(() => { + expect(screen.queryByTestId(annotationDraftCommentComposerTestIds.container)).not.toBeInTheDocument(); + }); + + await saveAnnotation({ + user, + target: await waitForDiagramElement(`[${mermaidAttrs.nodeId}="Login"]`), + body: 'Diagram comment above the text', + }); + + expect(getCommentCardBodies()).toEqual(['Diagram comment above the text', 'Text comment below the diagram']); + }); + it('prompts to discard a dirty diagram draft when clicking a different element', async () => { const user = userEvent.setup(); renderApp({ initialPayload: { contentKind: 'plan', content: MERMAID_PLAN.content } }); @@ -235,6 +263,18 @@ async function fillAroundHover(node: Element): Promise<{ resting: string; hovere return { resting, hovered: getComputedStyle(shape).fill }; } +/** Saved comment bodies in sidebar render order. */ +function getCommentCardBodies(): string[] { + const cardTestIdPrefix = annotationThreadCardTestIds.card(''); + const commentTestIdPrefix = annotationThreadCardTestIds.comment(''); + const cards = screen + .getByTestId(commentsSidebarTestIds.threadList) + .querySelectorAll(`[data-testid^="${cardTestIdPrefix}"]`); + return Array.from(cards).map( + (card) => card.querySelector(`[data-testid^="${commentTestIdPrefix}"]`)?.textContent ?? '', + ); +} + async function waitForDiagramElement(selector: string): Promise { return await waitFor( () => { diff --git a/packages/annotation/src/annotationResolvers.ts b/packages/annotation/src/annotationResolvers.ts index 860f149..f85658f 100644 --- a/packages/annotation/src/annotationResolvers.ts +++ b/packages/annotation/src/annotationResolvers.ts @@ -124,30 +124,12 @@ function withActiveDraftThread( ]; } +// Source lines are the one position every anchor kind shares — element anchors never resolve to a DOM Range. function compareResolvedThreads(left: ResolvedAnnotationThread, right: ResolvedAnnotationThread): number { - if (left.range && right.range) { - const order = left.range.compareBoundaryPoints(Range.START_TO_START, right.range); - if (order !== 0) { - return order; - } - } - - if (left.range && !right.range) { - return -1; - } - - if (!left.range && right.range) { - return 1; - } - - // Range-less threads (element anchors) order by source line, which tracks document order, - // before falling back to a per-anchor secondary key. - const lineOrder = left.anchor.sourceLines.start - right.anchor.sourceLines.start; - if (lineOrder !== 0) { - return lineOrder; - } - - return secondaryOrderKey(left.anchor) - secondaryOrderKey(right.anchor); + return ( + left.anchor.sourceLines.start - right.anchor.sourceLines.start || + secondaryOrderKey(left.anchor) - secondaryOrderKey(right.anchor) + ); } /** diff --git a/packages/annotation/src/useCommentNavigation.test.tsx b/packages/annotation/src/useCommentNavigation.test.tsx index 8f40b19..6101a3a 100644 --- a/packages/annotation/src/useCommentNavigation.test.tsx +++ b/packages/annotation/src/useCommentNavigation.test.tsx @@ -1,3 +1,4 @@ +import { elementAnnotationAnchor } from '@contextbridge/shared/testFactories'; import { cleanup, fireEvent, render, screen } from '@testing-library/react'; import { useState } from 'react'; import { afterEach, describe, expect, it, vi } from 'vitest'; @@ -10,17 +11,24 @@ describe('useCommentNavigation', () => { cleanup(); }); - it('excludes unresolved or range-less threads from navigation', () => { + it('excludes unresolved threads but keeps resolved range-less element threads navigable', () => { renderCommentNavigationHarness({ threads: [ resolvedAnnotationThread.build({ id: 'thr_live' }), resolvedAnnotationThread.build({ id: 'thr_unresolved', unresolved: true }), - resolvedAnnotationThread.build({ id: 'thr_range_less', range: null }), + // Anchor replaced wholesale: Fishery deep-merges params, which would blend the factory's + // default text anchor into the element anchor. + { + ...resolvedAnnotationThread.build({ id: 'thr_element' }), + anchor: elementAnnotationAnchor.build(), + range: null, + }, ], + selectedAnnotationId: 'thr_element', }); - expect(screen.getByTestId(commentNavigationTestIds.total)).toHaveTextContent('1'); - expect(screen.getByTestId(commentNavigationTestIds.currentAnnotationId)).toHaveTextContent('thr_live'); + expect(screen.getByTestId(commentNavigationTestIds.total)).toHaveTextContent('2'); + expect(screen.getByTestId(commentNavigationTestIds.currentAnnotationId)).toHaveTextContent('thr_element'); }); it('uses the selected thread as the current thread', () => { diff --git a/packages/annotation/src/useCommentNavigation.ts b/packages/annotation/src/useCommentNavigation.ts index 9013911..cb27bef 100644 --- a/packages/annotation/src/useCommentNavigation.ts +++ b/packages/annotation/src/useCommentNavigation.ts @@ -101,7 +101,7 @@ export function useCommentNavigation({ } function getNavigableThreads(threads: ResolvedAnnotationThread[]): ResolvedAnnotationThread[] { - return threads.filter((thread) => !thread.unresolved && thread.range !== null && !isNewThreadDraftOnly(thread)); + return threads.filter((thread) => !thread.unresolved && !isNewThreadDraftOnly(thread)); } function isNewThreadDraftOnly(thread: ResolvedAnnotationThread): boolean {