diff --git a/e2e/pdf.mjs b/e2e/pdf.mjs index 488392f..4f846ae 100644 --- a/e2e/pdf.mjs +++ b/e2e/pdf.mjs @@ -633,71 +633,91 @@ try { - /* ── a remark that turns into a note ── */ + /* ── linking a margin note from the notes beside the document ── */ + // A remark in the margin is short by design. When there is more to say about + // that passage it belongs in the notes — and it has to be possible to say + // *which* passage. await p.locator("[data-pdf-pin-mode]").click(); await p.waitForTimeout(400); const pageL = await p.locator('[data-pdf-page="1"]').boundingBox(); await p.mouse.click(pageL.x + pageL.width * 0.35, pageL.y + pageL.height * 0.2); await p.waitForTimeout(800); await p.getByLabel("Pinned note").fill("Zebrastreifen nachschlagen"); - await p.waitForTimeout(400); + await p.getByRole("button", { name: "Fertig" }).click(); + await p.waitForTimeout(2500); - const notesBefore = (await api(p, "GET", "/notes")).body.length; - await p.locator("[data-pin-make-note]").click(); - await p.waitForTimeout(4000); + await p.locator("[data-pdf-toggle-notes]").click(); + await p.waitForTimeout(1200); + const panel = p.locator("[data-pdf-notes] .ProseMirror"); + await panel.click(); + await p.keyboard.type("Dazu ausführlicher: "); + await p.keyboard.type("@"); + await p.waitForTimeout(1200); - const notesAfter = (await api(p, "GET", "/notes")).body; - check(notesAfter.length === notesBefore + 1, "a remark can become a note of its own", `${notesBefore} → ${notesAfter.length}`); - - const made = notesAfter.find((n) => n.title === "Zebrastreifen nachschlagen"); - check(!!made, "titled from what was written in the margin"); + const suggestions = await p.locator("body").innerText(); check( - (await p.locator(".ProseMirror").count()) > 0, - "and the app goes straight there rather than leaving it to be found" + /Zebrastreifen nachschlagen/.test(suggestions), + "the margin notes are offered when linking from the notes", + suggestions.slice(0, 160).replace(/\n/g, " ") ); - /* The link has to read in both directions, or a note born from a passage has - nothing on it saying which passage. */ - const body = (await api(p, "GET", `/notes/${made.id}`)).body.content; - check(body.includes(pdfNoteId), "the new note carries a link back to the document", body.slice(0, 120)); - - const back = await api(p, "GET", `/notes/${pdfNoteId}/backlinks`); + await p.keyboard.press("Enter"); + await p.waitForTimeout(2500); + const linkEl = panel.locator("[data-note-link]").first(); + check((await linkEl.count()) === 1, "one can be inserted as a link"); check( - Array.isArray(back.body) && back.body.some((b) => b.id === made.id), - "so the document lists it among its backlinks", - JSON.stringify(back.body).slice(0, 120) + (await linkEl.getAttribute("data-kind")) === "pin", + "recorded as a link to a spot, not to a note", + await linkEl.getAttribute("data-kind") + ); + check( + (await linkEl.innerText()).includes("S. 1"), + "labelled with the page it points at", + await linkEl.innerText() ); - /* Back to the document: the pin now leads to the note. */ - await p.locator("[data-note-row]").filter({ hasText: "e2e-test" }).first().click(); - await p.waitForTimeout(4000); - const goto = p.locator(`[data-pin-goto="${made.id}"]`); - check((await goto.count()) === 1, "and the margin card links to it"); + /* The link must not reach the backlink table: its target is a marker in this + note's own overlay, and no note has that id. */ + const backAfter = await api(p, "GET", `/notes/${pdfNoteId}/backlinks`); check( - (await goto.innerText()).includes("Zebrastreifen"), - "showing the note's current title", - await goto.innerText() + Array.isArray(backAfter.body) && backAfter.body.length === 0, + "and is not reported as a link to a note", + JSON.stringify(backAfter.body).slice(0, 100) ); - await goto.click(); + /* Following it goes to the passage. */ + await panel.locator("[data-note-link]").first().click(); await p.waitForTimeout(2500); check( - (await p.getByLabel("Note title").inputValue()) === "Zebrastreifen nachschlagen", - "clicking it goes to the note", - await p.getByLabel("Note title").inputValue() + (await p.getByLabel("Pinned note").inputValue()) === "Zebrastreifen nachschlagen", + "following the link opens the margin note it points at", + await p.getByLabel("Pinned note").inputValue() ); - /* Back once more, and let go of the link — the note must survive that. */ - await p.locator("[data-note-row]").filter({ hasText: "e2e-test" }).first().click(); - await p.waitForTimeout(4000); - await p.locator(`[data-pin-goto="${made.id}"]`).locator("xpath=ancestor::*[@data-pin-card][1]").click(); - await p.waitForTimeout(700); - await p.locator("[data-pin-unlink]").click(); - await p.waitForTimeout(2500); - check((await p.locator(`[data-pin-goto="${made.id}"]`).count()) === 0, "the link can be let go of"); + // With the notes panel open there is no room for a margin column beside the + // page, so the remark opens in the sheet instead. Closing the panel gives the + // column back — and the same remark is still the open one. + await p.locator("[data-pdf-toggle-notes]").click(); + await p.waitForTimeout(1500); + const openCard = p.locator("[data-pin-card]").filter({ has: p.getByLabel("Pinned note") }); + check((await openCard.count()) === 1, "and it is the card that is open in the margin"); + await p.keyboard.press("Escape"); + await p.waitForTimeout(600); + + /* ── showing and hiding them ── */ + check((await p.locator("[data-pdf-show-pins]").count()) === 1, "the margin notes can be hidden"); + await p.locator("[data-pdf-show-pins]").click(); + await p.waitForTimeout(1200); + check((await p.locator("[data-pin]").count()) === 0, "hiding takes the markers off the page"); + check((await p.locator("[data-pdf-margin]").count()) === 0, "and the column with them"); + + const pageWide = (await p.locator('[data-pdf-page="2"]').boundingBox()).width; + await p.locator("[data-pdf-show-pins]").click(); + await p.waitForTimeout(1200); + check((await p.locator("[data-pin]").count()) > 0, "showing brings them back"); check( - (await api(p, "GET", "/notes")).body.some((n) => n.id === made.id), - "and the note itself is still there" + (await p.locator('[data-pdf-page="2"]').boundingBox()).width < pageWide, + "and the page gives the column its width again" ); /* ── on a phone there is no margin to write in ── */ diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index d7260e7..6fec2d1 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -34,7 +34,6 @@ import { EditorPane } from "./features/notes/EditorPane.jsx"; import { HistoryModal } from "./features/notes/HistoryModal.jsx"; import { DrawingLayer } from "./features/drawing/DrawingLayer.jsx"; import { CANON_W, CANON_H, PREVIEW_DRAWING_W } from "./features/drawing/strokes.js"; -import { serializeNoteContent } from "./editor/content"; /** Views that are not a notebook and therefore cannot hold a new note. */ const VIRTUAL_FOLDERS = ["all", "starred", "trash", "dashboard"]; @@ -280,70 +279,9 @@ export default function App() { [notes, openFolder, openNote] ); - /** - * Turns a remark in a PDF's margin into a note of its own. - * - * The new note opens with a link back to the document it came from, so the - * connection reads in both directions: the pin leads here, and this note shows - * up in the document's backlinks. Without that a note born from a passage - * would have nothing on it saying which passage. - */ - const createNoteForPin = useCallback( - async (source, title) => { - const folderId = source.folderId || notebooks[0]?.id; - if (!folderId) return null; - const note = await data.createNote(folderId); - const doc = { - type: "doc", - content: [ - { - type: "paragraph", - content: [ - { type: "text", text: "Zu " }, - { - type: "noteLink", - attrs: { noteId: source.id, label: source.title || "Dokument", kind: "note" }, - }, - ], - }, - { type: "paragraph" }, - ], - }; - data.updateNote(note.id, { - title, - content: serializeNoteContent(doc), - links: [source.id], - attachments: [], - }); - return { ...note, title }; - }, - [data, notebooks] - ); - - /** - * Opens a note that was created a moment ago. - * - * `openLinkTarget` looks the target up in the notes list to find its notebook, - * and a note created in the same tick is not in the copy that callback closed - * over — so it went nowhere at all. Here the notebook is already known. - */ - const openCreatedNote = useCallback( - (id, folderId) => { - if (folderId) setActiveFolder(folderId); - openNote(id); - }, - [openNote] - ); - const linkProps = useMemo( - () => ({ - openLinkTarget, - searchLinkTargets, - resolveLinkLabel, - createNoteForPin, - openCreatedNote, - }), - [openLinkTarget, searchLinkTargets, resolveLinkLabel, createNoteForPin, openCreatedNote] + () => ({ openLinkTarget, searchLinkTargets, resolveLinkLabel }), + [openLinkTarget, searchLinkTargets, resolveLinkLabel] ); /* ── Actions ── */ diff --git a/frontend/src/editor/content.test.ts b/frontend/src/editor/content.test.ts index 93ea93c..de64546 100644 --- a/frontend/src/editor/content.test.ts +++ b/frontend/src/editor/content.test.ts @@ -181,3 +181,42 @@ describe("docToText", () => { expect(docToText(doc, 50).length).toBeLessThanOrEqual(50); }); }); + +describe("links to a spot in a PDF", () => { + /* They use the same node with kind "pin", but the target is a marker inside + * this note's own overlay — not another note. Reporting one as a note link + * would fill the backlink table with ids that match nothing at all. */ + const doc = { + type: "doc", + content: [ + { + type: "paragraph", + content: [ + { type: "noteLink", attrs: { noteId: "pin-abc", label: "S. 3", kind: "pin" } }, + { type: "noteLink", attrs: { noteId: "note-1", label: "Andere Notiz", kind: "note" } }, + ], + }, + ], + }; + + it("are left out of the links reported to the server", () => { + expect(extractNoteLinks(doc)).toEqual(["note-1"]); + }); + + it("do not stop the real links in the same paragraph being reported", () => { + expect(extractNoteLinks(doc)).toContain("note-1"); + }); + + it("are left out even when they are the only links there are", () => { + const onlyPins = { + type: "doc", + content: [ + { + type: "paragraph", + content: [{ type: "noteLink", attrs: { noteId: "pin-x", label: "S. 1", kind: "pin" } }], + }, + ], + }; + expect(extractNoteLinks(onlyPins)).toEqual([]); + }); +}); diff --git a/frontend/src/editor/content.ts b/frontend/src/editor/content.ts index 1df945c..bd0528d 100644 --- a/frontend/src/editor/content.ts +++ b/frontend/src/editor/content.ts @@ -117,6 +117,10 @@ function walk(node: DocNode | undefined | null, visit: (n: DocNode) => void): vo export function extractNoteLinks(doc: DocNode, selfId?: string): string[] { const ids = new Set(); walk(doc, (n) => { + // A link to a spot in a PDF uses the same node with kind "pin", but its + // target is a marker inside this note's own overlay, not another note. + // Reporting it would fill the backlink table with ids that match nothing. + if (n.attrs?.kind === "pin") return; if (n.type === "noteLink" && n.attrs?.noteId && n.attrs.noteId !== selfId) ids.add(n.attrs.noteId); }); return [...ids]; diff --git a/frontend/src/editor/noteLink.js b/frontend/src/editor/noteLink.js index 1decfeb..c52967b 100644 --- a/frontend/src/editor/noteLink.js +++ b/frontend/src/editor/noteLink.js @@ -1,9 +1,12 @@ /** - * Note links — type `@` to reference another note or notebook. + * Note links — type `@` to reference another note, a notebook, or, inside a PDF, + * one of the notes pinned to its pages. * * A link is an atomic inline node carrying the target's id, so it survives * renames: the label is refreshed from the current title on render, and the id - * is what the backlink index is built from. + * is what the backlink index is built from. A `pin` link is deliberately left + * out of that index — its target is a marker in the note's own overlay, so no + * note has that id. */ import { Node, mergeAttributes } from "@tiptap/core"; @@ -68,7 +71,7 @@ export const NoteLink = Node.create({ href: "#", title: node.attrs.label, }), - `${node.attrs.kind === "notebook" ? "📓" : "📝"} ${node.attrs.label}`, + `${{ notebook: "📓", pin: "📌" }[node.attrs.kind] || "📝"} ${node.attrs.label}`, ]; }, diff --git a/frontend/src/features/notes/EditorPane.jsx b/frontend/src/features/notes/EditorPane.jsx index 22aadb6..9ad87bf 100644 --- a/frontend/src/features/notes/EditorPane.jsx +++ b/frontend/src/features/notes/EditorPane.jsx @@ -230,8 +230,6 @@ export function EditorPane({ onOpenNote={linkProps.openLinkTarget} searchLinkTargets={linkProps.searchLinkTargets} resolveLinkLabel={linkProps.resolveLinkLabel} - onCreateNoteForPin={linkProps.createNoteForPin} - onOpenCreatedNote={linkProps.openCreatedNote} /> ) : ( CARD_H + (note.noteId ? 20 : 0); - /** Cards at their anchors, pushed down where they would sit on top of each other. */ function layout(notes, pageHeight, cssHeight) { let previousBottom = -Infinity; @@ -33,14 +28,13 @@ function layout(notes, pageHeight, cssHeight) { .map((note) => { const anchor = (note.y / pageHeight) * cssHeight - 12; const top = Math.max(anchor, previousBottom + CARD_GAP); - previousBottom = top + heightOf(note); + previousBottom = top + CARD_H; return { note, top }; }); } -function Card({ T, note, top, number, open, onOpen, onChange, onRemove, link }) { +function Card({ T, note, top, number, open, onOpen, onChange, onRemove }) { const [draft, setDraft] = useState(note.text); - const [picking, setPicking] = useState(false); const areaRef = useRef(null); // Adopt text that arrived from elsewhere — another device, or an undo — but @@ -144,23 +138,6 @@ function Card({ T, note, top, number, open, onOpen, onChange, onRemove, link }) > Entfernen - {!note.noteId && ( - <> - - - - )} - {picking && ( - setPicking(false)} - onPick={(target) => { - setPicking(false); - commit(); - link.onLink(note.id, target.id); - }} - /> - )} ) : (
)} - - {/* The note this remark grew into. Shown in both states: when the card - is closed it is the whole point of the card, and when it is open it - is what stops a second note being made for the same passage. */} - {note.noteId && ( -
- - {open && ( - - )} -
- )}
); } -const linkAction = (T) => ({ - border: "none", - background: "transparent", - color: T.accent, - cursor: "pointer", - fontSize: 10.5, - padding: 0, - marginRight: 10, -}); - -export function PdfMargin({ - T, - notes, - pageHeight, - cssHeight, - openId, - onOpen, - onChange, - onRemove, - numberOf, - link, -}) { +export function PdfMargin({ T, notes, pageHeight, cssHeight, openId, onOpen, onChange, onRemove, numberOf }) { const placed = layout(notes, pageHeight, cssHeight); // Cards near the foot of a page may need more room than the page is tall. - const last = placed[placed.length - 1]; - const needed = last ? last.top + heightOf(last.note) : 0; + const needed = placed.length ? placed[placed.length - 1].top + CARD_H : 0; return (
onChange(note.id, text)} onRemove={() => onRemove(note.id)} - link={link} /> ))}
diff --git a/frontend/src/features/pdf/PdfNoteSheet.jsx b/frontend/src/features/pdf/PdfNoteSheet.jsx index 869df19..b7594be 100644 --- a/frontend/src/features/pdf/PdfNoteSheet.jsx +++ b/frontend/src/features/pdf/PdfNoteSheet.jsx @@ -12,7 +12,7 @@ import { useEffect, useRef, useState } from "react"; * The page stays visible above it, so the passage the note is about is still * there while writing about it. */ -export function PdfNoteSheet({ T, note, number, onChange, onClose, onRemove, link }) { +export function PdfNoteSheet({ T, note, number, onChange, onClose, onRemove }) { const [draft, setDraft] = useState(note.text); const areaRef = useRef(null); @@ -109,55 +109,6 @@ export function PdfNoteSheet({ T, note, number, onChange, onClose, onRemove, lin }} /> - {/* Same two ways out as the margin card: some remarks turn into a page of - their own, and the pin should lead there afterwards. */} -
- {note.noteId ? ( - - ) : ( - - )} -
- + {/* Reading the document as it was printed is a normal thing to want, and + so is having every remark in view. Both are one click away. */} + {totalPinnedNotes(ink) > 0 && ( + + )} + {/* A note about a passage belongs on the passage. */} - )) - )} - - - ); -} diff --git a/frontend/src/features/pdf/pageStrokes.ts b/frontend/src/features/pdf/pageStrokes.ts index 0de35dc..7f46eb2 100644 --- a/frontend/src/features/pdf/pageStrokes.ts +++ b/frontend/src/features/pdf/pageStrokes.ts @@ -27,15 +27,6 @@ export interface PinnedNote { x: number; y: number; text: string; - /** - * A note this pin points at. - * - * A margin card holds a remark; some remarks turn into a page of their own. - * Keeping the id here means the passage and the note that came out of it stay - * connected in both directions — the pin leads to the note, and the note is - * written with a link back, so it shows up in the document's backlinks. - */ - noteId?: string | null; } export interface PageStrokes { diff --git a/readme.md b/readme.md index 21d5f69..dd55cf2 100644 --- a/readme.md +++ b/readme.md @@ -304,7 +304,7 @@ failure distinguished by comparing an error's `.message` to a magic string. Failures now carry a typed `SyncError.code`; `.message` still holds the same string, so the remaining JavaScript callers are unaffected. -The 215 unit tests concentrate on the same places, and several exist because the +The 218 unit tests concentrate on the same places, and several exist because the bug they cover actually happened: - a queue entry that fails must stay queued, with an attempt counter @@ -332,7 +332,7 @@ docker compose -f docker-compose.dev.yml up -d --build # must be running | `search-attachments` | 12 | body-only matches, ranking, umlauts; images as blob URLs, not inline bytes | | `history` | 10 | a large deletion is snapshotted, restoring works, restoring is itself undoable | | `drawing` | 7 | strokes survive the poller, and match across four viewport widths | -| `pdf` | 80 | import, page proportions, lazy loading, and reading with the network off | +| `pdf` | 82 | import, page proportions, lazy loading, and reading with the network off | | `attachments-lifecycle` | 10 | a picture is kept while any version shows it, and freed once none does | | `conflict` | 10 | a late write wins but the state it buries survives as a version | | `export` | 21 | the downloaded bytes are real Markdown, ZIP and PNG, unpacked and read back | @@ -795,24 +795,22 @@ handling rather than assuming: notes was therefore stored as `NULL` — every note on it lost on the next save. Both now ask about strokes *and* pins. -**A remark can become a note of its own.** "Notiz anlegen" on a card makes one, -titles it from what was written, and goes there; the pin keeps its id, so the -passage leads to that note ever after. The note is created with a link back to -the document, so the connection reads in both directions and the document lists -it among its backlinks — a note born from a passage should not be a note with -nothing on it saying which passage. An existing note can be linked instead, and a -link can be let go of without touching the note. +**The margin notes can be linked from the notes.** A remark in the margin is +short by design; when there is more to say about that passage it belongs in the +notes beside the document — and it has to be possible to say *which* passage. So +the pins are offered to the editor's `@` menu exactly like notes and notebooks +are, labelled with their page, and following one scrolls the document to the spot +and opens the remark. They are unhidden first: a link that led to something +invisible would read as broken. -Two things had to be fixed for that to work at all, both of them older than the -feature: +A pin link is the same node with `kind: "pin"`, and is deliberately left out of +the links reported to the server. Its target is a marker inside this note's own +overlay, so no note has that id, and reporting it would fill the backlink table +with rows that match nothing. -- `openLinkTarget` finds a note's notebook by looking it up in the notes list, - and a note created in the same tick is not in the copy that callback closed - over — so jumping to it went nowhere. Creating one now navigates by the - notebook it already knows. -- A drawing save waits 700ms for the next stroke. Leaving the document inside - that window dropped it, which is exactly what turning a remark into a note - does. The viewer now flushes a pending write when it unmounts. +**They can be hidden.** Reading the document as it was printed is a normal thing +to want; the toolbar toggles the markers and the column together, and the page +takes the width back while they are away. They come along when the document is exported with annotations: each becomes a numbered marker plus a small box with its text, placed on whichever side of the