diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cd9af4..899214d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,58 @@ not from the output of `docker push`. --- +## 2026-08-17 — One direction to drop in, and a new note that stays on top + +**Images** — web `sha256:PENDING`, server `sha256:PENDING` + +No backend source changed this round. + +**Fixed** +- **Every new note pulled the arrangement apart.** A note was made with no + position at all and left to the tie-break, which put it near the top by age — + which is not the same thing. A separator's position counts the rows above it, + so a row appearing without a renumber left every line one row too high, and a + little more of the notebook's order came undone with each note. A new note is + now put at the very top and the list is renumbered to say so; every other row + keeps its place, lines included. Rows that had never been dragged stop sliding + about as they are edited, because they now carry a position of their own. +- **A chapter could only be dropped *into*, never dragged past.** Taking the + whole row as a place quietly took reordering away: a chapter could not be moved + among its neighbours, and nothing — note or line — could be put in the gap + under one. The **bottom edge of a chapter's header is that gap** now; the rest + of the row is still the chapter, and still lights up to say so. +- **A link showed the sheet of paper whatever it pointed at**, despite last + round's work. Two things: the `@` menu drew each note's own mark but dropped it + on the way into the link, and the refresh that keeps a link's label and mark + current ran once, while the editor was still empty — the note's content is + poured in afterwards, deliberately without an update event, so nothing ever + looked again. It now runs whenever the document changes. +- **The mark in the title bar sat above the line of the title.** Both boxes were + centred, which is not the same as being on one line: a title's box is taller + than its letters by the depth of a descender, so its capitals sit above the + middle while an emoji's picture fills its box. Measured, the mark was 2px high + and dipped below a line the title never crossed. They share a **baseline** now, + which needs no number. +- **The task list's box hung off the line it belonged to.** Centred in the full + line it was a 15px square against a 10px letter, its top level with the + capitals and its bottom 4px past the baseline. It is centred on the *letters* + and sized in em, so it stays the size of a capital at every type size. + +**Changed** +- **A drop lands the row under the one it was released on, and only there.** + Dropping above a row and dropping below the row before it are the same move + described twice, so every gap could be aimed at from two sides — hard to aim + at, harder to read. There is now a strip **above the first row** as well as + below the last, because that is the one place a drop *under* something cannot + reach. +- ***Move to…* is divided under headings** — the notebook itself, *Chapters + here*, *Other notebooks* — instead of running two kinds of place together with + a rule between them. The headings are deliberately not shaped like entries: one + built like a menu row is a button that does nothing when pressed. Each row + carries the mark its chapter was given or the cover its notebook wears. + +--- + ## 2026-08-17 — One dialog per thing, and rows dragged into a chapter **Images** — web `sha256:18a68a2d…`, server `sha256:febc4b93…` diff --git a/e2e/chapters.mjs b/e2e/chapters.mjs index d1e6580..1659c02 100644 --- a/e2e/chapters.mjs +++ b/e2e/chapters.mjs @@ -279,15 +279,22 @@ try { /* Held and dropped in two steps, with a wait between: the highlight is React state, and asking for it in the same breath as the move that caused it reads the DOM before it has been drawn. */ - const holdOver = (dragId, chapterId) => + /* `where` picks the part of the row held over: a chapter is a place in the + body of its header and a neighbour along the bottom edge of it, which is + how a row can still be put in the gap under a chapter. */ + const holdOver = (dragId, chapterId, where = "body") => p.evaluate( - ([from, to]) => { + ([from, to, part]) => { const row = document.querySelector(`[data-reorder-id="${from}"]`); const grip = row?.querySelector("[data-reorder-grip]"); const target = document.querySelector(`[data-reorder-id="${to}"]`); if (!grip || !target) return `missing ${!!grip} ${!!target}`; - const box = target.getBoundingClientRect(); - window.__drop = { x: box.x + box.width / 2, y: box.y + box.height / 2 }; + const head = target.querySelector("[data-reorder-head]") || target; + const box = head.getBoundingClientRect(); + window.__drop = { + x: box.x + box.width / 2, + y: part === "edge" ? box.bottom - 2 : box.y + box.height / 2, + }; const fire = (el, type, x, y) => el.dispatchEvent( new PointerEvent(type, { @@ -305,7 +312,7 @@ try { fire(window, "pointermove", window.__drop.x, window.__drop.y); return "held"; }, - [dragId, chapterId] + [dragId, chapterId, where] ); const letGo = () => @@ -357,8 +364,15 @@ try { const shownChapters = await p .locator("[data-chapter-row]") .evaluateAll((els) => els.map((e) => e.getAttribute("data-chapter-row"))); + // The two strips at the ends of the list are drop targets, not rows: they + // carry a reorder id and nothing else, least of all a grip to drag by. const noteId = rowOrder.find( - (id) => id && !id.startsWith("sep-") && id !== "__end__" && !shownChapters.includes(id) + (id) => + id && + !id.startsWith("sep-") && + id !== "__end__" && + id !== "__start__" && + !shownChapters.includes(id) ); check( !!noteId && !!shownChapters[0], @@ -395,6 +409,37 @@ try { check(false, "a separator was there to drag", "none found"); } + /* ── a chapter is a neighbour too ── + * Taking the whole row as "into" quietly took reordering away: a chapter could + * not be moved among its neighbours, and nothing could be put in the gap under + * one. The bottom edge of the header is that gap, and asks for a line rather + * than lighting the chapter up. */ + const looseLeft = ( + await p.locator("[data-reorder-id]").evaluateAll((els) => els.map((e) => e.dataset.reorderId)) + ).find( + (id) => + id && + !id.startsWith("sep-") && + id !== "__end__" && + id !== "__start__" && + !shownChapters.includes(id) + ); + if (looseLeft) { + const held = await holdOver(looseLeft, shownChapters[0], "edge"); + await p.waitForTimeout(400); + const litAtEdge = await p.locator("[data-chapter-drop]").count(); + const lineAtEdge = await p.locator("[data-drop-line]").count(); + await letGo(); + await p.waitForTimeout(2000); + check( + held === "held" && litAtEdge === 0 && lineAtEdge === 1, + "held over the bottom edge of a chapter, it offers a line instead of taking the row in", + JSON.stringify({ held, litAtEdge, lineAtEdge }) + ); + } else { + check(false, "a loose note was left to try the edge with", "none"); + } + /* ── a dialog does not close because a selection ended outside it ── * Select a word in the field, drag a little past the edge, let go: the click * lands on the backdrop — the nearest ancestor of press and release — and the diff --git a/e2e/organise.mjs b/e2e/organise.mjs index 21b8a07..5af2199 100644 --- a/e2e/organise.mjs +++ b/e2e/organise.mjs @@ -127,13 +127,23 @@ try { }; const rows = p.locator("[data-note-row]"); + /* A drop lands *under* the row it was released on, and only there — both + directions meant every gap could be aimed at from two sides. */ await drag(rows.nth(2), rows.nth(0)); + const under = await titles(); + check( + under[1] === beforeDrag[2] && under[0] === beforeDrag[0], + "dropping a note on a row puts it under that row", + `${beforeDrag.join(",")} → ${under.join(",")}` + ); + // Which leaves the very top out of reach, so it has a strip of its own. + await drag(rows.nth(2), p.locator("[data-drop-start]")); const arranged = await titles(); check( - arranged[0] === beforeDrag[2], - "dragging a note to the top puts it there", - `${beforeDrag.join(",")} → ${arranged.join(",")}` + arranged[0] === under[2], + "and the strip above the first row puts it at the very top", + `${under.join(",")} → ${arranged.join(",")}` ); /* The search box is simply there, with nothing to open first. A magnifying @@ -293,6 +303,47 @@ try { await p.waitForTimeout(800); check((await p.locator("[data-separator]").count()) === 1, "and back on the way in"); + /* ── a new note goes on top, and takes nothing with it ── + * + * It used to be given no position at all and left to the tie-break, which put + * it near the top and moved everything else: a line's position counts the rows + * above it, so a row appearing without a renumber left every line one row too + * high, and the arrangement came apart a little more with each new note. */ + const layout = () => + p + .locator("[data-note-row], [data-separator]") + .evaluateAll((els) => + els.map((el) => + el.hasAttribute("data-separator") ? `── ${el.innerText.trim()}` : el.innerText.split("\n")[0].trim() + ) + ); + const beforeNew = await layout(); + await createNote(p); + await p.waitForTimeout(2500); + const afterNew = await layout(); + check( + afterNew[0] === "Untitled Note", + "a new note is made at the very top, above the lines as well", + afterNew.join(" | ") + ); + check( + afterNew.slice(1).join(" | ") === beforeNew.join(" | "), + "and every other row stayed exactly where it was", + `${beforeNew.join(" | ")} → ${afterNew.join(" | ")}` + ); + + // And it survives the trip to the server: the positions were written, not + // only drawn. + await p.reload({ waitUntil: "networkidle" }); + await p.waitForTimeout(2500); + await p.locator("button").filter({ hasText: "Ordnung" }).first().click(); + await p.waitForTimeout(1800); + check( + (await layout()).join(" | ") === afterNew.join(" | "), + "and the arrangement comes back the same after a reload", + (await layout()).join(" | ") + ); + /* ── the choices reach the server, and the screen ── * A notebook divided into three parts is easier to read at a glance when the * lines differ; grey-on-grey they all look like the same line repeated. */ @@ -338,7 +389,11 @@ try { (await p.locator("[data-confirm-dialog]").count()) === 0, "and the question is gone with it, not left standing over the app" ); - check((await titles()).length === 3, "with every note still there", (await titles()).join(", ")); + check( + (await titles()).join(",") === afterNew.filter((row) => !row.startsWith("──")).join(","), + "with every note still there", + (await titles()).join(", ") + ); /* ── exporting a note and importing it back ── */ await p.locator("[data-note-row]").filter({ hasText: "Beta" }).first().click(); @@ -640,6 +695,20 @@ try { "and the notebooks it offers are on the screen, not past its edge", JSON.stringify(await sub.boundingBox()) ); + + /* It holds two different kinds of place, so it says which is which. The + headings are deliberately not shaped like entries: one built like a menu row + is a button that does nothing when pressed. */ + const headings = await sub.locator("[data-menu-heading]").allInnerTexts(); + check( + headings.some((h) => /other notebooks/i.test(h)), + "the list is divided under headings rather than run together", + headings.join(" / ") + ); + check( + (await sub.locator("[data-menu-heading]").evaluateAll((els) => els.every((el) => el.tagName !== "BUTTON"))), + "and a heading is not a button" + ); await p.keyboard.press("Escape"); await p.waitForTimeout(400); await p.setViewportSize({ width: 1400, height: 950 }); diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index 7d1e292..7fa3de9 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -683,26 +683,49 @@ export default function App() { */ const moveSubmenu = (note) => { const here = data.chapters.filter((c) => c.notebookId === note.folderId && c.id !== note.chapterId); - const targets = [ - ...(note.chapterId - ? [{ label: "The notebook itself", action: () => data.moveNote(note.id, note.folderId, null) }] - : []), - ...here.map((c) => ({ - label: c.name, - icon: I.books, - action: () => data.moveNote(note.id, note.folderId, c.id), - })), - ...(here.length || note.chapterId ? [{ divider: true }] : []), - ...notebooks - .filter((nb) => nb.id !== note.folderId) - .map((nb) => ({ + const home = notebooks.find((nb) => nb.id === note.folderId); + const elsewhere = notebooks.filter((nb) => nb.id !== note.folderId); + const targets = []; + + /* The notebook the note is already in, when it is sitting in one of that + notebook's chapters — coming back out is a move like any other, and it + wears the notebook's own cover so it is read as a place. */ + if (note.chapterId && home) { + targets.push({ + label: home.name, + colorDot: coverBackground(home.cover, home.coverPattern), + action: () => data.moveNote(note.id, home.id, null), + }); + } + /* Grouped and named, because the list holds two different kinds of place and + used to run them together with a rule between: a chapter of this notebook + is one step, another notebook is a different one. Each row carries the + mark its chapter or notebook was given, so it is found the way it is found + everywhere else. */ + if (here.length) { + targets.push({ heading: "Chapters here" }); + targets.push( + ...here.map((c) => ({ + label: c.name, + icon: c.icon || I.books, + iconColor: c.color || undefined, + action: () => data.moveNote(note.id, note.folderId, c.id), + })) + ); + } + if (elsewhere.length) { + targets.push({ heading: "Other notebooks" }); + targets.push( + ...elsewhere.map((nb) => ({ label: nb.name, colorDot: coverBackground(nb.cover, nb.coverPattern), action: () => data.moveNote(note.id, nb.id, null), - })), - ]; - // A lone divider is not a menu. - const usable = targets.filter((t) => !t.divider); + })) + ); + } + + // Headings alone are not a menu. + const usable = targets.filter((t) => !t.divider && !t.heading); return usable.length > 0 ? [{ label: "Move to...", icon: I.move, submenu: targets }] : []; }; diff --git a/frontend/src/editor/editor.css b/frontend/src/editor/editor.css index 9aa920f..31d0a2c 100644 --- a/frontend/src/editor/editor.css +++ b/frontend/src/editor/editor.css @@ -249,25 +249,34 @@ align-items: flex-start; gap: 8px; } -/* The box sits in the middle of the *first line* beside it, not at a fixed - nudge from the top. `margin-top: 0.35em` was measured once against one font at - one size; changing the note's typeface, or writing a task that wraps, left the - box floating above the text it belongs to. Giving the label the height of one - line and centring inside it holds for every size. */ +/* The box is centred on the *letters* of the first line beside it — not on that + line's box, and not at a fixed nudge from the top. + + `margin-top: 0.35em` was measured once against one font at one size. Centring + in the full line was the next attempt and is still not it: a line box is + taller than the letters in it by the depth of a descender, so its middle sits + below the middle of a capital. Measured, that left the box's top level with + the capitals and its bottom 4px past the baseline — a square hanging off the + line. Two tenths of an em off the line's height is where the capitals' + middle actually is, for every face on offer here. */ .note-editor .ProseMirror ul[data-type="taskList"] li > label { flex: 0 0 auto; display: flex; align-items: center; - height: calc(1em * var(--ne-line)); + height: calc(1em * var(--ne-line) - 0.3em); user-select: none; } .note-editor .ProseMirror ul[data-type="taskList"] li > div { flex: 1 1 auto; } +/* Sized in em, so the box stays the size of a capital whatever the note's type + size is. At 15px it was a 15px square against a 10px letter, which read as a + box the text was standing next to rather than part of the same line. */ .note-editor .ProseMirror ul[data-type="taskList"] input[type="checkbox"] { accent-color: var(--ne-accent); - width: 15px; - height: 15px; + width: 0.9em; + height: 0.9em; + margin: 0; cursor: pointer; } .note-editor .ProseMirror ul[data-type="taskList"] li[data-checked="true"] > div { diff --git a/frontend/src/editor/noteLink.js b/frontend/src/editor/noteLink.js index fe86af4..db445a2 100644 --- a/frontend/src/editor/noteLink.js +++ b/frontend/src/editor/noteLink.js @@ -162,7 +162,10 @@ function renderSuggestionList() { const pick = (i) => { const item = items[i]; - if (item && command) command({ id: item.id, label: item.label, kind: item.kind }); + // The mark travels with the choice. Dropped here, every new link was born + // wearing the default sheet of paper — the menu showed the note's own mark + // and the link that came out of it did not. + if (item && command) command({ id: item.id, label: item.label, kind: item.kind, icon: item.icon || null }); }; const place = (rect) => { diff --git a/frontend/src/editor/useNoteEditor.jsx b/frontend/src/editor/useNoteEditor.jsx index f6cad9b..590889d 100644 --- a/frontend/src/editor/useNoteEditor.jsx +++ b/frontend/src/editor/useNoteEditor.jsx @@ -133,29 +133,60 @@ export function EditorSurface({ editor, dark, font, minHeight, onOpenNote, resol * node — the link survived on screen and vanished on the next reload. */ useEffect(() => { if (!editor || editor.isDestroyed || !resolveLinkLabel) return; - const edits = []; - editor.state.doc.descendants((node, pos) => { - if (node.type.name !== "noteLink") return; - const label = resolveLinkLabel(node.attrs.noteId); - const missing = label === null; - const nextLabel = missing ? node.attrs.label : label; - // The mark travels the same way, and for the same reason: it belongs to - // the note, so it has to follow when the note's changes. - const icon = (resolveLinkIcon ? resolveLinkIcon(node.attrs.noteId) : node.attrs.icon) ?? null; - if ( - nextLabel !== node.attrs.label || - missing !== !!node.attrs.missing || - icon !== (node.attrs.icon ?? null) - ) { - edits.push([pos, { ...node.attrs, label: nextLabel, missing, icon }]); - } - }); - if (edits.length === 0) return; - const tr = editor.state.tr; - for (const [pos, attrs] of edits) tr.setNodeMarkup(pos, undefined, attrs); - tr.setMeta(LABEL_REFRESH, true); - tr.setMeta("addToHistory", false); // undo should not step through renames - editor.view.dispatch(tr); + + const refresh = () => { + if (editor.isDestroyed) return; + const edits = []; + editor.state.doc.descendants((node, pos) => { + if (node.type.name !== "noteLink") return; + const label = resolveLinkLabel(node.attrs.noteId); + const missing = label === null; + const nextLabel = missing ? node.attrs.label : label; + // The mark travels the same way, and for the same reason: it belongs to + // the note, so it has to follow when the note's changes. + const icon = (resolveLinkIcon ? resolveLinkIcon(node.attrs.noteId) : node.attrs.icon) ?? null; + if ( + nextLabel !== node.attrs.label || + missing !== !!node.attrs.missing || + icon !== (node.attrs.icon ?? null) + ) { + edits.push([pos, { ...node.attrs, label: nextLabel, missing, icon }]); + } + }); + if (edits.length === 0) return; + const tr = editor.state.tr; + for (const [pos, attrs] of edits) tr.setNodeMarkup(pos, undefined, attrs); + tr.setMeta(LABEL_REFRESH, true); + tr.setMeta("addToHistory", false); // undo should not step through renames + editor.view.dispatch(tr); + }; + + refresh(); + + /* And again whenever the document changes. + * + * Running once was not enough, and the gap was the ordinary case: this + * effect runs while the editor is still empty, and the note's content is + * poured in afterwards — deliberately without an update event, so the + * editor is not marked dirty by its own loading. Every link therefore kept + * whatever it was saved with, which is why a reference always showed the + * sheet of paper however its target was marked. + * + * Deferred by a frame rather than dispatched from inside the handler: + * ProseMirror is still finishing the transaction that brought us here. */ + let frame = 0; + const schedule = () => { + if (frame) return; + frame = requestAnimationFrame(() => { + frame = 0; + refresh(); + }); + }; + editor.on("transaction", schedule); + return () => { + if (frame) cancelAnimationFrame(frame); + editor.off("transaction", schedule); + }; }, [editor, resolveLinkLabel, resolveLinkIcon]); const handleClick = useCallback( diff --git a/frontend/src/features/notes/EditorPane.jsx b/frontend/src/features/notes/EditorPane.jsx index d572537..16f25d3 100644 --- a/frontend/src/features/notes/EditorPane.jsx +++ b/frontend/src/features/notes/EditorPane.jsx @@ -197,21 +197,30 @@ export function EditorPane({ small gap of their own. Left as separate children of the header they were spaced like the buttons at the far end — the mark read as a control that happened to be parked beside the title. */} -
+ {/* Aligned on the baseline, not on the box. + Centring the two boxes looked right on paper and wrong on screen: a + title's box is taller than its letters by the depth of a descender, + so its capitals sit above the middle — while an emoji's picture fills + its box and sat on the middle. Measured, the mark ended up 2px high + and dipped below the line the title never crossed. Sharing a baseline + is what "on one line" means, and it needs no number. */} +
) : ( <> + {/* The one place a drop under a row cannot reach. Only while the + list is being rearranged, so it costs nothing the rest of the + time. */} + {moving && ( +
+ {dnd.overId === START_ID && } +
+ )} {entries.map((entry) => { + /* The line is drawn *under* the row, because that is where the + row being carried is about to land. One direction only: a gap + that can be aimed at from both sides is a gap nobody can aim + at, and the two ends of the list have strips of their own. + + The whole wrapper, line included, is the drop target. On the + inner element the line itself was a dead strip — it grows as it + appears, so the pointer could end up over the line rather than + over any row, and releasing there dropped on nothing. */ const showDropLine = moving && dnd.overId === entry.id; - /* The whole row, indicator included, is the drop target. - * - * Putting these on the inner element left the indicator itself as - * a dead strip: it pushes the row down as it appears, so the - * pointer ended up over the line rather than over the row, and - * releasing there dropped on nothing. */ const rowProps = moving ? dnd.rowProps(entry.id) : {}; const lifted = dnd.draggingId === entry.id; @@ -870,8 +881,9 @@ export function NoteList({ borderRadius: 10, }} > - {showDropLine && } -
+ {/* The header alone is the chapter for the purpose of a + drop — an open one carries its notes inside this row. */} +
{moving && }
{expandedChapter === entry.id && renderChapterNotes?.(entry.chapter)} + {showDropLine && }
); } @@ -889,13 +902,13 @@ export function NoteList({ if (entry.kind === "separator") { return (
- {showDropLine && } : null} T={T} separator={entry.separator} onContextMenu={onSeparatorContextMenu} /> + {showDropLine && }
); } @@ -904,7 +917,6 @@ export function NoteList({ const snippet = snippetFor(note.id); return (
- {showDropLine && }
{moving && }
+ {showDropLine && }
); })} diff --git a/frontend/src/features/notes/noteSort.test.ts b/frontend/src/features/notes/noteSort.test.ts index 255a3ca..e200889 100644 --- a/frontend/src/features/notes/noteSort.test.ts +++ b/frontend/src/features/notes/noteSort.test.ts @@ -2,8 +2,10 @@ import { describe, expect, it } from "vitest"; import { END_ID, + START_ID, canReorder, mergeEntries, + placeOnTop, reindex, reindexEntries, sortModesFor, @@ -89,8 +91,10 @@ describe("reindex", () => { note("d", { sortOrder: 3 }), ]; - it("moves a note onto the position of the one it was dropped on", () => { - expect(reindex(all, "d", "b")).toEqual(["a", "d", "b", "c"]); + /* Under, always under. Both directions meant every gap could be aimed at from + two sides, which is what made a drag hard to predict. */ + it("moves a note under the one it was dropped on", () => { + expect(reindex(all, "d", "b")).toEqual(["a", "b", "d", "c"]); }); it("works upwards as well as downwards", () => { @@ -198,18 +202,25 @@ describe("reindexEntries", () => { const notes = [note("a", { sortOrder: 0 }), note("b", { sortOrder: 1 }), note("c", { sortOrder: 2 })]; const separators = [{ id: "s1", label: "Teil 2", order: 1 }]; - it("moves a note across a separator and keeps the separator where it was", () => { + it("moves a note across a separator and leaves the line above the same note", () => { + // a │ ─── │ b │ c, and c is dropped under a. The line still sits above b; + // its number counts the rows before it, and there is one more of those now. const next = reindexEntries(notes, separators, "c", "a")!; - expect(next.noteIds).toEqual(["c", "a", "b"]); + expect(next.noteIds).toEqual(["a", "c", "b"]); expect(next.separators[0]!.order).toBe(2); }); it("moves a separator itself", () => { - const next = reindexEntries(notes, separators, "s1", "a")!; + const next = reindexEntries(notes, separators, "s1", START_ID)!; expect(next.separators[0]!.order).toBe(0); expect(next.noteIds).toEqual(["a", "b", "c"]); }); + it("sends a row to the top when dropped on the strip above the first one", () => { + // The one place a drop under a row cannot reach. + expect(reindexEntries(notes, [], "c", START_ID)!.noteIds).toEqual(["c", "a", "b"]); + }); + it("sends a row to the end when dropped below the last one", () => { expect(reindexEntries(notes, [], "a", END_ID)!.noteIds).toEqual(["b", "c", "a"]); }); @@ -224,3 +235,45 @@ describe("reindexEntries", () => { expect(next.separators).toEqual([{ id: "s1", label: "x", order: 0 }]); }); }); + +describe("placeOnTop", () => { + /* A notebook someone has arranged: two notes with a line between them, and a + chapter above both. The numbers are what a drag left behind. */ + const notes = [note("a", { sortOrder: 1 }), note("b", { sortOrder: 2 })]; + const chapters = [{ id: "ch", notebookId: "nb", name: "Kapitel", sortOrder: 0 }] as never; + const separators = [{ id: "s1", label: "Teil 2", order: 2 }]; + const kinds = (list: ReturnType) => list.map((e) => `${e.kind[0]}:${e.id}`); + + const fresh = [note("neu", { sortOrder: 0 }), ...notes]; + const arranged = placeOnTop(fresh, separators, chapters, "neu")!; + + it("puts the new note above everything, lines and chapters included", () => { + expect(arranged.notes[0]).toEqual({ id: "neu", order: 0 }); + expect(arranged.chapters).toEqual([{ id: "ch", order: 1 }]); + }); + + it("leaves every other row in the order it was in", () => { + expect(arranged.noteIds).toEqual(["neu", "a", "b"]); + }); + + /* The whole point. A separator's number counts the rows above it, so a row + appearing at the top without a renumber leaves every line one row too high + — which is how a notebook came apart a little more with each new note. */ + it("moves the lines down with the rows they sat above", () => { + expect(arranged.separators[0]!.order).toBe(3); + // Drawn again from exactly what was written back. + const after = [note("neu", { sortOrder: 0 }), note("a", { sortOrder: 2 }), note("b", { sortOrder: 3 })]; + const movedChapters = [{ id: "ch", notebookId: "nb", name: "Kapitel", sortOrder: 1 }] as never; + expect(kinds(mergeEntries(after, arranged.separators, movedChapters))).toEqual([ + "n:neu", + "c:ch", + "n:a", + "s:s1", + "n:b", + ]); + }); + + it("reports nothing to do for a note that is not in the list", () => { + expect(placeOnTop(notes, separators, chapters, "ghost")).toBeNull(); + }); +}); diff --git a/frontend/src/features/notes/noteSort.ts b/frontend/src/features/notes/noteSort.ts index 13ed6a7..221fae1 100644 --- a/frontend/src/features/notes/noteSort.ts +++ b/frontend/src/features/notes/noteSort.ts @@ -203,6 +203,13 @@ export function toArrangement(entries: Entry[]): Arrangement { * * `notes` must be every note in the notebook, not the filtered view. Returns * null when the move changes nothing, which spares the caller a pointless write. + * + * A drop lands the row **under** the one it was released on, and only ever + * there. Dropping above a row and dropping below the row before it are the same + * move described twice, and having both meant every gap could be aimed at from + * two sides — which is a hard thing to aim at and a harder thing to read. The + * two strips, one above the first row and one below the last, are how the ends + * are reached. */ export function reindexEntries( notes: NoteMeta[], @@ -216,21 +223,64 @@ export function reindexEntries( const from = entries.findIndex((e) => e.id === dragId); if (from < 0) return null; - // The strip below the last row means "put it at the end" — otherwise moving - // something past the final note needs a drop onto the note itself, which puts - // it above. - const to = targetId === END_ID ? entries.length - 1 : entries.findIndex((e) => e.id === targetId); - if (to < 0) return null; - const [moved] = entries.splice(from, 1); if (!moved) return null; - entries.splice(to, 0, moved); + + if (targetId === START_ID) entries.unshift(moved); + else if (targetId === END_ID) entries.push(moved); + else { + // Found after the row has been lifted out, so the index is the one in the + // list the row is being dropped into. + const to = entries.findIndex((e) => e.id === targetId); + if (to < 0) return null; + entries.splice(to + 1, 0, moved); + } + return toArrangement(entries); +} + +/** + * The whole list renumbered with one row put at the very top. + * + * For a note that has just been made. It used to be given no position at all and + * left to the tie-break, which put it near the top and moved everything else: + * a separator's position counts the rows above it, so a row appearing at the top + * without anybody being renumbered left every line one row too high — the + * notebook's arrangement came apart a little more with each new note. + * + * Renumbering the list settles that, and settles it for good: rows that were + * never dragged all carried position 0 and were being ordered by how recently + * they changed, so they slid about on their own. Afterwards each keeps the place + * it is in — which is what a hand-made order is for. + */ +export function placeOnTop( + notes: NoteMeta[], + separators: Separator[], + chapters: Chapter[], + id: string +): Arrangement | null { + const note = notes.find((n) => n.id === id); + if (!note) return null; + /* Built from the list *without* the new note, then the note put in front. + * + * Merging it in first would be measuring the room after the furniture moved: + * the new row already counts as a row, so every line reads as sitting one + * place further down than it was drawn, and renumbering would write that + * misreading back as the truth. */ + const entries = mergeEntries( + notes.filter((n) => n.id !== id), + separators, + chapters + ); + entries.unshift({ kind: "note", id, order: 0, note }); return toArrangement(entries); } /** Drop target standing for "after everything". */ export const END_ID = "__end__"; +/** And its opposite: the one place a drop under a row cannot reach. */ +export const START_ID = "__start__"; + /** A new separator, added at the top where it can be seen and then dragged. */ export const withNewSeparator = ( separators: Separator[], diff --git a/frontend/src/hooks/useNoteData.ts b/frontend/src/hooks/useNoteData.ts index ab43770..dd75cca 100644 --- a/frontend/src/hooks/useNoteData.ts +++ b/frontend/src/hooks/useNoteData.ts @@ -3,9 +3,19 @@ import { useCallback, useEffect, useRef, useState } from "react"; import * as sync from "../syncEngine"; import * as searchIndex from "../search"; import { sameStrokeDoc } from "../features/drawing/strokes.js"; -import { reindexEntries } from "../features/notes/noteSort"; +import { placeOnTop, reindexEntries } from "../features/notes/noteSort"; +import type { Arrangement } from "../features/notes/noteSort"; import { extractOnImport } from "../features/pdf/pdfText"; -import type { Backlink, Chapter, NoteContent, NoteMeta, Notebook, StrokeDoc, User } from "../types"; +import type { + Backlink, + Chapter, + NoteContent, + NoteMeta, + Notebook, + Separator, + StrokeDoc, + User, +} from "../types"; /** How often the whole account is reconciled with the server. */ const FULL_SYNC_MS = 30_000; @@ -202,34 +212,6 @@ export function useNoteData(user: User | null, activeNote: string | null): NoteD /* ── Notes ── */ - const createNote = useCallback(async (folderId: string, chapterId: string | null = null) => { - const note = await sync.createNote(folderId, chapterId); - setNotes((p) => [note, ...p]); - setNoteContents((p) => ({ ...p, [note.id]: EMPTY_CONTENT })); - return note; - }, []); - - /** - * Uploads a PDF and creates the note that holds it. - * - * Not queued for offline: the file has to reach the server before a note can - * point at it, and a note referring to bytes nobody has would be worse than - * being told to try again with a connection. - */ - const importPdf = useCallback(async (folderId: string, file: File) => { - const attachment = await sync.uploadPdf(file, file.name); - const title = file.name.replace(/\.pdf$/i, "") || "Dokument"; - const note = await sync.createPdfNote(folderId, title, attachment.id); - setNotes((p) => [note, ...p]); - /* Read the document here, now, on the machine that already has it open. - * - * Waiting until someone opens it means a filed invoice may never be read at - * all, and until it is, no device can find it. Not awaited: the note exists - * and should appear at once — the text catches up. */ - void extractOnImport(file, attachment.id); - return note; - }, []); - const updateNote = useCallback((id: string, edit: NoteEdit) => { if (edit.title !== undefined) { const title = edit.title; @@ -294,71 +276,158 @@ export function useNoteData(user: User | null, activeNote: string | null): NoteD }, []); /** - * Puts one row where another was, in the user's own order. + * Everything one list is made of: its notes, its lines, its chapters. * - * Notes and separators move the same way, so this takes both and writes both - * back. The new positions are computed over *every* note in that notebook, - * taken from the store rather than from whatever the list is showing: a search - * or a filter must never be able to renumber the notes it is hiding. + * `chapterId` scopes it to one list — the notebook itself, or one of its + * chapters. A row's place is only meaningful beside the rows it is shown with, + * and the separators of the other chapters have to come through a write + * untouched. The notes are taken from the store rather than from whatever the + * list is showing: a search must never be able to renumber what it is hiding. + * + * `extra` is a note that exists but has not reached the store yet — the one + * just created. Building the list without it would renumber the notebook + * around a row that is already on screen. */ - /* `chapterId` scopes the arrangement to one list: the notebook itself, or one - of its chapters. A note's place is only meaningful beside the notes it is - shown with, and the separators of the other chapters must come through the - write untouched. */ - const rearrange = useCallback( - (notebookId: string, dragId: string, targetId: string, chapterId: string | null = null) => { + const listOf = useCallback((notebookId: string, chapterId: string | null, extra?: NoteMeta) => { const notebook = notebooksRef.current.find((nb) => nb.id === notebookId); - if (!notebook) return; - const inNotebook = notesRef.current.filter( + if (!notebook) return null; + const known = extra ? [extra, ...notesRef.current.filter((n) => n.id !== extra.id)] : notesRef.current; + const notes = known.filter( (n) => !n.trashed && n.folderId === notebookId && (n.chapterId ?? null) === chapterId ); const all = notebook.separators || []; - const mine = all.filter((sep) => (sep.chapterId ?? null) === chapterId); - const others = all.filter((sep) => (sep.chapterId ?? null) !== chapterId); - /* Chapters are rows in this list too, and only in the notebook itself — a - chapter has no chapters of its own. */ - const inList = chapterId - ? [] - : chaptersRef.current.filter((c) => c.notebookId === notebookId); + return { + notes, + all, + mine: all.filter((sep) => (sep.chapterId ?? null) === chapterId), + others: all.filter((sep) => (sep.chapterId ?? null) !== chapterId), + /* Chapters are rows in this list too, and only in the notebook itself — a + chapter has no chapters of its own. */ + chapters: chapterId ? [] : chaptersRef.current.filter((c) => c.notebookId === notebookId), + }; + }, []); - const next = reindexEntries(inNotebook, mine, dragId, targetId, inList); - if (!next) return; - const merged = [...others, ...next.separators.map((sep) => ({ ...sep, chapterId }))]; + /** Writes an arrangement back — to the screen at once, to the server behind it. */ + const applyArrangement = useCallback( + ( + notebookId: string, + chapterId: string | null, + all: Separator[], + others: Separator[], + next: Arrangement + ) => { + const merged = [...others, ...next.separators.map((sep) => ({ ...sep, chapterId }))]; - const position = new Map(next.notes.map((n) => [n.id, n.order])); - setNotes((p) => p.map((n) => (position.has(n.id) ? { ...n, sortOrder: position.get(n.id)! } : n))); - void sync.reorderNotes(next.notes); + const position = new Map(next.notes.map((n) => [n.id, n.order])); + setNotes((p) => p.map((n) => (position.has(n.id) ? { ...n, sortOrder: position.get(n.id)! } : n))); + void sync.reorderNotes(next.notes); - if (next.chapters.length) { - const chapterAt = new Map(next.chapters.map((c) => [c.id, c.order])); - setChapters((p) => p.map((c) => (chapterAt.has(c.id) ? { ...c, sortOrder: chapterAt.get(c.id)! } : c))); - void sync.reorderChapters(next.chapters); - } + if (next.chapters.length) { + const chapterAt = new Map(next.chapters.map((c) => [c.id, c.order])); + setChapters((p) => p.map((c) => (chapterAt.has(c.id) ? { ...c, sortOrder: chapterAt.get(c.id)! } : c))); + void sync.reorderChapters(next.chapters); + } - /* Only written when it actually changed. - * - * A plain note drag leaves the separators alone, and a write for nothing - * costs more than a request: it reloads the notes, which reshuffles any that - * share an `updatedAt` second — so "last changed" quietly came out in a - * different order after every drag. - * - * Compared as a set rather than as an array, because the merge above puts - * the other chapters' separators first and that says nothing about whether - * anything changed. */ - const fingerprint = (list: typeof all) => - list - .map((sep) => `${sep.id}|${sep.label}|${sep.order}|${sep.chapterId ?? ""}`) - .sort() - .join("\n"); - const before = fingerprint(all); - if (fingerprint(merged) !== before) { - setNotebooks((p) => p.map((nb) => (nb.id === notebookId ? { ...nb, separators: merged } : nb))); - void sync.updateNotebook(notebookId, { separators: merged }); - } + /* Only written when it actually changed. + * + * A plain note drag leaves the separators alone, and a write for nothing + * costs more than a request: it reloads the notes, which reshuffles any + * that share an `updatedAt` second — so "last changed" quietly came out in + * a different order after every drag. + * + * Compared as a set rather than as an array, because the merge above puts + * the other chapters' separators first and that says nothing about whether + * anything changed. */ + const fingerprint = (seps: Separator[]) => + seps + .map((sep) => `${sep.id}|${sep.label}|${sep.order}|${sep.chapterId ?? ""}`) + .sort() + .join("\n"); + if (fingerprint(merged) !== fingerprint(all)) { + setNotebooks((p) => p.map((nb) => (nb.id === notebookId ? { ...nb, separators: merged } : nb))); + void sync.updateNotebook(notebookId, { separators: merged }); + } }, [] ); + /** + * Puts one row under the one it was dropped on, in the user's own order. + * + * Notes, lines and chapters move the same way, so this takes all three and + * writes all three back. + */ + const rearrange = useCallback( + (notebookId: string, dragId: string, targetId: string, chapterId: string | null = null) => { + const list = listOf(notebookId, chapterId); + if (!list) return; + const next = reindexEntries(list.notes, list.mine, dragId, targetId, list.chapters); + if (!next) return; + applyArrangement(notebookId, chapterId, list.all, list.others, next); + }, + [applyArrangement, listOf] + ); + + /** + * Puts a freshly made note at the top of the list it was made in. + * + * A new note used to be given no position at all, and the sort put it near the + * top by age. That is not the same thing: a separator's position counts the + * rows above it, so a row that appears without anybody being renumbered leaves + * every line one row too high — and each new note pulled the notebook's + * arrangement further apart. Renumbering the list once, here, keeps every + * other row exactly where it was and puts the new one above all of them, + * lines and chapters included. + */ + const putOnTop = useCallback( + (note: NoteMeta) => { + if (!note.folderId) return; + const chapterId = note.chapterId ?? null; + const list = listOf(note.folderId, chapterId, note); + if (!list) return; + const next = placeOnTop(list.notes, list.mine, list.chapters, note.id); + if (!next) return; + applyArrangement(note.folderId, chapterId, list.all, list.others, next); + }, + [applyArrangement, listOf] + ); + + const createNote = useCallback( + async (folderId: string, chapterId: string | null = null) => { + const note = await sync.createNote(folderId, chapterId); + setNotes((p) => [note, ...p]); + setNoteContents((p) => ({ ...p, [note.id]: EMPTY_CONTENT })); + putOnTop(note); + return note; + }, + [putOnTop] + ); + + /** + * Uploads a PDF and creates the note that holds it. + * + * Not queued for offline: the file has to reach the server before a note can + * point at it, and a note referring to bytes nobody has would be worse than + * being told to try again with a connection. + */ + const importPdf = useCallback( + async (folderId: string, file: File) => { + const attachment = await sync.uploadPdf(file, file.name); + const title = file.name.replace(/\.pdf$/i, "") || "Dokument"; + const note = await sync.createPdfNote(folderId, title, attachment.id); + setNotes((p) => [note, ...p]); + putOnTop(note); + /* Read the document here, now, on the machine that already has it open. + * + * Waiting until someone opens it means a filed invoice may never be read + * at all, and until it is, no device can find it. Not awaited: the note + * exists and should appear at once — the text catches up. */ + void extractOnImport(file, attachment.id); + return note; + }, + [putOnTop] + ); + /* ── Notebooks ── */ /* ── Chapters ── diff --git a/frontend/src/hooks/useReorderDrag.ts b/frontend/src/hooks/useReorderDrag.ts index 11574c7..f355573 100644 --- a/frontend/src/hooks/useReorderDrag.ts +++ b/frontend/src/hooks/useReorderDrag.ts @@ -106,10 +106,27 @@ export function useReorderDrag( const row = el?.closest?.("[data-reorder-id]") as HTMLElement | null; const target = row?.dataset.reorderId ?? null; const next = target && target !== d.id ? target : null; - /* A chapter takes what is dropped on it rather than standing beside it, - * which is what "drag it into the chapter" means. Filing is only offered - * when there is somewhere to file *to*. */ - const into = next && row?.dataset.reorderInto !== undefined && fileInto ? next : null; + /* A chapter is both a place and a neighbour, so which one it is depends on + * where in the row the pointer is. + * + * Anywhere on the row meant "into", and that quietly took reordering away: + * a chapter could no longer be moved among its neighbours, and nothing — + * note or line — could be put in the gap under one. The bottom edge is + * that gap; the body of the row is the chapter itself and takes what is + * carried in. The indicator says which: a line in the gap, the whole row + * lit up when it is about to take something. */ + let into: string | null = null; + if (next && row && row.dataset.reorderInto !== undefined && fileInto) { + // The header, not the row: an open chapter's row includes the notes + // inside it, and an edge measured on *that* would be half a notebook. + const head = (row.querySelector("[data-reorder-head]") as HTMLElement | null) || row; + const rect = head.getBoundingClientRect(); + const edge = Math.min(10, rect.height / 3); + // Only that band is the gap. Below it lie the chapter's own notes, and + // holding over them plainly means the chapter. + const inGap = e.clientY >= rect.bottom - edge && e.clientY <= rect.bottom; + if (!inGap) into = next; + } if (into !== intoRef.current) { intoRef.current = into; setIntoId(into); diff --git a/frontend/src/ui/ContextMenu.jsx b/frontend/src/ui/ContextMenu.jsx index 53f5469..af94920 100644 --- a/frontend/src/ui/ContextMenu.jsx +++ b/frontend/src/ui/ContextMenu.jsx @@ -10,6 +10,7 @@ export function ContextMenu({ x, y, items, onClose, dark }) { border: dark ? "#30363d" : "#d0d7de", text: dark ? "#e6edf3" : "#1f2328", hover: dark ? "#262c36" : "#f3f4f6", + faint: dark ? "#7d8590" : "#8c959f", danger: dark ? "#f85149" : "#cf222e", }), [dark] @@ -76,15 +77,56 @@ export function ContextMenu({ x, y, items, onClose, dark }) { }} onClick={(e) => e.stopPropagation()} > - {items.map((item, i) => - item.divider ? ( -
- ) : item.submenu ? ( - - ) : ( - - ) - )} + {renderItems(items, T, onClose)} +
+ ); +} + +/** + * The items of one menu, in order. + * + * Shared by the menu and by anything that flies out of it, so a heading or a + * divider reads the same wherever it appears. + */ +function renderItems(items, T, onClose, small) { + return items.map((item, i) => + item.divider ? ( +
+ ) : item.heading ? ( + + {item.heading} + + ) : item.submenu ? ( + + ) : ( + + ) + ); +} + +/** + * What the rows under it are. + * + * Deliberately unlike the rows: small, faint, spaced out and set left of them. + * Given the shape of a menu entry it would be a button that does nothing when + * pressed, which is worse than no heading at all. + */ +function Heading({ T, children }) { + return ( +
+ {children}
); } @@ -119,7 +161,23 @@ function MenuButton({ T, item, onClose, small }) { {item.colorDot && (
)} - {item.icon && {item.icon}} + {/* A chapter's own mark, in its own colour, at full strength — the point + of giving one a mark is to find it by that and not by reading. The + drawn glyphs stay a shade back, as they always were. */} + {item.icon && ( + + {item.icon} + + )} {item.label} ); @@ -180,9 +238,7 @@ function SubMenu({ item, T, onClose }) { } } > - {item.submenu.map((sub, j) => ( - - ))} + {renderItems(item.submenu, T, onClose, true)}
); diff --git a/readme.md b/readme.md index b797974..711aa5c 100644 --- a/readme.md +++ b/readme.md @@ -332,6 +332,14 @@ devices get different starts, because they want different things: The row under the finger is found with `elementFromPoint`, not with enter/leave handlers — a finger generates no `pointerover` on anything it did not press. +**A drop lands the row under the one it was released on, and only ever there.** +Dropping above a row and dropping below the row before it are the same move +described twice, so every gap could be aimed at from two sides — which made a +drag hard to aim and harder to read. One direction, one line, drawn under the +row it will land under. The two ends of the list have strips of their own: the +one below the last row was always there, and there is now one above the first, +because that is the single place a drop *under* something cannot reach. + **Rearranging is a mode**, switched on by the handle button beside the sort menu and off again by itself when the list is left or the order stops being a hand-made one. Only while it is on does a row wear its grip or move at all. Rows @@ -348,6 +356,13 @@ hover, there was no way to open it in the first place. It is now placed by measuring, opens on a tap, unfolds in place on a narrow screen, and flips to the other side on a wide one when the right would not fit. +*Move to…* holds two different kinds of place, so it **says which is which**: +the notebook itself, then *Chapters here*, then *Other notebooks*. The headings +are deliberately not shaped like entries — small, faint and set apart — because +a heading built like a menu row is a button that does nothing when pressed. Each +row carries the mark its chapter was given or the cover its notebook wears, so a +place is recognised in the menu the same way it is recognised in the list. + ### Installing it The install entry used to appear only when the browser had offered a prompt @@ -1017,8 +1032,15 @@ took three visits. Its menu therefore holds one *Edit* and one *Remove*. **A row is put into a chapter by dragging it onto it.** While rearranging is on, a chapter row takes what is dropped on it and lights up to say so, rather than -growing a drop line above it: a chapter is a place, not a neighbour. Notes and -separators both go in this way. +growing a drop line: a chapter is a place, not a neighbour. Notes and separators +both go in this way. + +But it is a neighbour as well, and taking the *whole* row as "into" quietly took +that away — a chapter could no longer be moved among its neighbours, and nothing +could be put in the gap under one. So the **bottom edge of a chapter's header is +the gap** and the rest of it is the chapter. The header, not the row: an open +chapter carries its notes inside that row, and holding over those plainly means +the chapter. A chapter shows even when the notebook holds nothing else. "Nothing here" is asked of every row, not of the notes alone — asked of the notes, a notebook whose @@ -1083,6 +1105,16 @@ still reads newest-first — untouched rows all carry position 0 and fall back t it — so nothing is lost by starting there, while starting in "last changed" made both separators and dragging look broken until the sort menu was found. +**A new note is put at the very top, and the list is renumbered to say so.** +Left to the tie-break it landed near the top by age, which is not the same thing: +a separator's position counts the rows above it, so a row appearing without a +renumber left every line one row too high, and the arrangement came apart a +little more with each new note. `placeOnTop` builds the order from the list +*without* the new note — measuring the room after the furniture has moved would +write the misreading back as the truth — and puts the note in front of it. Every +other row keeps its place, and the rows that had never been dragged stop sliding +about, because now they carry a position of their own. + The views are different: "All Notes", "Starred" and the bin are questions asked of every notebook at once, so there is nothing for an arrangement to belong to and nowhere to keep it. They start in "last changed" and are not offered the