Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
102 changes: 102 additions & 0 deletions frontend/src/components/AnnotationsPopover.test.tsx
Original file line number Diff line number Diff line change
@@ -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> = {}): 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(
<MemoryRouter>
<AnnotationsPopover
annotations={annotations}
anchor={anchor}
onClose={onClose}
onOpen={props.onOpen}
/>
</MemoryRouter>,
);
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();
});
});
104 changes: 104 additions & 0 deletions frontend/src/components/AnnotationsPopover.tsx
Original file line number Diff line number Diff line change
@@ -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 (
<Popover anchor={anchor} onClose={onClose} ariaLabel={`${annotations.length} notes on this verse`}>
<div className="mb-1 flex items-start justify-between gap-2">
<span className="text-xs font-semibold uppercase tracking-wide text-amber-700">
Notes · {annotations.length}
</span>
<button
type="button"
className="rounded p-1 text-gray-400 dark:text-gray-500 hover:bg-gray-100 dark:hover:bg-gray-700 hover:text-gray-700 dark:hover:text-gray-200"
onClick={onClose}
aria-label="Close"
>
</button>
</div>

<ul className="flex flex-col">
{sorted.map((annotation) => (
<li
key={annotation.id}
className="border-t border-gray-100 py-2 first:border-0 first:pt-0"
>
{!annotation.in_scope && annotation.scope_translations.length > 0 && (
<p className="mb-1 text-xs italic text-gray-500 dark:text-gray-400">
Written for {annotation.scope_translations.join(", ")}
</p>
)}
<p className="whitespace-pre-wrap break-words text-gray-800 dark:text-gray-100">
{annotation.note_markdown}
</p>
{annotation.tags.length > 0 && (
<ul className="mt-2 flex flex-wrap gap-1">
{annotation.tags.map((tag) => (
<li
key={tag}
className="rounded-full bg-gray-100 dark:bg-gray-700 px-2 py-0.5 text-xs font-medium text-gray-600 dark:text-gray-300"
>
{tag}
</li>
))}
</ul>
)}
<div className="mt-2">
{onOpen ? (
<button
type="button"
className="text-sm font-medium text-blue-700 dark:text-blue-400 hover:underline"
onClick={() => onOpen(annotation)}
>
Open →
</button>
) : (
<Link
to={readerLink(annotation)}
className="text-sm font-medium text-blue-700 dark:text-blue-400 hover:underline"
>
Open in reader →
</Link>
)}
</div>
</li>
))}
</ul>
</Popover>
);
}
33 changes: 33 additions & 0 deletions frontend/src/routes/CompareView.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 }]);
Expand Down
61 changes: 45 additions & 16 deletions frontend/src/routes/CompareView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -35,9 +36,12 @@ export function CompareView(): JSX.Element {
const [columns, setColumns] = useState<string[]>(() => [
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 });
Expand Down Expand Up @@ -271,25 +275,43 @@ 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 && (
<span className="ml-0.5 rounded-full bg-amber-100 px-1 text-[0.7em] font-semibold text-amber-700">
{inScope.length}
</span>
)}
</button>
)}
{outScope.length > 0 && (
<button
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 && (
<span className="ml-0.5 rounded-full bg-gray-100 px-1 text-[0.7em] font-semibold text-gray-600">
{outScope.length}
</span>
)}
</button>
)}
</>
Expand All @@ -306,13 +328,20 @@ export function CompareView(): JSX.Element {
)}
</main>

{openNote && (
<AnnotationPopover
annotation={openNote.annotation}
anchor={openNote.anchor}
onClose={() => setOpenNote(null)}
/>
)}
{openNote &&
(openNote.annotations.length === 1 ? (
<AnnotationPopover
annotation={openNote.annotations[0]!}
anchor={openNote.anchor}
onClose={() => setOpenNote(null)}
/>
) : (
<AnnotationsPopover
annotations={openNote.annotations}
anchor={openNote.anchor}
onClose={() => setOpenNote(null)}
/>
))}
</div>
);
}
Loading
Loading