diff --git a/.project/cards/T-0201-every-filter-but-free-text-dies-on-reload-in-the-r.md b/.project/cards/T-0201-every-filter-but-free-text-dies-on-reload-in-the-r.md index c6e2f91..b25ad60 100644 --- a/.project/cards/T-0201-every-filter-but-free-text-dies-on-reload-in-the-r.md +++ b/.project/cards/T-0201-every-filter-but-free-text-dies-on-reload-in-the-r.md @@ -1,7 +1,7 @@ --- id: T-0201 title: Every filter but free text dies on reload in the record views -status: backlog +status: done type: bug priority: medium area: ui @@ -10,7 +10,12 @@ effort: S scope: [packages/workfile/ui/src] origin: [T-0195] created: 2026-08-05 -updated: 2026-08-05 +updated: 2026-08-07 +verified: + at: "2026-08-07T19:27:25.363Z" + method: local + commit: 94c9db17dcc2adea13b1e2e0f4d18e373136ee19 + digest: "sha256:f03dcd31b0af609f8e57ea210b9d8e85af6dbd9e11dbb197b4757df42bf70c1e" --- T-0195 moved the three free-text boxes off local `useState` and into the URL, the @@ -33,8 +38,17 @@ the URL one view at a time is five chances to name the parameter differently. ## Acceptance criteria -- [ ] Docs, history and memory restore every filter they offer after a reload. -- [ ] They survive a view switch and a return, the way the work views' filters do. -- [ ] The parameter names are decided in one place, next to `q` and `find`. -- [ ] Clearing a filter removes its parameter rather than leaving it empty in the URL. -- [ ] A test covers the round trip for each view. +- [x] Docs, history and memory restore every filter they offer after a reload. +- [x] They survive a view switch and a return, the way the work views' filters do. +- [x] The parameter names are decided in one place, next to `q` and `find`. +- [x] Clearing a filter removes its parameter rather than leaving it empty in the URL. +- [x] A test covers the round trip for each view. + +## Activity + +- 2026-08-07 19:09Z illodev@local#42eb42f5 · claimed +- 2026-08-07 19:27Z illodev@local#42eb42f5 · released + +## Notes + +- 2026-08-07 19:27Z illodev@local#42eb42f5 — local verification: Playwright against a live server on the repo workspace, 24 checks: Memory narrowed 55 to 32 records and kept it across a reload, a switch to Explorer and back; History 171 rows to 2 with state and visibility both restored; the docs indexed group gone with the toggle still pressed after a reload; Back walked the narrowings. The guard was mutation-proven five ways, including the parameter-name clash with the card status filter. diff --git a/.project/cards/T-0207-only-cards-can-be-read-in-sequence-every-other-kin.md b/.project/cards/T-0207-only-cards-can-be-read-in-sequence-every-other-kin.md index 7ab482d..8521de9 100644 --- a/.project/cards/T-0207-only-cards-can-be-read-in-sequence-every-other-kin.md +++ b/.project/cards/T-0207-only-cards-can-be-read-in-sequence-every-other-kin.md @@ -1,16 +1,21 @@ --- id: T-0207 title: Only cards can be read in sequence; every other kind loses its place -status: backlog +status: done type: feature priority: medium area: ui tags: [navigation] effort: S -scope: [packages/workfile/ui/src/components/RecordPanel.tsx] +scope: [packages/workfile/ui/src/components/RecordPanel.tsx, packages/workfile/ui/src/main.tsx] origin: [ADR-0017, T-0197] created: 2026-08-05 -updated: 2026-08-05 +updated: 2026-08-07 +verified: + at: "2026-08-07T19:48:43.969Z" + method: local + commit: 94c9db17dcc2adea13b1e2e0f4d18e373136ee19 + digest: "sha256:6cb976fc73786d6336d6bc3ed17ae612bab13464996c1f937b92d6b3301e73e8" --- `Inspector` carries a previous/next cursor fed by `orderedIds`, so a reader @@ -36,8 +41,24 @@ something different depending on how you arrived is worse than no next. ## Acceptance criteria -- [ ] Opening a record from a list lets the reader move to the next and previous in that list without closing the drawer. -- [ ] The order matches what the view is showing, filters and sort included. -- [ ] Opening a record from somewhere with no list shows no cursor rather than an arbitrary one. -- [ ] The control is the one `Inspector` already uses, not a second one that looks like it. -- [ ] The keyboard reaches it, and it does not capture keys the drawer already binds. +- [x] Opening a record from a list lets the reader move to the next and previous in that list without closing the drawer. +- [x] The order matches what the view is showing, filters and sort included. +- [x] Opening a record from somewhere with no list shows no cursor rather than an arbitrary one. +- [x] The control is the one `Inspector` already uses, not a second one that looks like it. +- [x] The keyboard reaches it, and it does not capture keys the drawer already binds. + +## Activity + +- 2026-08-07 19:27Z illodev@local#42eb42f5 · claimed +- 2026-08-07 19:48Z illodev@local#42eb42f5 · released + +## Notes + +- 2026-08-07 19:45Z illodev@local#42eb42f5 — Two premises corrected while doing this, both from ADR-0018 landing after the card was written. + +First, which panel serves what. The card says RecordPanel covers 'decisions, learnings, incidents, conventions and documents opened from outside the docs view'. It does not: memory records get MemoryPanel and documents get DocPanel, and RecordPanel is the fallback for changelog fragments and releases. The gap was real in all three, so the fix is a control every panel renders rather than one added to RecordPanel — which is also what the criterion asking for 'the one Inspector already uses' turns out to require. + +Second, and larger: ADR-0018 gave docs and history their own readers, so the shared drawer no longer opens for either. The cursor had to go into those two panes as well or serial reading in history — the very complaint T-0197 filed — would still be broken with the card closed. Five call sites, one shared control, and the list published by whichever view was showing it. + +And one thing the card claims that is not being delivered: Workflow gets no cursor. Its list is a force-directed layout, not an order, so next would mean whichever node the simulation happened to place — which is exactly what the card's own rule forbids. The graph opens records with no list behind it, deliberately. +- 2026-08-07 19:48Z illodev@local#42eb42f5 — local verification: Playwright against a live server on the repo workspace, 25 checks. Memory walks its lane order in the drawer and stops at the top; History walks its rail across the section break from unpublished into releases, and a state filter narrows the cursor with the list; Docs shows no cursor until a row is clicked, then steps; the workflow graph and a record opened by URL show none at all; the card cursor still walks the visible table. Keyboard: the control takes focus and steps on Enter, Escape still dismisses the drawer and Ctrl-K still opens the palette, so it binds nothing of its own. The rule is unit-tested through recordNeighbours and mutation-proven four ways, including a lookalike control added back to the inspector. diff --git a/.project/changelog/unreleased/CHG-0151-the-record-views-filters-survive-a-reload.md b/.project/changelog/unreleased/CHG-0151-the-record-views-filters-survive-a-reload.md new file mode 100644 index 0000000..a77b12c --- /dev/null +++ b/.project/changelog/unreleased/CHG-0151-the-record-views-filters-survive-a-reload.md @@ -0,0 +1,12 @@ +--- +id: CHG-0151 +title: The record views' filters survive a reload +type: fixed +area: ui +visibility: public +cards: [T-0201] +created: 2026-08-07 +updated: 2026-08-07 +--- + +Docs' managed toggle and the four chips in History and Memory now ride the address bar like the free text beside them. Narrowing Memory to open incidents, opening one and coming back keeps the narrowing instead of silently handing back the whole workspace. diff --git a/.project/changelog/unreleased/CHG-0152-every-kind-of-record-can-be-read-in-sequence-not-only-cards.md b/.project/changelog/unreleased/CHG-0152-every-kind-of-record-can-be-read-in-sequence-not-only-cards.md new file mode 100644 index 0000000..7e1580a --- /dev/null +++ b/.project/changelog/unreleased/CHG-0152-every-kind-of-record-can-be-read-in-sequence-not-only-cards.md @@ -0,0 +1,12 @@ +--- +id: CHG-0152 +title: Every kind of record can be read in sequence, not only cards +type: added +area: ui +visibility: public +cards: [T-0207] +created: 2026-08-07 +updated: 2026-08-07 +--- + +The previous/next cursor the card inspector has always carried is now one shared control, rendered by every panel that reads a record: memory, changelog fragments and releases, and the readers Docs and History own themselves. It walks the list the view was showing, in the order it was showing it, so it narrows when the filters do — and it is absent, rather than arbitrary, where there is no list behind the record: a link inside a body, a related row, the command palette, or a node of the workflow graph. diff --git a/.project/memory/learnings/LRN-0033-a-card-outlives-the-decision-it-was-filed-under-and-its-mechanics-go-s.md b/.project/memory/learnings/LRN-0033-a-card-outlives-the-decision-it-was-filed-under-and-its-mechanics-go-s.md new file mode 100644 index 0000000..1abacf7 --- /dev/null +++ b/.project/memory/learnings/LRN-0033-a-card-outlives-the-decision-it-was-filed-under-and-its-mechanics-go-s.md @@ -0,0 +1,20 @@ +--- +id: LRN-0033 +title: A card outlives the decision it was filed under, and its mechanics go stale in silence +status: active +confidence: high +related: [ADR-0018, LRN-0027, T-0207] +tags: [ui, process] +created: 2026-08-07 +updated: 2026-08-07 +--- + +T-0207 was filed as ADR-0017's surviving finding. ADR-0018 then superseded ADR-0017 and moved the layout: docs and history became views that own their readers, so the shared drawer stopped opening for either. The card was never touched, and it did not have to be wrong to be misleading — every sentence in it had been true when written. + +What it still said was that `RecordPanel` serves "decisions, learnings, incidents, conventions and documents opened from outside the docs view". It serves none of those: memory records get `MemoryPanel`, documents get `DocPanel`, and `RecordPanel` is the fallback for changelog fragments and releases. So the card's scope named one file, `RecordPanel.tsx`, and a fix that touched only that file would have satisfied the card, passed review, and left serial reading in history — the complaint the whole chain started from — exactly as broken as it was. + +The superseding record even flagged this. ADR-0018 has a "What survives from ADR-0017" section naming T-0207 and saying it "stands on its own". It does stand, but it stands on the old geometry: what survived was the *finding*, not the mechanics the card wrote down around it. + +**Why:** a card is a snapshot of an understanding, and a decision record is the thing that changes understandings. Superseding an ADR updates the decision graph and leaves the cards that cite it describing a codebase that has moved. Nothing reports that, because nothing is broken: the links resolve, `doctor` is clean, and the prose is internally consistent. + +**How to apply:** when a card's `origin` or `related` names a decision that is now `superseded`, re-read the card against the superseding one before starting — the finding usually survives and the mechanics usually do not. Treat its `scope` as the weakest part: it was written from the old arrangement and it is what silently narrows the fix. Then do what [[LRN-0027]] says and open the view, which is the only thing that settles which panel is actually on screen. Correct the premise on the card rather than quietly working around it, so the next reader inherits the correction instead of the snapshot. diff --git a/packages/workfile/docs/ui.md b/packages/workfile/docs/ui.md index 0c6f60a..61cd8c1 100644 --- a/packages/workfile/docs/ui.md +++ b/packages/workfile/docs/ui.md @@ -142,6 +142,33 @@ file is checked. rides the address bar like every other filter (`?q=` for cards, `?find=` for docs, history and memory). `test/filter-search.test.ts` fails if a view grows a box of its own or a wording of its own. +- **A filter that is not in the URL is a filter that dies on reload.** Every + one of them is state the shell owns and `ui/src/query.ts` serialises — the + card axes flat (`?status=`, `?area=`, …), the record collections' axes + namespaced by view (`?docs-managed=1`, `?history-state=`, + `?memory-collection=`, `?memory-status=`). The prefix is a rule and not a + case-by-case choice: the obvious name for Memory's is `status`, which the + card filter already owns, and the loser of a clash like that filters by + nothing without saying so. A record view therefore takes its filters as a + prop and reports changes as a patch, so its coupled pairs — picking a Memory + collection clears the status that belonged to it — reach the address bar in + one write. Same suite: it fails on a view that takes one back into a + `useState`, and on a parameter that collides with a card axis. +- **A record opened from a list can be read as a sequence.** Every panel that + reads a record — the card inspector, the memory panel, the generic record + panel, and the readers Docs and History own themselves — renders + `ui/src/record-cursor.tsx`, and the rule for where previous and next go is + `recordNeighbours` in `navigation.ts`, beside the other navigation rules. The + list is whatever the view was showing, in the order it was showing it, so it + narrows when the filters do; each view publishes its own as the second + argument to `onSelect`. **Absent, not guessed, where there is no list:** a + `[[LRN-0004]]` in a body, a `related` row, the command palette, and a node of + the Workflow graph all open a record with nothing behind it, and a force + layout is not an order. At the ends of a real list the control renders with + one half disabled, which is how a reader tells "no next" from "there was + never a sequence here". It is a context rather than a prop for the reason + `read-only.tsx` gives: the panels sit in three different places, and all + three have to reach it. - **The filter bar is one container, and it decides what may scroll away.** `ui/src/components/FilterBar.tsx` owns the whole bar in every view that has one — the shell, Docs, History, Memory, Workflow and the Gantt toolbar — and diff --git a/packages/workfile/test/filter-search.test.ts b/packages/workfile/test/filter-search.test.ts index 27a5f6a..5e81354 100644 --- a/packages/workfile/test/filter-search.test.ts +++ b/packages/workfile/test/filter-search.test.ts @@ -35,7 +35,47 @@ const suite = canLoadTypeScript const uiRoot = new URL("../ui/src/", import.meta.url); -const read = (path: string) => readFile(new URL(path, uiRoot), "utf8"); +// Normalised in the reader, not per assertion, per LRN-0026: a regex here that +// anchors on `\n` matches nothing on a Windows checkout and reports the code it +// was looking at as missing. The next regex added to this file inherits the fix, +// which is the point — its author will be on a machine where the bug cannot +// reproduce. +const read = async (path: string) => + (await readFile(new URL(path, uiRoot), "utf8")).replaceAll("\r\n", "\n"); + +/** + * The address bar, stubbed, and every URL written through it. + * + * `query.ts` reads `location.search` and writes through `history`, so a round + * trip needs both. Shared by the suites below rather than set up twice: two + * copies of a stub is two chances for one of them to drift into testing a + * browser neither suite is running in. + */ +function addressBar() { + const calls: Array<[string, string]> = []; + const wrote = (kind: string, url: string) => { + calls.push([kind, url]); + globalThis.location.search = url.includes("?") + ? url.slice(url.indexOf("?")) + : ""; + }; + globalThis.location = { pathname: "/", search: "" }; + globalThis.history = { + pushState: (_state, _title, url) => wrote("push", url), + replaceState: (_state, _title, url) => wrote("replace", url) + }; + return { + calls, + /** The parameter names the last write put in the URL, in order. */ + names: () => [ + ...new URLSearchParams(globalThis.location.search).keys() + ], + restore: () => { + delete globalThis.location; + delete globalThis.history; + } + }; +} /** Every ui/src source file, path → content. */ async function sources() { @@ -74,7 +114,6 @@ const RECORD_VIEWS = [ */ suite("free text round-trips through the URL beside the card query", async () => { const query = await import("../ui/src/query.ts"); - const calls: Array<[string, string]> = []; const filters = { search: "", status: "", @@ -85,18 +124,8 @@ suite("free text round-trips through the URL beside the card query", async () => showIdeas: false, showClosed: false }; - const wrote = (kind: string, url: string) => { - calls.push([kind, url]); - globalThis.location.search = url.includes("?") - ? url.slice(url.indexOf("?")) - : ""; - }; - - globalThis.location = { pathname: "/", search: "" }; - globalThis.history = { - pushState: (_state, _title, url) => wrote("push", url), - replaceState: (_state, _title, url) => wrote("replace", url) - }; + const bar = addressBar(); + const { calls } = bar; try { // Absent when empty, so every URL the app wrote before this parameter @@ -139,11 +168,221 @@ suite("free text round-trips through the URL beside the card query", async () => assert.deepEqual(calls.at(-1), ["replace", "/?view=docs"]); assert.equal(query.readUrlState().recordSearch, ""); } finally { - delete globalThis.location; - delete globalThis.history; + bar.restore(); + } +}); + +/** The card filters, all empty, which is the base every write below starts from. */ +const NO_CARD_FILTERS = { + search: "", + status: "", + area: "", + type: "", + priority: "", + milestone: "", + showIdeas: false, + showClosed: false +}; + +/** + * And the axis filters beside the free text, which is the rest of the same job. + * + * T-0195 moved the three boxes into the URL and left the five chips where they + * were: Docs' `managed` toggle, History's state and visibility, Memory's + * collection and status. Narrowing Memory to open incidents, opening one to read + * it and coming back handed you every record in the workspace, with nothing in + * the interface saying the narrowing had ever been there. + * + * The names are the part worth pinning. Every one of them is prefixed by its + * view, and the reason is that the obvious name for Memory's is `status`, which + * the card filter already owns — a clash `readUrlState` would not report but + * quietly resolve, because it validates the card `status` against `STATUSES` and + * answers `""` for anything that is not one. So the loser of the clash filters + * by nothing, in silence. The disjointness assertion below is that trap, and it + * derives both sets from what `writeUrlState` actually emits rather than + * restating them, so a new axis on either side is covered the day it is added. + */ +suite("every record filter round-trips, in a namespace of its own", async () => { + const query = await import("../ui/src/query.ts"); + const bar = addressBar(); + const { calls } = bar; + const everyRecordFilter = { + docs: { managedOnly: true }, + history: { state: "unreleased", visibility: "internal" }, + memory: { collection: "incidents", status: "open" } + }; + + try { + // Absent when nothing is narrowed, so every URL the app wrote before + // these parameters existed is the URL it writes now. + query.writeUrlState("memory", NO_CARD_FILTERS, null, { + recordFilters: query.NO_RECORD_FILTERS + }); + assert.deepEqual(calls.at(-1), ["replace", "/?view=memory"]); + // And omitting the bag entirely is the same as passing an empty one: + // the views with no record filters to reflect pass neither it nor `find`. + query.writeUrlState("memory", NO_CARD_FILTERS, null); + assert.deepEqual(calls.at(-1), ["replace", "/?view=memory"]); + + query.writeUrlState("docs", NO_CARD_FILTERS, null, { + recordFilters: everyRecordFilter + }); + const recordNames = bar.names().filter((name) => name !== "view"); + + query.writeUrlState( + "explorer", + { + search: "T-0201", + status: "doing", + area: "ui", + type: "bug", + priority: "medium", + milestone: "0.9.0", + showIdeas: true, + showClosed: true + }, + null, + { find: "filters" } + ); + const cardNames = bar.names().filter((name) => name !== "view"); + + // Checked before the names themselves, so a clash is reported as a clash + // rather than as the renamed parameter it also is. + const clashes = recordNames.filter((name) => cardNames.includes(name)); + assert.deepEqual( + clashes, + [], + "claimed by both a card filter and a record filter, so one of them " + + `will silently filter by nothing: ${clashes.join(", ")}` + ); + assert.deepEqual( + recordNames, + [ + "docs-managed", + "history-state", + "history-visibility", + "memory-collection", + "memory-status" + ], + "a record filter is missing from the URL, or its parameter was renamed" + ); + + // Each view's round trip, one at a time, so a failure names the view. + for (const [view, narrowed] of [ + ["docs", { docs: everyRecordFilter.docs }], + ["history", { history: everyRecordFilter.history }], + ["memory", { memory: everyRecordFilter.memory }] + ] as const) { + const recordFilters = { ...query.NO_RECORD_FILTERS, ...narrowed }; + query.writeUrlState(view, NO_CARD_FILTERS, null, { recordFilters }); + assert.deepEqual( + query.readUrlState().recordFilters, + recordFilters, + `${view} does not restore its filters from the address bar` + ); + } + + // A view switch is what actually broke: the filters ride the URL whatever + // the current view is, so leaving Memory for a card and coming back finds + // the narrowing still there rather than silently widened. + query.writeUrlState("explorer", NO_CARD_FILTERS, "T-0201", { + recordFilters: everyRecordFilter + }); + assert.deepEqual( + query.readUrlState().recordFilters, + everyRecordFilter, + "the record filters do not survive a view that does not render them" + ); + + // Clearing one takes its parameter out rather than leaving an empty + // `memory-status=` behind for the next reader to wonder about. + query.writeUrlState("memory", NO_CARD_FILTERS, null, { + recordFilters: { + ...query.NO_RECORD_FILTERS, + memory: { collection: "incidents", status: "" } + } + }); + assert.deepEqual(calls.at(-1), [ + "replace", + "/?view=memory&memory-collection=incidents" + ]); + assert.equal(query.readUrlState().recordFilters.memory.status, ""); + + // The card filters and these are two bags, and neither reaches into the + // other: dropping a record axis into `Filters` would put it behind the + // work strip's reset chip, in a view that never renders it. + const restored = query.readUrlState(); + for (const key of ["docs", "history", "memory"]) { + assert.ok( + !(key in restored.filters), + `${key} filters are reachable as a card filter` + ); + } + } finally { + bar.restore(); } }); +/** + * And the shell owns all five, which is what makes the round trip above reach + * the interface at all. + * + * A view that keeps its own `useState` for one of these serialises nothing: the + * URL would carry a value the chip never reads. The pair shape is the tell — + * `const [collection, setCollection] = useState("")` — so it is what this looks + * for, in the three views that used to have five of them between them. + */ +suite("no record view owns its own axis filter", async () => { + const all = await sources(); + const owners: Array<[string, string[], string]> = [ + ["components/Docs.tsx", ["managedOnly"], "DocsFilters"], + ["components/History.tsx", ["state", "visibility"], "HistoryFilters"], + ["components/Memory.tsx", ["collection", "status"], "MemoryFilters"] + ]; + + for (const [path, fields, type] of owners) { + const source = all.get(path); + assert.ok(source, `${path} is gone`); + for (const field of fields) { + assert.doesNotMatch( + source, + new RegExp(`\\[\\s*${field}\\s*,\\s*set[A-Z]\\w*\\s*\\]`), + `${path} holds ${field} in local state, so it dies on reload` + ); + } + assert.match( + source, + new RegExp(`filters: ${type};`), + `${path} does not take its filters from the shell` + ); + assert.match( + source, + /onFiltersChange: \(patch: Partial\(/); + for (const view of ["docs", "history", "memory"]) { + assert.match( + shell, + new RegExp(`filters=\\{recordFilters\\.${view}\\}`), + `main.tsx does not pass ${view} its filters` + ); + } + assert.match(shell, /writeUrlState\([\s\S]{0,200}recordFilters/); + // Both directions. Writing without restoring on `popstate` means Back walks + // the address bar over a narrowing the chips never come off. + assert.match( + shell, + /setRecordFilters\(next\.recordFilters\)/, + "main.tsx does not restore the record filters when the user goes Back" + ); +}); + /** * One control, and one sentence per corpus saying what it matches. * diff --git a/packages/workfile/test/navigation.test.ts b/packages/workfile/test/navigation.test.ts index 1a85887..74668cc 100644 --- a/packages/workfile/test/navigation.test.ts +++ b/packages/workfile/test/navigation.test.ts @@ -6,6 +6,7 @@ import { CARD_VIEWS, drawerCovers, recordCollection, + recordNeighbours, viewForRecord } from "../ui/src/navigation.ts"; import type { View } from "../ui/src/types.ts"; @@ -183,3 +184,122 @@ test("a view that already renders the selection is not covered by the drawer", ( assert.equal(drawerCovers("docs", "changelog"), true); assert.equal(drawerCovers("explorer", "changelog"), true); }); + +/** + * The reading cursor, which every kind now has and only cards used to. + * + * `Inspector` has carried previous/next for cards since the rail became a + * drawer. Reading three changelog fragments in a row meant dismissing the + * reader, finding your place in the list, and clicking again — T-0207, and the + * one finding of ADR-0017 that survived being superseded. + */ +test("the reading cursor is absent where there is no list, not guessed", () => { + const list = ["CHG-0151", "REL-0021", "CHG-0149"]; + + // No list at all: a `[[LRN-0004]]` in a card body, a `related` row, the + // command palette, a node of the Workflow graph. + assert.deepEqual(recordNeighbours([], "CHG-0151"), { + previousId: null, + nextId: null + }); + // A record the list does not hold, which is the same thing: the reader + // followed a link out of it. + assert.deepEqual(recordNeighbours(list, "ADR-0018"), { + previousId: null, + nextId: null + }); + // And nothing selected, which is the docs view on a wide screen before the + // reader has picked anything: it shows the first document as a fallback, + // and a fallback is not a place in a sequence. + assert.deepEqual(recordNeighbours(list, null), { + previousId: null, + nextId: null + }); + // A list of one has nowhere to step, so the control is absent rather than + // present with both halves dead. + assert.deepEqual(recordNeighbours(["CHG-0151"], "CHG-0151"), { + previousId: null, + nextId: null + }); +}); + +test("the reading cursor walks the list it was given, and stops at both ends", () => { + const list = ["CHG-0151", "REL-0021", "CHG-0149"]; + + // One end, the middle, the other end. At an end one side is null and the + // other is not, which renders one disabled button — that is how a reader + // tells "no next" from "there was never a sequence here". + assert.deepEqual(recordNeighbours(list, "CHG-0151"), { + previousId: null, + nextId: "REL-0021" + }); + assert.deepEqual(recordNeighbours(list, "REL-0021"), { + previousId: "CHG-0151", + nextId: "CHG-0149" + }); + assert.deepEqual(recordNeighbours(list, "CHG-0149"), { + previousId: "REL-0021", + nextId: null + }); + + // The order is the caller's, never re-sorted here. History's rail draws + // unpublished, then releases, then published fragments, and a cursor that + // walked the ids in any other order would disagree with the column beside + // it. + assert.equal(recordNeighbours(list, "REL-0021").nextId, "CHG-0149"); +}); + +/** + * One control, in every panel that reads a record. + * + * The card inspector had its own previous/next pair inline. Copying it into the + * memory panel, the record panel, and the readers Docs and History own would + * have been four more chances for one of them to disagree — about where the + * control sits, whether it disables or vanishes at the ends, or what its + * accessible name is. So the pair moved out and each panel renders it. + */ +test("every panel that reads a record renders the same cursor", async () => { + const read = (name: string) => + readFile(new URL(`../ui/src/${name}`, import.meta.url), "utf8").then( + (source) => source.replaceAll("\r\n", "\n") + ); + + const panels = [ + "components/Inspector.tsx", + "components/RecordPanel.tsx", + "components/Memory.tsx", + "components/Docs.tsx", + "components/History.tsx" + ]; + for (const panel of panels) { + const source = await read(panel); + assert.match( + source, + /import \{ RecordCursor \} from "\.\.\/record-cursor"/, + `${panel} does not use the shared cursor` + ); + assert.match(source, / void; + // The second argument is the list the click came from, in display order, + // which is what the reader's previous/next cursor walks (T-0207). + onSelect: (id: string | null, orderedIds?: string[]) => void; onOpenCard: (id: string) => void; // Owned by the shell, shared with history and memory, and serialised to // the address bar: the local state this replaced died on every reload. search: string; onSearchChange: (value: string) => void; + // Owned by the shell for the same reason, and by the same route (T-0201). + // This one was left behind when the free text moved. + filters: DocsFilters; + onFiltersChange: (patch: Partial) => void; }) { const readOnly = useReadOnly(); const [documents, setDocuments] = useState([]); const [loading, setLoading] = useState(true); const [error, setError] = useState(""); - const [managedOnly, setManagedOnly] = useState(false); + const { managedOnly } = filters; // Only managed documents can be edited: an indexed one is read-only // through the protocol by definition, and offering an editor for it would // promise something the server will refuse. @@ -486,6 +500,16 @@ export function DocsView({ { key: "indexed", label: "indexed · read only", docs: indexed } ].filter((group) => group.docs.length > 0); }, [visible]); + /** + * The list the reader's previous/next cursor walks, in the order the rail + * draws it: managed first, then indexed, both after the search and the + * toggle. Derived from `groups` rather than from `documents` on purpose — a + * cursor that steps onto a document the list is not showing has jumped. + */ + const order = useMemo( + () => groups.flatMap((group) => group.docs.map((doc) => doc.id)), + [groups] + ); // Below `lg` the list and the reader are one pane at a time, so a default // selection would open a document over the list every visit and leave the @@ -676,7 +700,9 @@ export function DocsView({ on={managedOnly} onLabel="only" offLabel="all" - onChange={setManagedOnly} + onChange={(next) => + onFiltersChange({ managedOnly: next }) + } />
onSelect(document.id)} + onSelect={() => + onSelect(document.id, order) + } /> ))}
@@ -808,6 +836,11 @@ export function DocsView({ ) : null} + {/* This view owns its reader rather than raising + the drawer (ADR-0018), so the cursor has to be + here — the drawer's copy never opens over docs. + T-0207. */} +

{active.title} diff --git a/packages/workfile/ui/src/components/History.tsx b/packages/workfile/ui/src/components/History.tsx index fdc926b..b26f4bd 100644 --- a/packages/workfile/ui/src/components/History.tsx +++ b/packages/workfile/ui/src/components/History.tsx @@ -34,10 +34,12 @@ import { cn } from "@/lib/utils"; import { api } from "../api"; import { READING_MEASURE } from "../layout"; import { READ_ONLY_HINT, useReadOnly } from "../read-only"; +import { RecordCursor } from "../record-cursor"; import { changeTouches, useWorkspaceChanges } from "../store/live"; import { recordStatusColor, severityColor, statusColor } from "../theme"; import type { ChangeRecord, + HistoryFilters, HistoryRecord, RecordLink, ReleasePreview, @@ -733,10 +735,14 @@ export function HistoryView({ schema, areas, search, - onSearchChange + onSearchChange, + filters, + onFiltersChange }: { selectedId: string | null; - onSelect: (id: string) => void; + // The second argument is the list the click came from, in display order, + // which is what the reader's previous/next cursor walks (T-0207). + onSelect: (id: string, orderedIds?: string[]) => void; onOpenRecord: (id: string) => void; schema: ChangelogSchema; areas: string[]; @@ -744,11 +750,15 @@ export function HistoryView({ // address bar: the local state this replaced died on every reload. search: string; onSearchChange: (value: string) => void; + // These two by the same route and for the same reason (T-0201). They were + // left behind when the free text moved, so a reader who narrowed to + // unreleased fragments, opened one and came back got the whole history. + filters: HistoryFilters; + onFiltersChange: (patch: Partial) => void; }) { const readOnly = useReadOnly(); const [records, setRecords] = useState([]); - const [state, setState] = useState(""); - const [visibility, setVisibility] = useState(""); + const { state, visibility } = filters; const [loading, setLoading] = useState(true); const [error, setError] = useState(""); const [actionError, setActionError] = useState(""); @@ -870,6 +880,19 @@ export function HistoryView({ () => nextVersionHint(releases, unpublished), [releases, unpublished] ); + /** + * The list the reader's previous/next cursor walks, in the order the rail + * draws it: unpublished, then releases, then published fragments. Not + * `sorted`, which is the same records in a different order — a cursor that + * disagrees with the column beside it is worse than none (T-0207). + */ + const order = useMemo( + () => + [...unpublished, ...releases, ...published].map( + (record) => record.id + ), + [published, releases, unpublished] + ); const active = selectedId ? recordById.get(selectedId) : undefined; const openRelation = (id: string) => { @@ -880,8 +903,7 @@ export function HistoryView({ // A fragment shipped in an older release falls outside the active // filters; widen them instead of dead-ending on a missing record. if (/^(CHG|REL)-/.test(id)) { - setState(""); - setVisibility(""); + onFiltersChange({ state: "", visibility: "" }); onSelect(id); return; } @@ -974,7 +996,9 @@ export function HistoryView({ { value: "unreleased" }, { value: "released" } ]} - onChange={setState} + onChange={(next) => + onFiltersChange({ state: next }) + } /> ({ value }))} - onChange={setVisibility} + onChange={(next) => + onFiltersChange({ visibility: next }) + } /> @@ -1017,19 +1043,19 @@ export function HistoryView({ label="unpublished" records={unpublished} selectedId={selectedId} - onSelect={onSelect} + onSelect={(id) => onSelect(id, order)} /> onSelect(id, order)} /> onSelect(id, order)} /> )} @@ -1156,6 +1182,19 @@ export function HistoryView({ they used to crowd the row and push the close control off the right edge. */} + {/* First in the group, because it is the one + that gets used repeatedly: reading the + fragments of a release in order is what + T-0197 was complaining about, and this view + owns its reader (ADR-0018) so the drawer's + copy of this control never opens here. */} + {newFragmentButton} - - - - + {/* The cursor this panel used to own outright. It moved to + `record-cursor.tsx` unchanged so every other kind could have + the same one rather than a second control that looks like it + — T-0207. The list it moves along now arrives by context, + which is why `orderedIds` is gone from here. */} + {record.status} + {/* Right end of the identity bar, which is where the card + inspector puts the same control. Absent unless the reader + arrived from a list — T-0207. */} + +
@@ -1195,21 +1202,29 @@ export function MemoryView({ onOpenRecord, schema, search, - onSearchChange + onSearchChange, + filters, + onFiltersChange }: { selectedId: string | null; - onSelect: (id: string) => void; + // The second argument is the list the click came from, in display order, + // which is what the reader's previous/next cursor walks (T-0207). + onSelect: (id: string, orderedIds?: string[]) => void; onOpenRecord: (id: string) => void; schema: RuntimeSchema["memory"]; // Owned by the shell, shared with docs and history, and serialised to the // address bar: the local state this replaced died on every reload. search: string; onSearchChange: (value: string) => void; + // These two by the same route and for the same reason (T-0201). Narrowing + // to open incidents, opening one and coming back used to hand back every + // record in the workspace, with nothing saying the narrowing had been there. + filters: MemoryFilters; + onFiltersChange: (patch: Partial) => void; }) { const readOnly = useReadOnly(); const [records, setRecords] = useState([]); - const [collection, setCollection] = useState(""); - const [status, setStatus] = useState(""); + const { collection, status } = filters; const [loading, setLoading] = useState(true); const [error, setError] = useState(""); const [createFor, setCreateFor] = useState(null); @@ -1218,9 +1233,13 @@ export function MemoryView({ // tile would otherwise dismiss the drawer the click just retargeted. const lastSelectRef = useRef(0); const selectRecord = useCallback( - (id: string) => { + // `orderedIds` is omitted where the caller is not a lane tile — a + // relation row inside the panel, say — so the cursor goes absent + // rather than claiming the lanes as context for something reached + // from outside them. + (id: string, orderedIds?: string[]) => { lastSelectRef.current = performance.now(); - onSelect(id); + onSelect(id, orderedIds); }, [onSelect] ); @@ -1288,6 +1307,18 @@ export function MemoryView({ return known; }, [schema.collections, sorted, collection]); + /** + * The list the reader's previous/next cursor walks: lane by lane, each lane + * top to bottom, which is how the columns are read. Derived from `lanes` + * rather than from `sorted` so the collection filter narrows it too — a + * cursor that steps into a lane the reader has filtered away has jumped + * (T-0207). + */ + const order = useMemo( + () => lanes.flatMap((lane) => lane.records.map((record) => record.id)), + [lanes] + ); + const active = sorted.find((record) => record.id === selectedId); const statuses = collection ? collectionStatuses(schema.collections, collection) @@ -1341,10 +1372,13 @@ export function MemoryView({ options={schema.collections.map((item) => ({ value: item.id }))} - onChange={(next) => { - setCollection(next); - setStatus(""); - }} + // One patch, not two calls: the status belongs to the + // collection's vocabulary, so it clears with it, and + // clearing it separately would put a state no chip ever + // showed into the address bar on the way past. + onChange={(next) => + onFiltersChange({ collection: next, status: "" }) + } /> onFiltersChange({ status: next })} /> {error ? ( @@ -1420,7 +1454,7 @@ export function MemoryView({ record={record} selected={record.id === selectedId} onSelect={() => - selectRecord(record.id) + selectRecord(record.id, order) } /> ))} diff --git a/packages/workfile/ui/src/components/RecordPanel.tsx b/packages/workfile/ui/src/components/RecordPanel.tsx index 0dc0469..46b29a7 100644 --- a/packages/workfile/ui/src/components/RecordPanel.tsx +++ b/packages/workfile/ui/src/components/RecordPanel.tsx @@ -5,6 +5,7 @@ import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; import { api } from "../api"; +import { RecordCursor } from "../record-cursor"; import type { BaseRecord } from "../types"; import { documentOutline, MarkdownBody } from "./Markdown"; import { OutlineRail } from "./OutlineRail"; @@ -83,6 +84,9 @@ export function RecordPanel({

{record?.title ?? (error ? "Unavailable" : "…")} diff --git a/packages/workfile/ui/src/main.tsx b/packages/workfile/ui/src/main.tsx index 6fc2b68..20b37f7 100644 --- a/packages/workfile/ui/src/main.tsx +++ b/packages/workfile/ui/src/main.tsx @@ -88,6 +88,7 @@ import { activeClaims, orderClaims, overlapsByCard } from "./claims"; import { drawerCovers, recordCollection, viewForRecord } from "./navigation"; import { filterTasks, readUrlState, writeUrlState } from "./query"; import { READ_ONLY_HINT, ReadOnlyProvider } from "./read-only"; +import { RecordCursorProvider } from "./record-cursor"; import { changeTouches, useWorkspaceChanges } from "./store/live"; import { drawableCount, @@ -99,8 +100,12 @@ import { STATUSES, TYPES, type ActivitySnapshot, + type DocsFilters, type Filters, type HealthReport, + type HistoryFilters, + type MemoryFilters, + type RecordFilters, type RuntimeSchema, type Task, type TaskPatch, @@ -468,6 +473,45 @@ function App() { * it once, and try each collection with it. */ const [recordSearch, setRecordSearch] = useState(INITIAL.recordSearch); + /** + * And their axis filters, for the same reason and by the same route. + * + * T-0195 moved the free text here and left these behind, so Docs' `managed` + * toggle and the four chips in History and Memory kept dying on a reload + * while the box above them survived one. Held per view rather than pooled: + * a collection only Memory has and a visibility only History has are not one + * filter under two names, and sharing them would apply a narrowing the view + * you switched to never offered. + */ + const [recordFilters, setRecordFilters] = useState( + INITIAL.recordFilters + ); + /** + * One updater per view, each taking a patch rather than a value. + * + * A patch because Memory's two are coupled — picking a collection clears the + * status, which belongs to that collection's vocabulary — and two sequential + * setters would write the address bar twice, once through a state no chip + * ever showed. + */ + const patchDocsFilters = useCallback((patch: Partial) => { + setRecordFilters((current) => ({ + ...current, + docs: { ...current.docs, ...patch } + })); + }, []); + const patchHistoryFilters = useCallback((patch: Partial) => { + setRecordFilters((current) => ({ + ...current, + history: { ...current.history, ...patch } + })); + }, []); + const patchMemoryFilters = useCallback((patch: Partial) => { + setRecordFilters((current) => ({ + ...current, + memory: { ...current.memory, ...patch } + })); + }, []); const [showNewCard, setShowNewCard] = useState(false); const [showPalette, setShowPalette] = useState(false); /** @@ -668,14 +712,43 @@ function App() { * record-open stamps this ref; the dismiss handlers ignore anything * arriving in its shadow. */ const lastSelectRef = useRef(0); - const selectRecord = useCallback((id: string | null) => { - lastSelectRef.current = performance.now(); - setSelectedId(id); - // Any kind. This read `=== "cards"` while the drawer only held cards, - // and left behind the state where selecting a doc moved the selection - // and opened nothing. - if (id) setInspectorOpen(true); - }, []); + /** + * The list the reader is stepping through, published by the view that was + * showing it (T-0207). + * + * Only the record collections need this. Cards are read against the work + * views' `visibleTasks`, which the shell already holds, so the cursor for a + * card has never needed anyone to hand it a list. + */ + const [recordOrder, setRecordOrder] = useState([]); + const recordOrderRef = useRef([]); + recordOrderRef.current = recordOrder; + const selectRecord = useCallback( + /** + * `orderedIds` is the list the click came from, in display order. + * + * Omitting it does not always clear the cursor, and the difference is + * the point: stepping to the next record calls this with one argument, + * and so does a `[[LRN-0004]]` in a body. The first stays inside the + * list and keeps it; the second leaves it and drops it. So the rule is + * about where the new record *is*, not about who asked — which is also + * the honest answer for a `related` row that happens to point back into + * the list the reader is already walking. + */ + (id: string | null, orderedIds?: string[]) => { + lastSelectRef.current = performance.now(); + setSelectedId(id); + if (orderedIds) setRecordOrder(orderedIds); + else if (!id || !recordOrderRef.current.includes(id)) { + setRecordOrder([]); + } + // Any kind. This read `=== "cards"` while the drawer only held cards, + // and left behind the state where selecting a doc moved the selection + // and opened nothing. + if (id) setInspectorOpen(true); + }, + [] + ); useEffect(() => { document.title = projectName; @@ -688,8 +761,12 @@ function App() { const signature = `${view}|${selectedId ?? ""}`; const push = signature !== lastNavigation.current; lastNavigation.current = signature; - writeUrlState(view, filters, selectedId, { push, find: recordSearch }); - }, [filters, recordSearch, selectedId, view]); + writeUrlState(view, filters, selectedId, { + push, + find: recordSearch, + recordFilters + }); + }, [filters, recordFilters, recordSearch, selectedId, view]); useEffect(() => { const onPopState = () => { @@ -699,6 +776,7 @@ function App() { setFilters(next.filters); setSelectedId(next.selectedId); setRecordSearch(next.recordSearch); + setRecordFilters(next.recordFilters); }; window.addEventListener("popstate", onPopState); return () => window.removeEventListener("popstate", onPopState); @@ -883,6 +961,19 @@ function App() { return pairs; }, [tasks]); const selected = selectedId ? taskById.get(selectedId) : undefined; + /** + * The list the previous/next cursor moves along. + * + * Cards read against the work views' visible list, which is filtered and + * sorted here already and needs nobody to publish it; every other kind reads + * against the list the view that was showing it handed over. Memoised + * because it is a context value: a fresh array every render would re-render + * every panel that reads the cursor, on every keystroke anywhere. + */ + const cursorIds = useMemo( + () => (selected ? visibleTasks.map((task) => task.id) : recordOrder), + [recordOrder, selected, visibleTasks] + ); const milestones = useMemo( () => [ @@ -1185,6 +1276,17 @@ function App() { return ( + {/* The list the previous/next cursor moves along, wrapped around the + whole tree because the panels that render the control are in three + different places: the drawer, the docs reader and the history pane. + Cards come from the work views' own visible list, which the shell + already holds; every other kind is published by the view that was + showing it. */} + ) : view === "workflow" ? ( ) : view === "memory" ? ( ) : ( @@ -1990,9 +2098,6 @@ function App() { tasks={tasks} areas={areas} schema={schema} - orderedIds={visibleTasks.map( - (task) => task.id - )} // `openRecord`, not `selectRecord`. This prop reaches the // body's `[[DOC-0002]]` links and the `origin` and // `related` rows, all of which carry records of any kind. @@ -2076,6 +2181,7 @@ function App() { /> )} + ); } diff --git a/packages/workfile/ui/src/navigation.ts b/packages/workfile/ui/src/navigation.ts index 28fec19..4a0c60c 100644 --- a/packages/workfile/ui/src/navigation.ts +++ b/packages/workfile/ui/src/navigation.ts @@ -136,3 +136,39 @@ export function drawerCovers(view: View, collection: string | null): boolean { if (!collection) return false; return VIEW_OWNS_DRAWER[view] !== collection; } + +/** + * Where previous and next go from here, or nothing when there is nowhere. + * + * The list is whatever the view was showing, in the order it was showing it: + * filtered, sorted, grouped as the reader sees it. `null` on both sides means + * the control is not rendered at all, which happens in three ways and they are + * deliberately the same answer: + * + * - no list, because the record was reached from a `[[LRN-0004]]` in a body, a + * `related` row, the command palette, or a node of the Workflow graph; + * - a record that is not in the list, because the reader followed a link out of + * it — the neighbours of where they used to be are not the neighbours of + * where they are; + * - a list of one, where there is nothing to step to. + * + * Absent rather than guessed, per ADR-0018: a next that means something + * different depending on how you arrived is worse than no next. At the ends of a + * real list one side is null and the other is not, so the control renders with + * one button disabled — which is how a reader tells "no next" from "there was + * never a sequence here". + * + * Here rather than in the component so the rule can be tested without a + * renderer, and beside the other navigation rules because it is one. + */ +export function recordNeighbours( + ids: readonly string[], + selectedId: string | null +): { previousId: string | null; nextId: string | null } { + const index = selectedId ? ids.indexOf(selectedId) : -1; + if (index < 0) return { previousId: null, nextId: null }; + return { + previousId: index > 0 ? ids[index - 1] : null, + nextId: index < ids.length - 1 ? ids[index + 1] : null + }; +} diff --git a/packages/workfile/ui/src/query.ts b/packages/workfile/ui/src/query.ts index 712a63a..431221c 100644 --- a/packages/workfile/ui/src/query.ts +++ b/packages/workfile/ui/src/query.ts @@ -3,6 +3,7 @@ import { STATUSES, TYPES, type Filters, + type RecordFilters, type Task, type View } from "./types"; @@ -30,12 +31,46 @@ const VIEWS: View[] = [ "health" ]; +/** + * Which parameter each record view's axis filters ride in, decided here beside + * `q` and `find`. + * + * Prefixed by view, every one of them, and the rule is worth more than the five + * characters it costs. `status` is already taken — by the card filter — so + * Memory's would have had to differ anyway, and a table where four names are + * bare and the fifth is not invites the next author to read the exception as + * arbitrary and drop the prefix. The prefix also keeps a new card axis and a new + * record axis from colliding later, which is a collision nothing would report: + * `readUrlState` validates the card `status` against `STATUSES` and answers `""` + * for anything else, so the loser of a name clash filters by nothing and says + * so nowhere. + * + * These values are *not* validated on the way in, unlike the card axes. The + * legal ones come from the workspace schema — which collections exist, which + * statuses each declares, which visibilities the changelog offers — and none of + * it has been fetched when this runs. An unknown value therefore reaches the + * server, which answers nothing, and the chip renders empty; the way out is the + * chip's own "all", which is one click and always present. + */ +const RECORD_FILTER_PARAMS = { + docs: { managedOnly: "docs-managed" }, + history: { state: "history-state", visibility: "history-visibility" }, + memory: { collection: "memory-collection", status: "memory-status" } +} as const; + +/** Nothing narrowed, which is what a fresh URL means. */ +export const NO_RECORD_FILTERS: RecordFilters = { + docs: { managedOnly: false }, + history: { state: "", visibility: "" }, + memory: { collection: "", status: "" } +}; export function readUrlState(): { view: View; selectedId: string | null; recordSearch: string; filters: Filters; + recordFilters: RecordFilters; } { const params = new URLSearchParams(location.search); const view = params.get("view") as View | null; @@ -72,6 +107,28 @@ export function readUrlState(): { milestone: params.get("milestone") || "", showIdeas: params.get("ideas") === "1", showClosed: params.get("closed") === "1" + }, + /** + * And the record collections' axes, which lived in three `useState`s + * until T-0201 and so died on every reload while the free text beside + * them survived. Kept out of `filters` for the reason stated above it: + * these are not card filters and must not answer to the work strip. + */ + recordFilters: { + docs: { + managedOnly: + params.get(RECORD_FILTER_PARAMS.docs.managedOnly) === "1" + }, + history: { + state: params.get(RECORD_FILTER_PARAMS.history.state) || "", + visibility: + params.get(RECORD_FILTER_PARAMS.history.visibility) || "" + }, + memory: { + collection: + params.get(RECORD_FILTER_PARAMS.memory.collection) || "", + status: params.get(RECORD_FILTER_PARAMS.memory.status) || "" + } } }; } @@ -85,16 +142,21 @@ export function readUrlState(): { * under one entry per keystroke. Everything used to be `replaceState`, so the * history never grew and Back left the application entirely. * - * `find` rides the options bag rather than a fifth positional argument: the - * three- and four-argument calls are the whole of how the rest of the app - * writes a URL, and a new parameter in front of the bag would rewrite them - * all to say nothing new. + * `find` and `recordFilters` ride the options bag rather than a fifth and sixth + * positional argument: the three- and four-argument calls are the whole of how + * the rest of the app writes a URL, and a new parameter in front of the bag + * would rewrite them all to say nothing new. Both default to empty, so a caller + * with no record view to reflect passes neither. */ export function writeUrlState( view: View, filters: Filters, selectedId: string | null, - { push = false, find = "" }: { push?: boolean; find?: string } = {} + { + push = false, + find = "", + recordFilters = NO_RECORD_FILTERS + }: { push?: boolean; find?: string; recordFilters?: RecordFilters } = {} ) { const params = new URLSearchParams(); if (view !== "overview") params.set("view", view); @@ -109,6 +171,28 @@ export function writeUrlState( if (filters.milestone) params.set("milestone", filters.milestone); if (filters.showIdeas) params.set("ideas", "1"); if (filters.showClosed) params.set("closed", "1"); + // Each of these is set only when it narrows something, so clearing a filter + // takes its parameter out of the address bar instead of leaving an empty one + // behind for the next reader to inherit and wonder about. + // `history` renamed on the way out: the bag's field is named after the view, + // and binding that name here would shadow `window.history` for the rest of + // this function — which ends in `history.pushState`. + const { docs, history: changes, memory } = recordFilters; + if (docs.managedOnly) { + params.set(RECORD_FILTER_PARAMS.docs.managedOnly, "1"); + } + if (changes.state) { + params.set(RECORD_FILTER_PARAMS.history.state, changes.state); + } + if (changes.visibility) { + params.set(RECORD_FILTER_PARAMS.history.visibility, changes.visibility); + } + if (memory.collection) { + params.set(RECORD_FILTER_PARAMS.memory.collection, memory.collection); + } + if (memory.status) { + params.set(RECORD_FILTER_PARAMS.memory.status, memory.status); + } if (selectedId) params.set("record", selectedId); const query = params.toString(); const url = `${location.pathname}${query ? `?${query}` : ""}`; diff --git a/packages/workfile/ui/src/record-cursor.tsx b/packages/workfile/ui/src/record-cursor.tsx new file mode 100644 index 0000000..f920b1e --- /dev/null +++ b/packages/workfile/ui/src/record-cursor.tsx @@ -0,0 +1,109 @@ +import { createContext, useContext, useMemo, type ReactNode } from "react"; +import { ChevronLeft, ChevronRight } from "lucide-react"; + +import { Button } from "@/components/ui/button"; +import { ButtonGroup } from "@/components/ui/button-group"; + +import { recordNeighbours } from "./navigation"; + +/** + * Where the reader is in the list they opened this record from. + * + * `Inspector` has carried a previous/next cursor for cards since the rail became + * a drawer, so working through the Explorer never means going back to the table. + * Every other kind had none: reading three changelog fragments in a row meant + * dismissing the reader, finding your place in the list and clicking again. That + * is T-0207, and ADR-0018's surviving finding. + * + * A context rather than five sets of props, for the reason `read-only.tsx` gives + * for the same choice: the panels this has to reach — the card inspector, the + * memory panel, the generic record panel, and the readers Docs and History own + * themselves — already take eight props each, and the list has to arrive at all + * of them or the cursor is back to being one view's feature. + * + * The list is whatever the view was showing, in the order it was showing it: + * filtered, sorted, and grouped as the reader sees it, never the whole corpus. + * A cursor over a list the reader cannot see is a cursor that jumps. + */ +interface Cursor { + previousId: string | null; + nextId: string | null; + step: (id: string) => void; +} + +/** Nowhere to step, which is the default for a tree with no provider over it. */ +const NO_CURSOR: Cursor = { + previousId: null, + nextId: null, + step: () => {} +}; + +const RecordCursorContext = createContext(NO_CURSOR); + +export function RecordCursorProvider({ + ids, + selectedId, + onStep, + children +}: { + /** The list the reader is moving along, in display order. */ + ids: string[]; + selectedId: string | null; + onStep: (id: string) => void; + children: ReactNode; +}) { + // The rule itself is in `navigation.ts`, with the other navigation rules and + // where a test can reach it without a renderer. + const value = useMemo( + () => ({ ...recordNeighbours(ids, selectedId), step: onStep }), + [ids, onStep, selectedId] + ); + return ( + + {children} + + ); +} + +export function useRecordCursor() { + return useContext(RecordCursorContext); +} + +/** + * The control, which is the one the card inspector has always used. + * + * Absent when there is nowhere to step — no list, or a list of one — and + * disabled at each end of a real one, so the reader can tell "there is no next" + * from "there was never a sequence here". Two focusable buttons and no key + * binding of their own: the drawer already owns Escape and the shell owns ⌘K, + * and a reader typing in the editor inside that drawer must not have a keystroke + * taken to move the record out from under them. + */ +export function RecordCursor({ noun = "record" }: { noun?: string }) { + const { previousId, nextId, step } = useRecordCursor(); + if (!previousId && !nextId) return null; + return ( + + + + + ); +} diff --git a/packages/workfile/ui/src/types.ts b/packages/workfile/ui/src/types.ts index 6b12bdf..385b261 100644 --- a/packages/workfile/ui/src/types.ts +++ b/packages/workfile/ui/src/types.ts @@ -339,6 +339,38 @@ export interface Filters { showClosed: boolean; } +/** The axis filters Docs offers. */ +export interface DocsFilters { + managedOnly: boolean; +} + +/** The axis filters History offers. */ +export interface HistoryFilters { + state: string; + visibility: string; +} + +/** The axis filters Memory offers. */ +export interface MemoryFilters { + collection: string; + status: string; +} + +/** + * The record collections' axis filters, one bag per view. + * + * `Filters` above is card-shaped and the work views share every field of it. + * These are not: a collection only Memory has and a visibility only History has + * are not one filter under two names, so each view gets exactly its own bag and + * can neither read nor write another's. Which URL parameter each rides in is + * decided in `query.ts`, beside `q` and `find`. + */ +export interface RecordFilters { + docs: DocsFilters; + history: HistoryFilters; + memory: MemoryFilters; +} + export type SortKey = | "id" | "title"