diff --git a/CHANGELOG.md b/CHANGELOG.md index 633d414..4a18662 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,46 @@ not from the output of `docker push`. --- +## 2026-08-17 — A chapter is a list too, and deleting stops moving things + +**Images** — web `sha256:PENDING`, server `sha256:PENDING` + +No backend source changed this round. + +**Fixed** +- **Deleting a note moved everything else.** A row leaving is the same problem as + a row arriving, in reverse: a line's position counts the rows above it, so a + note deleted above a line left the line counting rows that were no longer + there, and everything under it appeared to jump up past it. Every place a row + leaves a list now closes the gap — the bin, a move to another notebook or + chapter, and a chapter being removed. Coming back from the bin, or arriving by + a move, lands at the top like anything else arriving; an old position belongs + to an arrangement that has moved on. +- **A line could not be dragged under another line.** Two lines in the same gap + carry the same position, and the comparator said "a separator comes first" + without asking what the other one was — claiming both directions at once for + that case. A comparator that contradicts itself has no defined result, so the + sort returned them in whichever order it happened to walk, and the line sprang + back above every time. They keep the order they are written in now. +- **Removing a chapter is two moves, and only one was being made.** Its row + leaves the count, *and* the notes it held arrive in the notebook — at the top, + in the order they had inside it, since their old positions were numbered + against a list that no longer exists. + +**Changed** +- **A chapter is always in the user's own order, and can be arranged like any + other list.** Its notes wear handles under the *same* switch as the list beside + them — two switches for two lists looked at together is one too many — and are + dragged among themselves, strips at both ends included. The two lists keep out + of each other's way by scope: a chapter unfolded under its row holds rows + inside a row of the notebook's list, so "the nearest row above the pointer" + cannot say which list a drag belongs to. +- **The sort menu is gone inside a chapter.** The mode belongs to the notebook + and governs the notebook's own list; a chapter is a place someone filled by + hand, and a menu there would offer a choice that cannot be honoured. + +--- + ## 2026-08-17 — One direction to drop in, and a new note that stays on top **Images** — web `sha256:b8bc284a…`, server `sha256:a66cc8eb…` diff --git a/e2e/chapters.mjs b/e2e/chapters.mjs index 1659c02..78ad29f 100644 --- a/e2e/chapters.mjs +++ b/e2e/chapters.mjs @@ -282,12 +282,17 @@ try { /* `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") => + const holdOver = (dragId, chapterId, where = "body", scope = "") => p.evaluate( - ([from, to, part]) => { - const row = document.querySelector(`[data-reorder-id="${from}"]`); + ([from, to, part, sc]) => { + /* Both ends looked up in one list. The two ends of a list have strips + with the same names in every list on screen, so a bare id would find + the notebook's when the chapter's was meant. */ + const find = (id) => + document.querySelector(`[data-reorder-id="${id}"][data-reorder-scope="${sc}"]`); + const row = find(from); const grip = row?.querySelector("[data-reorder-grip]"); - const target = document.querySelector(`[data-reorder-id="${to}"]`); + const target = find(to); if (!grip || !target) return `missing ${!!grip} ${!!target}`; const head = target.querySelector("[data-reorder-head]") || target; const box = head.getBoundingClientRect(); @@ -312,7 +317,7 @@ try { fire(window, "pointermove", window.__drop.x, window.__drop.y); return "held"; }, - [dragId, chapterId, where] + [dragId, chapterId, where, scope] ); const letGo = () => @@ -359,8 +364,17 @@ try { const looseBefore = (await titles()).length; /* Both ends of the drag are read off the rows on screen: what is draggable is what is rendered, and the server's idea of the notebook includes rows this - list is not showing. */ - const rowOrder = await p.locator("[data-reorder-id]").evaluateAll((els) => els.map((e) => e.dataset.reorderId)); + list is not showing. + Only the notebook's own rows: an open chapter draws its notes with handles + of their own, and those belong to a second list — dragging one of those from + here would be measuring one list against the other. */ + const notebookRows = () => + p + .locator("[data-reorder-id]") + .evaluateAll((els) => + els.filter((e) => (e.dataset.reorderScope ?? "") === "").map((e) => e.dataset.reorderId) + ); + const rowOrder = await notebookRows(); const shownChapters = await p .locator("[data-chapter-row]") .evaluateAll((els) => els.map((e) => e.getAttribute("data-chapter-row"))); @@ -391,9 +405,7 @@ try { /* A line is drawn in the notebook and dragged in, which is how a chapter is divided at all. */ - const sepRow = ( - await p.locator("[data-reorder-id]").evaluateAll((els) => els.map((e) => e.dataset.reorderId)) - ).find((id) => id && id.startsWith("sep-")); + const sepRow = (await notebookRows()).find((id) => id && id.startsWith("sep-")); if (sepRow) { await dragOnto(sepRow, shownChapters[0]); await p.waitForTimeout(2500); @@ -414,9 +426,7 @@ try { * 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( + const looseLeft = (await notebookRows()).find( (id) => id && !id.startsWith("sep-") && @@ -440,6 +450,75 @@ try { check(false, "a loose note was left to try the edge with", "none"); } + /* ── a chapter is a list of its own, and can be arranged like one ── + * Its notes are shown with handles under the same switch as the list beside + * them, and dragged among themselves. They could not be dragged at all before: + * a chapter was somewhere to put notes and not somewhere to arrange them. */ + /* The one that actually holds notes, asked of the server rather than assumed + to be the first row: which chapter comes first is an arrangement, and this + test is not about that. */ + const fullest = (await api(p, "GET", "/notes")).body + .filter((n) => n.chapter_id) + .reduce((counts, n) => counts.set(n.chapter_id, (counts.get(n.chapter_id) || 0) + 1), new Map()); + const busiest = [...fullest.entries()].sort((a, b) => b[1] - a[1])[0]?.[0]; + const openRow = p.locator(`[data-chapter-row="${busiest}"]`); + if ((await p.locator(`[data-chapter-notes="${busiest}"]`).count()) === 0) { + await openRow.click(); + await p.waitForTimeout(1600); + } + if ((await p.locator("[data-reorder-toggle]").getAttribute("data-reorder-toggle")) === "off") { + await p.locator("[data-reorder-toggle]").click(); + await p.waitForTimeout(700); + } + const inChapterRows = () => + p + .locator("[data-reorder-id]") + .evaluateAll((els) => + els + .filter((e) => (e.dataset.reorderScope ?? "").startsWith("chapter:")) + .map((e) => e.dataset.reorderId) + ); + const chapterOrder = () => + p.locator("[data-chapter-note]").evaluateAll((els) => els.map((e) => e.innerText.trim())); + const handles = await inChapterRows(); + check(handles.length > 0, "the notes inside a chapter wear handles too", JSON.stringify(handles)); + // The name of the chapter's list, which both ends of the drag are looked up in. + const chapterScope = await p + .locator("[data-reorder-id]") + .evaluateAll( + (els) => els.map((e) => e.dataset.reorderScope ?? "").find((s) => s.startsWith("chapter:")) ?? "" + ); + + const beforeInside = await chapterOrder(); + if (beforeInside.length >= 2) { + const rows = await inChapterRows(); + const last = rows.filter((id) => id !== "__start__" && id !== "__end__").pop(); + await holdOver(last, "__start__", "body", chapterScope); + await p.waitForTimeout(400); + await letGo(); + await p.waitForTimeout(2500); + const afterInside = await chapterOrder(); + check( + afterInside[0] === beforeInside[beforeInside.length - 1] && afterInside.length === beforeInside.length, + "and can be dragged among themselves, the strip at the top included", + `${beforeInside.join(",")} → ${afterInside.join(",")}` + ); + // Written, not only drawn: the chapter's order is the notebook's business + // like any other, and has to survive the trip. + await p.reload({ waitUntil: "networkidle" }); + await p.waitForTimeout(3000); + await openNotebook(); + await openRow.click(); + await p.waitForTimeout(1800); + check( + (await chapterOrder()).join(",") === afterInside.join(","), + "and the chapter comes back in the order it was left in", + (await chapterOrder()).join(",") + ); + } else { + check(false, "a chapter with two notes to arrange", beforeInside.join(",")); + } + /* ── 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 @@ -537,8 +616,17 @@ try { /* ── a chapter is not a wastebasket ── * Removing one must never remove what it held: the notes come back to the - * notebook, where they can be seen and filed again. */ - await p.locator("[data-chapter-row]").first().click({ button: "right", force: true }); + * notebook, where they can be seen and filed again. + * + * The one holding notes, named rather than taken as "the first row": a chapter + * is a row in the arrangement like any other and can be anywhere in it. */ + const holdings = (await api(p, "GET", "/notes")).body.filter((n) => n.chapter_id); + const doomed = holdings[0]?.chapter_id; + const freedTitles = holdings + .filter((n) => n.chapter_id === doomed) + .map((n) => n.title) + .sort(); + await p.locator(`[data-chapter-row="${doomed}"]`).click({ button: "right", force: true }); await p.waitForTimeout(600); await p.getByText("Remove chapter", { exact: true }).click(); await p.waitForTimeout(700); @@ -562,15 +650,15 @@ try { "the chapter is gone, and only that one", String(await p.locator("[data-chapter-row]").count()) ); - const after = (await titles()).sort().join(","); + const after = (await titles()).sort(); check( - after === "Gartenzaun,Stromrechnung,Wasserrechnung,Zählerstand", + freedTitles.every((t) => after.includes(t)), "and every note it held is back in the notebook", - after + `${freedTitles.join(",")} → ${after.join(",")}` ); const freed = (await api(p, "GET", "/notes")).body; check( - freed.every((n) => !n.chapter_id), + freed.every((n) => n.chapter_id !== doomed), "with nothing on the server still pointing at a chapter that is gone", JSON.stringify(freed.map((n) => [n.title, n.chapter_id])) ); diff --git a/e2e/organise.mjs b/e2e/organise.mjs index 5af2199..2f7368c 100644 --- a/e2e/organise.mjs +++ b/e2e/organise.mjs @@ -344,6 +344,25 @@ try { (await layout()).join(" | ") ); + /* ── and taking one away is the same promise, in reverse ── + * A line's position counts the rows above it, so deleting a note above one + * left the line counting rows that were no longer there: everything under it + * appeared to jump up past the line, which is what made deleting feel as + * though it scrambled the notebook. */ + const beforeDelete = await layout(); + await p.locator("[data-note-row]").first().click({ button: "right", force: true }); + await p.waitForTimeout(600); + await p.locator("[data-context-menu]").getByText("Move to Trash", { exact: true }).click(); + await p.waitForTimeout(700); + await p.locator("[data-confirm-dialog]").getByRole("button", { name: "Move to Trash", exact: true }).click(); + await p.waitForTimeout(2500); + const arrangedRows = await layout(); + check( + arrangedRows.join(" | ") === beforeDelete.slice(1).join(" | "), + "deleting a note takes out only that note — the lines keep the rows they were drawn above", + `${beforeDelete.join(" | ")} → ${arrangedRows.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. */ @@ -390,7 +409,7 @@ try { "and the question is gone with it, not left standing over the app" ); check( - (await titles()).join(",") === afterNew.filter((row) => !row.startsWith("──")).join(","), + (await titles()).join(",") === arrangedRows.filter((row) => !row.startsWith("──")).join(","), "with every note still there", (await titles()).join(", ") ); diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index 7fa3de9..d34a57d 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -29,7 +29,6 @@ import { ChapterNotes, NoteList } from "./features/notes/NoteList.jsx"; import { DEFAULT_NOTEBOOK_SORT, DEFAULT_SORT, - canReorder, sortModesFor, sortNotes, separatorsFor, @@ -77,6 +76,13 @@ export default function App() { * question. It used to be answered silently by "a chapter is open", so a new * note went into the chapter however far away the user had since clicked. */ const [inChapter, setInChapter] = useState(false); + /* Rearranging is a mode, and it belongs to the screen rather than to one list + on it: a chapter shown beside or under the notebook is a second list, and + one switch for both is what "I am rearranging now" means. It ends by itself + when the list is left — coming back to a notebook to find every row wearing + a handle, because it was rearranged last week, is not where anybody left + off. */ + const [reordering, setReordering] = useState(false); const [searchQuery, setSearchQuery] = useState(""); const [showNewNb, setShowNewNb] = useState(false); @@ -245,6 +251,12 @@ export default function App() { (dragId, targetId) => data.rearrange(activeFolder, dragId, targetId, listChapter), [data, activeFolder, listChapter] ); + /* And the same for the chapter shown beside or under the list, which is a + second list on the same screen with an arrangement of its own. */ + const rearrangeInChapter = useCallback( + (dragId, targetId) => data.rearrange(activeFolder, dragId, targetId, activeChapter), + [data, activeFolder, activeChapter] + ); const trashedNotes = useMemo(() => notes.filter((n) => n.trashed), [notes]); const isTrashView = activeFolder === "trash"; @@ -356,6 +368,17 @@ export default function App() { const isSearching = searchQuery.trim().length > 0; + /* Whether an arrangement is in effect, and so whether there is anything to + rearrange. A chapter always has one; the notebook's own list has one when it + is in the hand-made order. Searching suspends both — dragging against a + narrowed list would renumber notes it is hiding. */ + const listReorderable = !!activeNotebook && !isSearching && (!!listChapter || sortMode === "manual"); + const chapterReorderable = !!activeNotebook && !isSearching && !!activeChapter; + const listKey = `${activeFolder}:${listChapter ?? ""}`; + useEffect(() => { + setReordering(false); + }, [listKey, listReorderable]); + const filteredNotes = useMemo(() => { let list; if (activeFolder === "trash") list = trashedNotes; @@ -367,6 +390,13 @@ export default function App() { look like it had done nothing. */ else list = activeNotes.filter((n) => n.folderId === activeFolder && !n.chapterId); + /* Inside a chapter the order is always the user's own. + * + * The mode belongs to the notebook and governs the notebook's own list; a + * chapter is a place someone made and filled by hand, and filing a note into + * one only means something if it stays where it was put. */ + const mode = listChapter ? "manual" : sortMode; + if (searchQuery.trim()) { // A search reads every note's text, so results keep the index's relevance // order instead of being re-sorted by date. @@ -378,7 +408,7 @@ export default function App() { const q = searchQuery.toLowerCase(); return list.filter((n) => n.title.toLowerCase().includes(q)); } - return sortNotes(list, sortMode); + return sortNotes(list, mode); }, [ activeFolder, listChapter, @@ -389,11 +419,11 @@ export default function App() { sortMode, ]); - /* The notes of the chapter being looked into, in the same order they would - have in a list of their own. */ + /* The notes of the chapter being looked into, in the order they were put in. + Always the hand-made one, as in the list — see `filteredNotes`. */ const chapterNotes = useMemo( - () => (activeChapter ? sortNotes(activeNotes.filter((n) => n.chapterId === activeChapter), sortMode) : []), - [activeChapter, activeNotes, sortMode] + () => (activeChapter ? sortNotes(activeNotes.filter((n) => n.chapterId === activeChapter), "manual") : []), + [activeChapter, activeNotes] ); /* And the lines drawn inside it, which are its own — the notebook's stay in the notebook, and each chapter's stay in that chapter. */ @@ -849,9 +879,15 @@ export default function App() { const chapterContents = (compact) => ( setReordering((v) => !v)} separators={separators} onAddSeparator={addSeparator} onSeparatorContextMenu={separatorContextMenu} @@ -983,7 +1021,7 @@ export default function App() { renderChapterNotes={inlineChapters ? renderChapterNotes : null} onReorder={rearrangeNotes} onFileInto={fileIntoChapter} - listKey={`${activeFolder}:${listChapter ?? ""}`} + listKey={listKey} isCurrent={showsWhere ? !inChapter : null} onFocusList={showsWhere ? () => setInChapter(false) : undefined} newNotePlace={ diff --git a/frontend/src/features/notes/NoteList.jsx b/frontend/src/features/notes/NoteList.jsx index 74233d7..b779b1d 100644 --- a/frontend/src/features/notes/NoteList.jsx +++ b/frontend/src/features/notes/NoteList.jsx @@ -216,36 +216,67 @@ function ChapterRow({ T, chapter, open, onOpen, onContextMenu }) { */ export function ChapterNotes({ T, + chapterId, notes, separators, reorderable, + moving = false, + onReorder, activeNote, onSeparatorContextMenu, onSelectNote, onNoteContextMenu, compact = false, }) { - // Lines only mean something where the order is the user's own, exactly as in - // the notebook's list — in "newest first" the notes slide past them. + // A chapter is always in the order it was filled in, so its lines always mean + // something and its rows can always be dragged. const entries = reorderable ? mergeEntries(notes, separators) : notes.map(asNoteEntry); + /* Its own drag, scoped to this chapter. + * + * Unfolded under its row, these rows sit *inside* a row of the notebook's + * list, so "the nearest row above the pointer" cannot say which of the two + * lists is being dragged in. The scope says it. */ + const dnd = useReorderDrag( + !!moving && !!onReorder, + onReorder || (() => {}), + undefined, + `chapter:${chapterId ?? ""}` + ); return ( <> {entries.length === 0 && (
Nothing in here yet.
)} + {moving && entries.length > 0 && ( +
+ {dnd.overId === START_ID && } +
+ )} {entries.map((entry) => entry.kind === "separator" ? ( - + {...(moving ? dnd.rowProps(entry.id) : {})} + style={{ opacity: dnd.draggingId === entry.id ? 0.45 : 1 }} + > + : null} + /> + {moving && dnd.overId === entry.id && } + ) : ( - + + {moving && dnd.overId === entry.id && } + ) )} - {/* No button to add one here. A line is drawn in the notebook and dragged + {moving && entries.length > 0 && ( +
+ {dnd.overId === END_ID && } +
+ )} + {/* No button to add a line here. One is drawn in the notebook and dragged into the chapter, the same way a note is put in one — a second way to make the same thing, in a second place, is a second thing to keep straight. */} @@ -518,6 +557,10 @@ export function NoteList({ query, onQueryChange, reorderable, + /** Whether rearranging is switched on. Held above this list, because a chapter + shown beside it is a second list under the same switch. */ + reordering, + onToggleReorder, separators, onAddSeparator, onSeparatorContextMenu, @@ -546,16 +589,7 @@ export function NoteList({ onImportMarkdown, onEmptyTrash, }) { - /* Rearranging is a mode, switched on for as long as it is being done. - * - * It ends by itself when the list is left: coming back to a notebook to find - * every row wearing a handle, because it was rearranged last week, is not - * where anybody left off. */ - const [reordering, setReordering] = useState(false); - // Off again on the way to another list, and whenever a hand-made arrangement - // stops being in effect at all — an order the app chose cannot be dragged. - useEffect(() => setReordering(false), [listKey, reorderable]); - const moving = !!reorderable && reordering; + const moving = !!reorderable && !!reordering; /* Reordering by pointer rather than by the HTML5 drag API, which mobile browsers never fire from touch — on a phone the arrangement could not be @@ -644,10 +678,13 @@ export function NoteList({
- {reorderable && ( - setReordering((v) => !v)} /> + {reorderable && } + {/* Not inside a chapter: the mode belongs to the notebook, and a + chapter is always in the order it was filled in. A menu there + would offer a choice that cannot be honoured. */} + {!isTrashView && !openChapter && ( + )} - {!isTrashView && } {!isTrashView && ( { expect(placeOnTop(notes, separators, chapters, "ghost")).toBeNull(); }); }); + +describe("two lines in the same gap", () => { + /* A separator's position counts the rows above it, so two lines with nothing + between them carry the same number. Which of them comes first is then only + the order they are written in — and the comparator used to claim *both* + directions for that case, so a line dragged under another sprang back. */ + const notes = [note("a", { sortOrder: 0 }), note("b", { sortOrder: 1 })]; + const kinds = (list: ReturnType) => list.map((e) => `${e.kind[0]}:${e.id}`); + + it("keeps the order they were written in", () => { + const two = [ + { id: "s1", label: "Erst", order: 1 }, + { id: "s2", label: "Dann", order: 1 }, + ]; + expect(kinds(mergeEntries(notes, two))).toEqual(["n:a", "s:s1", "s:s2", "n:b"]); + expect(kinds(mergeEntries(notes, [two[1]!, two[0]!]))).toEqual(["n:a", "s:s2", "s:s1", "n:b"]); + }); + + it("can be swapped by dragging one under the other", () => { + const two = [ + { id: "s1", label: "Erst", order: 1 }, + { id: "s2", label: "Dann", order: 1 }, + ]; + const next = reindexEntries(notes, two, "s1", "s2")!; + expect(next.separators.map((s) => s.id)).toEqual(["s2", "s1"]); + expect(kinds(mergeEntries(notes, next.separators))).toEqual(["n:a", "s:s2", "s:s1", "n:b"]); + }); +}); + +describe("withoutRow", () => { + /* a │ b │ ─── │ c: deleting a note above the line used to leave the line + counting rows that were no longer there, so everything under it appeared to + jump up past it. */ + const notes = [note("a", { sortOrder: 0 }), note("b", { sortOrder: 1 }), note("c", { sortOrder: 2 })]; + const separators = [{ id: "s1", label: "Teil 2", order: 2 }]; + const kinds = (list: ReturnType) => list.map((e) => `${e.kind[0]}:${e.id}`); + + it("closes the gap and takes the lines down with it", () => { + const next = withoutRow(notes, separators, [], "a")!; + expect(next.noteIds).toEqual(["b", "c"]); + expect(next.separators[0]!.order).toBe(1); + const left = [note("b", { sortOrder: 0 }), note("c", { sortOrder: 1 })]; + expect(kinds(mergeEntries(left, next.separators))).toEqual(["n:b", "s:s1", "n:c"]); + }); + + it("leaves a line that was already above the gap where it was", () => { + const next = withoutRow(notes, [{ id: "s1", label: "", order: 1 }], [], "c")!; + expect(next.separators[0]!.order).toBe(1); + }); + + it("reports nothing to do for a row that is not in the list", () => { + expect(withoutRow(notes, separators, [], "ghost")).toBeNull(); + }); +}); + +describe("placeOnTop with several notes", () => { + /* Removing a chapter frees its notes into the notebook. They arrive together, + at the top, in the order they had inside it. */ + const notes = [note("x", { sortOrder: 0 }), note("y", { sortOrder: 1 })]; + const freed = [note("p"), note("q")]; + const separators = [{ id: "s1", label: "", order: 1 }]; + + it("puts them all in front, in the order given", () => { + const next = placeOnTop([...freed, ...notes], separators, [], ["p", "q"])!; + expect(next.noteIds).toEqual(["p", "q", "x", "y"]); + expect(next.separators[0]!.order).toBe(3); + }); +}); diff --git a/frontend/src/features/notes/noteSort.ts b/frontend/src/features/notes/noteSort.ts index 221fae1..e75e2db 100644 --- a/frontend/src/features/notes/noteSort.ts +++ b/frontend/src/features/notes/noteSort.ts @@ -161,11 +161,20 @@ export function mergeEntries( separator, })); - return [...rowEntries, ...separatorEntries].sort( + return [...rowEntries, ...separatorEntries].sort((a, b) => { + if (a.order !== b.order) return a.order - b.order; + /* Two lines in the same gap keep the order they were written in. + * + * This used to say "a separator comes first" without asking what the other + * one was — so for two separators it claimed *both* directions at once. A + * comparator that contradicts itself has no defined result, and the sort + * returned them in whichever order it happened to walk: a line dragged under + * another sprang back above it, every time. */ + if (a.kind === "separator" && b.kind === "separator") return 0; // A separator sitting at the same position as a row comes first, so // "a line above this one" lands where it was asked for. - (a, b) => a.order - b.order || (a.kind === "separator" ? -1 : 1) - ); + return a.kind === "separator" ? -1 : 1; + }); } /** The result of a drag: what to write back for each kind of row. */ @@ -256,22 +265,51 @@ export function placeOnTop( notes: NoteMeta[], separators: Separator[], chapters: Chapter[], - id: string + ids: string | 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. + const wanted = Array.isArray(ids) ? ids : [ids]; + const arriving = wanted + .map((id) => notes.find((n) => n.id === id)) + .filter((n): n is NoteMeta => !!n); + if (arriving.length === 0) return null; + /* Built from the list *without* the new notes, then those 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 + * Merging them in first would be measuring the room after the furniture + * moved: a 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 held = new Set(arriving.map((n) => n.id)); const entries = mergeEntries( - notes.filter((n) => n.id !== id), + notes.filter((n) => !held.has(n.id)), separators, chapters ); - entries.unshift({ kind: "note", id, order: 0, note }); + entries.unshift(...arriving.map((note) => ({ kind: "note" as const, id: note.id, order: 0, note }))); + return toArrangement(entries); +} + +/** + * The whole list renumbered with one row taken out of it. + * + * For a note being thrown away or moved elsewhere. A row leaving is the same + * problem as a row arriving, in reverse: a separator's position counts the rows + * above it, so a note deleted above a line left that line one row too *low*, and + * everything below the line appeared to jump up past it. Nothing here moves + * except what is leaving. + * + * `notes` must still contain the row — it is the list as it is drawn, which is + * what the lines were counted against. + */ +export function withoutRow( + notes: NoteMeta[], + separators: Separator[], + chapters: Chapter[], + id: string +): Arrangement | null { + const entries = mergeEntries(notes, separators, chapters); + const at = entries.findIndex((e) => e.id === id); + if (at < 0) return null; + entries.splice(at, 1); return toArrangement(entries); } diff --git a/frontend/src/hooks/useNoteData.ts b/frontend/src/hooks/useNoteData.ts index dd75cca..321015c 100644 --- a/frontend/src/hooks/useNoteData.ts +++ b/frontend/src/hooks/useNoteData.ts @@ -3,7 +3,7 @@ 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 { placeOnTop, reindexEntries } from "../features/notes/noteSort"; +import { placeOnTop, reindexEntries, sortNotes, withoutRow } from "../features/notes/noteSort"; import type { Arrangement } from "../features/notes/noteSort"; import { extractOnImport } from "../features/pdf/pdfText"; import type { @@ -248,16 +248,6 @@ export function useNoteData(user: User | null, activeNote: string | null): NoteD setNotes((p) => p.map((n) => (n.id === id ? { ...n, starred } : n))); }, []); - const trashNote = useCallback(async (id: string) => { - await sync.trashNote(id); - setNotes((p) => p.map((n) => (n.id === id ? { ...n, trashed: true } : n))); - }, []); - - const restoreNote = useCallback(async (id: string) => { - await sync.restoreNoteLocal(id); - setNotes((p) => p.map((n) => (n.id === id ? { ...n, trashed: false } : n))); - }, []); - const permanentDeleteNote = useCallback(async (id: string) => { await sync.permanentDeleteNoteLocal(id); searchIndex.removeEntry(id); @@ -270,11 +260,6 @@ export function useNoteData(user: User | null, activeNote: string | null): NoteD setNotes((p) => p.filter((n) => !n.trashed)); }, []); - const moveNote = useCallback(async (id: string, folderId: string, chapterId: string | null = null) => { - await sync.moveNote(id, folderId, chapterId); - setNotes((p) => p.map((n) => (n.id === id ? { ...n, folderId, chapterId } : n))); - }, []); - /** * Everything one list is made of: its notes, its lines, its chapters. * @@ -392,6 +377,62 @@ export function useNoteData(user: User | null, activeNote: string | null): NoteD [applyArrangement, listOf] ); + /** + * Takes a row out of the list it is drawn in, and renumbers what is left. + * + * A row leaving is the same problem as a row arriving, in reverse. A line's + * position counts the rows above it, so deleting a note above a line left the + * line one row too low — and everything under it appeared to jump up past it. + * Called *before* the note is taken out of the store, because the list as it + * is drawn is what the lines were counted against. + */ + const takeOut = useCallback( + (note: NoteMeta | undefined) => { + if (!note?.folderId || note.trashed) return; + const chapterId = note.chapterId ?? null; + const list = listOf(note.folderId, chapterId); + if (!list) return; + const next = withoutRow(list.notes, list.mine, list.chapters, note.id); + if (!next) return; + applyArrangement(note.folderId, chapterId, list.all, list.others, next); + }, + [applyArrangement, listOf] + ); + + const trashNote = useCallback( + async (id: string) => { + takeOut(notesRef.current.find((n) => n.id === id)); + await sync.trashNote(id); + setNotes((p) => p.map((n) => (n.id === id ? { ...n, trashed: true } : n))); + }, + [takeOut] + ); + + /* Back from the bin and back to the top, like anything else arriving in a + list: its old position belongs to an arrangement that has moved on. */ + const restoreNote = useCallback( + async (id: string) => { + await sync.restoreNoteLocal(id); + setNotes((p) => p.map((n) => (n.id === id ? { ...n, trashed: false } : n))); + const note = notesRef.current.find((n) => n.id === id); + if (note) putOnTop({ ...note, trashed: false }); + }, + [putOnTop] + ); + + /* Out of one list and in at the top of the other. Both ends need saying: the + list it leaves has to close the gap, or its lines slip. */ + const moveNote = useCallback( + async (id: string, folderId: string, chapterId: string | null = null) => { + const before = notesRef.current.find((n) => n.id === id); + takeOut(before); + await sync.moveNote(id, folderId, chapterId); + setNotes((p) => p.map((n) => (n.id === id ? { ...n, folderId, chapterId } : n))); + if (before) putOnTop({ ...before, folderId, chapterId }); + }, + [putOnTop, takeOut] + ); + const createNote = useCallback( async (folderId: string, chapterId: string | null = null) => { const note = await sync.createNote(folderId, chapterId); @@ -456,13 +497,40 @@ export function useNoteData(user: User | null, activeNote: string | null): NoteD await sync.updateChapter(id, updates); }, []); + /** + * Removing a chapter: the row goes, the notes come back. + * + * Both halves have to be said in the notebook's arrangement, or the lines + * around them slip. The chapter's row is taken out of the count, and its notes + * arrive at the top in the order they had inside it — their old positions + * were numbered against the chapter's list, which no longer exists. + */ const deleteChapter = useCallback( async (id: string) => { + const chapter = chaptersRef.current.find((c) => c.id === id); + const held = notesRef.current.filter((n) => !n.trashed && n.chapterId === id); await sync.deleteChapter(id); setChapters((p) => p.filter((c) => c.id !== id)); setNotes((p) => p.map((n) => (n.chapterId === id ? { ...n, chapterId: null } : n))); + + const notebookId = chapter?.notebookId; + if (!notebookId) return; + const list = listOf(notebookId, null); + if (!list) return; + const freed = sortNotes(held, "manual").map((n) => ({ ...n, chapterId: null })); + const next = freed.length + ? placeOnTop( + [...freed, ...list.notes], + list.mine, + // Without the chapter itself: its row leaves the count as well. + list.chapters.filter((c) => c.id !== id), + freed.map((n) => n.id) + ) + : // An empty one only leaves a gap to close. + withoutRow(list.notes, list.mine, list.chapters, id); + if (next) applyArrangement(notebookId, null, list.all, list.others, next); }, - [] + [applyArrangement, listOf] ); const createNotebook = useCallback(async (fields: { name: string; cover: number }) => { diff --git a/frontend/src/hooks/useReorderDrag.ts b/frontend/src/hooks/useReorderDrag.ts index f355573..1546df5 100644 --- a/frontend/src/hooks/useReorderDrag.ts +++ b/frontend/src/hooks/useReorderDrag.ts @@ -38,6 +38,7 @@ export interface ReorderDrag { into?: boolean ) => { "data-reorder-id": string; + "data-reorder-scope": string; "data-reorder-into"?: string; onPointerDown: (e: React.PointerEvent) => void; }; @@ -45,10 +46,19 @@ export interface ReorderDrag { gripProps: { "data-reorder-grip": string }; } +/** + * `scope` names the list these rows belong to, and is what keeps two lists on + * the same screen out of each other's way: a chapter unfolded under its row + * holds rows *inside* a row of the notebook's list, so "the nearest row above + * the pointer" is not enough to say which list is being dragged in. Rows are + * looked up by scope, and a note carried in the notebook simply finds no target + * over the notes inside a chapter. + */ export function useReorderDrag( enabled: boolean, reorder: (dragId: string, targetId: string) => void, - fileInto?: (dragId: string, targetId: string) => void + fileInto?: (dragId: string, targetId: string) => void, + scope = "" ): ReorderDrag { const [overId, setOverId] = useState(null); const [intoId, setIntoId] = useState(null); @@ -73,6 +83,12 @@ export function useReorderDrag( const onPointerDown = useCallback( (id: string) => (e: React.PointerEvent) => { if (!enabled || e.button > 0) return; + /* Only presses that belong to this list. A chapter's rows sit inside a row + of the notebook's list, so its events travel up through one — and a grip + pressed in the chapter would otherwise pick up the chapter's whole row + as well. The nearest scoped row decides whose press it is. */ + const own = (e.target as HTMLElement)?.closest?.("[data-reorder-scope]") as HTMLElement | null; + if (own && own.dataset.reorderScope !== scope) return; const fromGrip = !!(e.target as HTMLElement)?.closest?.("[data-reorder-grip]"); // A finger on the row itself is a scroll or a long press, not a drag. if (e.pointerType !== "mouse" && !fromGrip) return; @@ -84,7 +100,7 @@ export function useReorderDrag( e.preventDefault(); } }, - [enabled] + [enabled, scope] ); useEffect(() => { @@ -103,7 +119,7 @@ export function useReorderDrag( // tracked with enter/leave handlers, because a finger generates no // pointerover on anything it did not press. const el = document.elementFromPoint(e.clientX, e.clientY) as HTMLElement | null; - const row = el?.closest?.("[data-reorder-id]") as HTMLElement | null; + const row = el?.closest?.(`[data-reorder-scope="${scope}"]`) as HTMLElement | null; const target = row?.dataset.reorderId ?? null; const next = target && target !== d.id ? target : null; /* A chapter is both a place and a neighbour, so which one it is depends on @@ -156,7 +172,7 @@ export function useReorderDrag( window.removeEventListener("pointerup", up); window.removeEventListener("pointercancel", stop); }; - }, [enabled, reorder, fileInto, stop]); + }, [enabled, reorder, fileInto, scope, stop]); // A drag that outlives its list would leave the page unselectable. useEffect( @@ -172,6 +188,7 @@ export function useReorderDrag( draggingId, rowProps: (id: string, into = false) => ({ "data-reorder-id": id, + "data-reorder-scope": scope, ...(into ? { "data-reorder-into": "" } : {}), onPointerDown: onPointerDown(id), }), diff --git a/readme.md b/readme.md index 711aa5c..e307275 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. +**Two lines in the same gap keep the order they were written in.** A separator's +position counts the rows above it, so two lines with nothing between them carry +the same number, and only the order they are stored in can tell them apart. The +comparator used to say "a separator comes first" without asking what the other +one was — claiming both directions at once for that case. A comparator that +contradicts itself has no defined result, and a line dragged under another sprang +back above it every time. + **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 @@ -1105,6 +1113,27 @@ 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. +**Inside a chapter the order is always the user's own.** The mode belongs to the +notebook and governs the notebook's own list; a chapter is a place someone made +and filled by hand, and filing a note into one only means something if it stays +where it was put. So a chapter has no sort menu of its own, its lines always +show, and its rows can always be dragged — with the *same* switch as the list +beside it, because two switches for two lists looked at together is one too many. +The two lists keep out of each other's way by scope: a chapter unfolded under its +row holds rows *inside* a row of the notebook's list, so "the nearest row above +the pointer" cannot say which list a drag belongs to. + +**A row leaving is the same problem as a row arriving, in reverse.** Deleting a +note above a line left the line counting rows that were no longer there, so +everything under it appeared to jump up past it — which is what made deleting +feel as though it scrambled the notebook. `withoutRow` renumbers the list the row +is leaving, and every place a row leaves one says so: the bin, a move to another +notebook or chapter, and a chapter being removed. Restoring from the bin and +moving into a list are arrivals, and arrive at the top like anything else — an +old position belongs to an arrangement that has moved on. Removing a chapter is +both at once: its row leaves the count, and the notes it held arrive at the top +in the order they had inside it. + **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