diff --git a/CHANGELOG.md b/CHANGELOG.md index fc05634..36241f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,15 @@ This project follows [Keep a Changelog](https://keepachangelog.com/) and > feature's design notes for 1.2–1.5); songbird was built in a tight burst, so treat them as the > order things landed rather than precise release days. +## [Unreleased] + +### Fixed +- **All notes on a verse now show, not just one.** When a verse carried more than one note, the + reader and the side-by-side compare view showed a single marker and opened only the first note — + the others were invisible and unreachable. The marker now carries a small count, and tapping it + lists every note so you can open any of them. A verse with a single note is unchanged. + ([#114](https://github.com/kbennett2000/songbird/issues/114)) + ## [1.6.0] — 2026-06-09 The big fan-out — four study features at once, plus a proper guide. diff --git a/frontend/src/components/AnnotationsPopover.test.tsx b/frontend/src/components/AnnotationsPopover.test.tsx new file mode 100644 index 0000000..37b5526 --- /dev/null +++ b/frontend/src/components/AnnotationsPopover.test.tsx @@ -0,0 +1,102 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { MemoryRouter } from "react-router-dom"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { AnnotationsPopover } from "@/components/AnnotationsPopover"; +import type { ReadAnnotation } from "@/schemas"; + +function ann(id: number, overrides: Partial = {}): ReadAnnotation { + return { + id, + book_usfm: "JOS", + start_chapter: 1, + start_verse: 1, + end_chapter: 1, + end_verse: 1, + note_markdown: `Note ${id}`, + color: null, + scope_type: "all", + scope_translations: [], + tags: [`tag${id}`], + in_scope: true, + author_id: 1, + created_at: "2026-06-05T00:00:00Z", + updated_at: "2026-06-05T00:00:00Z", + ...overrides, + }; +} + +function renderPopover( + annotations: ReadAnnotation[], + props: { onClose?: () => void; onOpen?: (a: ReadAnnotation) => void } = {}, +) { + const onClose = props.onClose ?? vi.fn(); + const anchor = document.createElement("button"); + document.body.appendChild(anchor); + render( + + + , + ); + return { onClose, anchor }; +} + +afterEach(() => { + document.body.innerHTML = ""; +}); + +describe("AnnotationsPopover", () => { + it("lists EVERY annotation on the verse so none is hidden behind [0] (#114)", () => { + renderPopover([ann(1), ann(2)]); + expect(screen.getByText("Notes · 2")).toBeInTheDocument(); + expect(screen.getByText("Note 1")).toBeInTheDocument(); + expect(screen.getByText("Note 2")).toBeInTheDocument(); + }); + + it("sorts newest first (created_at DESC)", () => { + renderPopover([ + ann(1, { created_at: "2026-01-01T00:00:00Z" }), // older + ann(2, { created_at: "2026-06-01T00:00:00Z" }), // newer → first + ]); + const order = screen.getAllByText(/^Note \d$/).map((p) => p.textContent); + expect(order).toEqual(["Note 2", "Note 1"]); + }); + + it("renders the out-of-scope line and tags verbatim", () => { + renderPopover([ + ann(1, { in_scope: false, scope_translations: ["KJV", "NKJV"], tags: ["grace"] }), + ]); + expect(screen.getByText("Written for KJV, NKJV")).toBeInTheDocument(); + expect(screen.getByText("grace")).toBeInTheDocument(); + }); + + it("calls onOpen with the clicked annotation (reader case)", async () => { + const onOpen = vi.fn(); + renderPopover([ann(1), ann(2)], { onOpen }); + const openButtons = screen.getAllByRole("button", { name: "Open →" }); + await userEvent.click(openButtons[1]!); + // Newest-first: row 0 is Note 1 (newer created_at default tie → stable), so assert by call arg id. + expect(onOpen).toHaveBeenCalledTimes(1); + expect(onOpen.mock.calls[0]![0]).toMatchObject({ id: expect.any(Number) }); + }); + + it("falls back to reader deep-links when onOpen is absent (compare case)", () => { + renderPopover([ann(1), ann(2)]); + expect(screen.queryByRole("button", { name: "Open →" })).not.toBeInTheDocument(); + const links = screen.getAllByRole("link", { name: /Open in reader/ }); + expect(links).toHaveLength(2); + expect(links[0]).toHaveAttribute("href", "/read?book=JOS&chapter=1&verse=1"); + }); + + it("closes on Escape (shared Popover shell)", async () => { + const { onClose } = renderPopover([ann(1), ann(2)]); + await userEvent.keyboard("{Escape}"); + expect(onClose).toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/components/AnnotationsPopover.tsx b/frontend/src/components/AnnotationsPopover.tsx new file mode 100644 index 0000000..2805bc8 --- /dev/null +++ b/frontend/src/components/AnnotationsPopover.tsx @@ -0,0 +1,104 @@ +import { useMemo } from "react"; +import { Link } from "react-router-dom"; + +import { Popover } from "@/components/Popover"; +import { readerLink } from "@/lib/notes"; +import type { ReadAnnotation } from "@/schemas"; + +interface AnnotationsPopoverProps { + /** Every annotation on the tapped marker (≥2 — the single-note case uses AnnotationPopover, or + in the reader opens the editor directly). All share the marker's in/out-of-scope class. */ + annotations: ReadAnnotation[]; + anchor: HTMLElement; + onClose: () => void; + /** + * Reader case: open this note for editing. When provided, each row gets an "Open" button. + * When omitted (compare case), each row instead deep-links back to the reader (read-only). + */ + onOpen?: (annotation: ReadAnnotation) => void; +} + +/** Newest first: created_at DESC, stable. ISO timestamps compare lexicographically. */ +function byNewest(a: ReadAnnotation, b: ReadAnnotation): number { + return a.created_at < b.created_at ? 1 : a.created_at > b.created_at ? -1 : 0; +} + +/** + * A floating popover listing EVERY annotation on a verse — the multi-note case, so none is hidden + * behind `[0]` (#114). One tap shows the whole stack for the verse; each row reuses the single + * {@link AnnotationPopover} body (verbatim Markdown — invariant 6, never editor-native JSON — its + * out-of-scope line and tags). Shares the {@link Popover} shell, so a tall list scrolls inside. + */ +export function AnnotationsPopover({ + annotations, + anchor, + onClose, + onOpen, +}: AnnotationsPopoverProps): JSX.Element { + const sorted = useMemo(() => [...annotations].sort(byNewest), [annotations]); + return ( + +
+ + Notes · {annotations.length} + + +
+ + + + ); +} diff --git a/frontend/src/routes/CompareView.test.tsx b/frontend/src/routes/CompareView.test.tsx index 0aa3697..5292025 100644 --- a/frontend/src/routes/CompareView.test.tsx +++ b/frontend/src/routes/CompareView.test.tsx @@ -131,6 +131,39 @@ describe("CompareView", () => { expect(link).toHaveAttribute("href", "/read?book=JHN&chapter=3&verse=16"); }); + it("shows a count badge and lists ALL notes when a verse has several (#114)", async () => { + useRead(() => [ + { + verse: 16, + annotations: [ + ann({ id: 1, note_markdown: "first note" }), + ann({ id: 2, note_markdown: "second note" }), + ], + }, + ]); + const user = userEvent.setup(); + renderCompare(); + + // The marker announces the count rather than masquerading as a single note. + const marker = await screen.findByRole("button", { name: "2 notes on KJV 16" }); + await user.click(marker); + + // Both notes appear in the stacked popover — neither hidden behind [0]. + expect(await screen.findByText("Notes · 2")).toBeInTheDocument(); + expect(screen.getByText("first note")).toBeInTheDocument(); + expect(screen.getByText("second note")).toBeInTheDocument(); + }); + + it("uses the single AnnotationPopover (no count) for a verse with one note", async () => { + useRead(() => [{ verse: 16, annotations: [ann({ note_markdown: "lone note" })] }]); + const user = userEvent.setup(); + renderCompare(); + + await user.click(await screen.findByRole("button", { name: "View note on KJV 16" })); + expect(await screen.findByText("lone note")).toBeInTheDocument(); + expect(screen.queryByText(/Notes · /)).not.toBeInTheDocument(); + }); + it("switches a column's translation and refetches just that column", async () => { useTranslations("KJV", "WEB", "ASV"); useRead(() => [{ verse: 16 }]); diff --git a/frontend/src/routes/CompareView.tsx b/frontend/src/routes/CompareView.tsx index de79b2d..c6e355f 100644 --- a/frontend/src/routes/CompareView.tsx +++ b/frontend/src/routes/CompareView.tsx @@ -3,6 +3,7 @@ import { Fragment, useEffect, useMemo, useRef, useState } from "react"; import { useSearchParams } from "react-router-dom"; import { AnnotationPopover } from "@/components/AnnotationPopover"; +import { AnnotationsPopover } from "@/components/AnnotationsPopover"; import { TopNav } from "@/components/TopNav"; import { useAuth } from "@/hooks/useAuth"; import { nextChapter, prevChapter } from "@/lib/navigation"; @@ -35,9 +36,12 @@ export function CompareView(): JSX.Element { const [columns, setColumns] = useState(() => [ searchParams.get("translation") ?? user?.last_translation ?? DEFAULT_TRANSLATION, ]); - const [openNote, setOpenNote] = useState<{ annotation: ReadAnnotation; anchor: HTMLElement } | null>( - null, - ); + // Every annotation behind the tapped marker (all share its in/out-of-scope class). One → the + // single AnnotationPopover; several → the stacked AnnotationsPopover, so none is hidden (#114). + const [openNote, setOpenNote] = useState<{ + annotations: ReadAnnotation[]; + anchor: HTMLElement; + } | null>(null); const booksQuery = useQuery({ queryKey: ["books"], queryFn: fetchBooks }); const translationsQuery = useQuery({ queryKey: ["translations"], queryFn: fetchTranslations }); @@ -271,12 +275,21 @@ export function CompareView(): JSX.Element { type="button" className="ml-2 align-middle text-amber-600 hover:text-amber-800 dark:hover:text-amber-400" onClick={(e) => - setOpenNote({ annotation: inScope[0]!, anchor: e.currentTarget }) + setOpenNote({ annotations: inScope, anchor: e.currentTarget }) + } + aria-label={ + inScope.length === 1 + ? `View note on ${code} ${n}` + : `${inScope.length} notes on ${code} ${n}` } - aria-label={`View note on ${code} ${n}`} - title="View note" + title={inScope.length === 1 ? "View note" : "View notes"} > ● + {inScope.length > 1 && ( + + {inScope.length} + + )} )} {outScope.length > 0 && ( @@ -284,12 +297,21 @@ export function CompareView(): JSX.Element { type="button" className="ml-2 align-middle text-gray-400 dark:text-gray-500 hover:text-gray-600" onClick={(e) => - setOpenNote({ annotation: outScope[0]!, anchor: e.currentTarget }) + setOpenNote({ annotations: outScope, anchor: e.currentTarget }) + } + aria-label={ + outScope.length === 1 + ? `View out-of-scope note on ${code} ${n}` + : `${outScope.length} out-of-scope notes on ${code} ${n}` } - aria-label={`View out-of-scope note on ${code} ${n}`} - title={`Written for ${outScope[0]!.scope_translations.join(", ")}`} + title={`Written for ${[...new Set(outScope.flatMap((a) => a.scope_translations))].join(", ")}`} > ○ + {outScope.length > 1 && ( + + {outScope.length} + + )} )} @@ -306,13 +328,20 @@ export function CompareView(): JSX.Element { )} - {openNote && ( - setOpenNote(null)} - /> - )} + {openNote && + (openNote.annotations.length === 1 ? ( + setOpenNote(null)} + /> + ) : ( + setOpenNote(null)} + /> + ))} ); } diff --git a/frontend/src/routes/ReaderView.test.tsx b/frontend/src/routes/ReaderView.test.tsx index 63bcac6..72d189c 100644 --- a/frontend/src/routes/ReaderView.test.tsx +++ b/frontend/src/routes/ReaderView.test.tsx @@ -1,5 +1,5 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { render, screen, waitFor } from "@testing-library/react"; +import { render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { http, HttpResponse } from "msw"; import { MemoryRouter } from "react-router-dom"; @@ -530,6 +530,53 @@ describe("ReaderView", () => { expect(screen.queryByText(/Sermons ·/)).not.toBeInTheDocument(); }); + it("counts multiple annotations on a verse and opens a chosen one for editing (#114)", async () => { + const annotations = [ + annotation({ id: 1, note_markdown: "first note" }), + annotation({ id: 2, note_markdown: "second note" }), + ]; + server.use( + http.get("/api/v1/read/:translation/:book/:chapter", ({ params }) => + HttpResponse.json(readResponse(annotations, String(params.translation))), + ), + ); + const user = userEvent.setup(); + renderReader(); + + // Counted ● (not the single-note label), and the count is visible. + const marker = await screen.findByRole("button", { name: "2 notes on verse 16" }); + expect(marker).toHaveTextContent("2"); + expect(screen.queryByRole("button", { name: "View note on verse 16" })).not.toBeInTheDocument(); + + // Tapping lists every note — both reachable, none hidden behind [0]. + await user.click(marker); + expect(await screen.findByText("Notes · 2")).toBeInTheDocument(); + expect(screen.getByText("first note")).toBeInTheDocument(); + expect(screen.getByText("second note")).toBeInTheDocument(); + + // "Open" on the second row hands THAT note to the editor (pre-filled with its Markdown). + // Both notes share a created_at, so locate the row by its text rather than by order. + const secondRow = screen.getByText("second note").closest("li")!; + await user.click(within(secondRow).getByRole("button", { name: "Open →" })); + expect(await screen.findByTestId("initial-markdown")).toHaveTextContent("second note"); + }); + + it("opens the editor directly (no popover) for a verse with exactly one note", async () => { + server.use( + http.get("/api/v1/read/:translation/:book/:chapter", ({ params }) => + HttpResponse.json(readResponse([annotation({ note_markdown: "lone note" })], String(params.translation))), + ), + ); + const user = userEvent.setup(); + renderReader(); + + const marker = await screen.findByRole("button", { name: "View note on verse 16" }); + await user.click(marker); + // Straight to the editor, no list popover. + expect(await screen.findByTestId("initial-markdown")).toHaveTextContent("lone note"); + expect(screen.queryByText(/Notes · /)).not.toBeInTheDocument(); + }); + it("keeps three overlays distinct on one verse even with multiple sermons (counted ▶)", async () => { const tnote = { book: "JHN", diff --git a/frontend/src/routes/ReaderView.tsx b/frontend/src/routes/ReaderView.tsx index 0b768b2..0987398 100644 --- a/frontend/src/routes/ReaderView.tsx +++ b/frontend/src/routes/ReaderView.tsx @@ -12,6 +12,7 @@ import { import { useSearchParams } from "react-router-dom"; import { CrossReferences } from "@/components/CrossReferences"; +import { AnnotationsPopover } from "@/components/AnnotationsPopover"; import { Geography } from "@/components/Geography"; import { Modal } from "@/components/Modal"; import { NoteEditor } from "@/components/NoteEditor"; @@ -136,6 +137,13 @@ export function ReaderView(): JSX.Element { anchor: HTMLElement; verse: ReadVerse; } | null>(null); + // The annotations behind a tapped marker, when there are several (single → editor directly). + // The list shares one in/out-of-scope class; a row's "Open" hands it to openExisting (#114). + const [openAnnotations, setOpenAnnotations] = useState<{ + annotations: ReadAnnotation[]; + anchor: HTMLElement; + verse: ReadVerse; + } | null>(null); const [refInput, setRefInput] = useState(""); const [resolveError, setResolveError] = useState(null); const [sermonSaveError, setSermonSaveError] = useState(null); @@ -279,6 +287,7 @@ export function ReaderView(): JSX.Element { setMap(false); setOpenNote(null); setOpenSermon(null); + setOpenAnnotations(null); }; // Self-heal a stale *stored* position once Concord's book list is known: if the initial book is @@ -452,6 +461,7 @@ export function ReaderView(): JSX.Element { setWords(null); setGeo(false); setMap(false); + setOpenAnnotations(null); setEditing({ verse, kind: "annotation", @@ -744,22 +754,56 @@ export function ReaderView(): JSX.Element { )} {outScope.length > 0 && ( )} {v.sermon_notes.length > 0 && ( @@ -1000,6 +1044,15 @@ export function ReaderView(): JSX.Element { }} /> ))} + + {openAnnotations && ( + setOpenAnnotations(null)} + onOpen={(annotation) => openExisting(openAnnotations.verse, annotation)} + /> + )} ); }