A document can be renamed, and the list header is one field
Three of Paul's four notes, and they turned out to be one change.
The list header held a search field, a "Search documents" button, a
"Title" field and a "New document" button — four controls, two of them
redundant. The title field could set a name and never correct one,
because nothing in the app could rename a document at all. So removing
it is only defensible alongside the thing that replaces it: the title
is now an editable field on the document's own bar, which is both where
a person is looking when they decide what a document is called and the
only place they can change their mind later.
The search button goes because the field can now answer on its own.
fixtures/live-search.mjs fetches GET /?q=… — the same URL the form
submits to, the same handler, the same template — and swaps in that
response's [docs-results] region. It renders no rows, formats no dates
and holds no copy. The alternative was building the list from the JSON
the palette already uses, which would have put "No documents match",
the recency group labels and the relative-time strings into a bundle,
in twelve locales. Enter still submits for real, so the query still
lands in history; the live update uses replaceState so typing does not.
Renaming is a real <form method="post"> around a real <input>, posting
to a new POST /d/{sqid}/title. It works with no JavaScript: the submit
button that :focus-within reveals posts and 303s back with the new name
in the bar, the tab and <title>. fixtures/title.mjs adds Enter, blur
and Escape on top, and when its fetch fails it submits the form for
real rather than showing a message about it.
The field shows what the server STORED, never what was typed.
title.go's normaliseTitle collapses whitespace runs, turns control
characters and line breaks into spaces, and caps at 200 runes — by
rune, because slicing by byte cuts a multi-byte character in half — and
an empty rename becomes docs.document.untitled. A client that trusted
its own string would show a name the server does not have and go on
showing it until the next reload.
Store.Rename scopes by owner in its WHERE clause, like Get and List, so
a stranger's document is never selected rather than found and refused:
404 for someone else's and 404 for a missing one, spec §4.2. It bumps
UpdatedAt, because the list orders by it and naming a document is
touching it. It does not write to the CRDT, so a collaborator sees the
old name until they reload — stated as a boundary in Store.Rename
rather than left to be discovered.
The File menu's sideways scrollbar was a third thing entirely.
rastrillo's tokens.css deliberately has no `* { box-sizing }` reset —
it says so, and gives the reason — so [docs-menu-item] was content-box
and `inline-size: 100%` plus its padding overflowed every panel by
24px. It showed up on File and only on File because a <button> gets
border-box from the UA stylesheet and an <a> does not, and File is the
one menu that is mostly links. The horizontal scrollbar came from
[docs-menu-items]'s own overflow-y: auto, which forces the other axis
to compute to auto. Nothing here sets overflow-x: hidden; that would
have hidden this one and the next one.
Two new test files, both in the gate. The ordering guard in
live-search.mjs was mutation-tested — removing both sequence checks
turns "a slow earlier response cannot overwrite a newer one" red — and
the second of the two is documented as uncovered by the fake, so the
next person to find it looking spare knows why.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DoVTH3jZqj9svgt2UD9t1p
14 files changed,
+1903
−165
.amadan/ci.d/10-gate+6 −0fixtures/jump-to.mjs+43 −0fixtures/live-search.mjs+234 −0fixtures/live-search.test.mjs+257 −0fixtures/title.mjs+180 −0fixtures/title.test.mjs+228 −0internal/docs/app.go+11 −0internal/docs/handlers.go+122 −25internal/docs/render.go+23 −4internal/docs/title.go+151 −0internal/docs/title_internal_test.go+87 −0internal/docs/title_test.go+291 −0internal/docs/views/list.html+107 −116internal/docs/views/show.html+163 −20
Intent
diff --git a/.amadan/ci.d/10-gate b/.amadan/ci.d/10-gate| index f23a066..629b58a 100755 |
| --- a/.amadan/ci.d/10-gate |
| +++ b/.amadan/ci.d/10-gate |
| @@ -57,6 +57,12 @@ echo "=== fixtures: the resync retry/backoff state machine (fix round 2 regressi |
| echo "=== fixtures: the live-update cursor and connection policy" |
| (cd fixtures && node --test live-updates.test.mjs) |
| +echo "=== fixtures: the document list's live search" |
| +(cd fixtures && node --test live-search.test.mjs) |
| + |
| +echo "=== fixtures: renaming a document from its title bar" |
| +(cd fixtures && node --test title.test.mjs) |
| + |
| # fixtures/build-bundles.mjs is byte-reproducible: no sourcemaps, no |
| # banners, no timestamps, no absolute paths baked into the output, and |
| # esbuild's actual work happens in a pinned native binary (0.28.2), |
diff --git a/fixtures/jump-to.mjs b/fixtures/jump-to.mjs| index ed90bde..4a2879d 100644 |
| --- a/fixtures/jump-to.mjs |
| +++ b/fixtures/jump-to.mjs |
| @@ -37,6 +37,9 @@ |
| // is stated here rather than only cited, and this comment does not |
| // depend on which order they merge in.) |
| +import { mountLiveSearch } from './live-search.mjs' |
| +import { mountTitleEditor } from './title.mjs' |
| + |
| const DEBOUNCE_MS = 120 |
| export function createJumpTo(root) { |
| @@ -290,3 +293,43 @@ if (mount) { |
| console.error(err) |
| } |
| } |
| + |
| +// Live search on the list header's own field (fixtures/live-search.mjs). |
| +// |
| +// It rides in this bundle rather than one of its own because this |
| +// bundle IS the list page's script — assets.go serves two, `editor` |
| +// and `jumpto`, and the list page loads this one. A third bundle for |
| +// one small module would be a third hashed asset and a third <script> |
| +// on a page that has one job. |
| +// |
| +// In its own try/catch, and mounted second, for the reason the palette |
| +// above has one: these are two independent enhancements over a form |
| +// that already works, and neither is allowed to cost a person the |
| +// other. mountLiveSearch returns null on any screen without the form, |
| +// so this is a no-op on a document page, where the palette above is |
| +// the only thing from this file that runs. |
| +try { |
| + mountLiveSearch() |
| +} catch (err) { |
| + console.error(err) |
| +} |
| + |
| +// Renaming a document from its own title bar (fixtures/title.mjs). |
| +// |
| +// In THIS bundle rather than the editor's, and that is the interesting |
| +// choice here. The document page loads both, and the title form sits |
| +// in the shell bar beside the editor — so the editor bundle looks like |
| +// the natural home. It is the wrong one: that bundle is ProseMirror |
| +// and Yjs, it is two orders of magnitude larger than this one, and a |
| +// person whose editor failed to load can still read their document and |
| +// should still be able to fix its name. Renaming does not depend on |
| +// there being an editor, so it does not depend on the editor arriving. |
| +// |
| +// Own try/catch, mounted last, for the same reason as the two above: |
| +// three independent enhancements over markup that already works, none |
| +// of them allowed to cost a person the others. |
| +try { |
| + mountTitleEditor() |
| +} catch (err) { |
| + console.error(err) |
| +} |
diff --git a/fixtures/live-search.mjs b/fixtures/live-search.mjs| new file mode 100644 |
| index 0000000..4521d4b |
| --- /dev/null |
| +++ b/fixtures/live-search.mjs |
| @@ -0,0 +1,234 @@ |
| +// Live search on the document list: the results below the header |
| +// follow the field as it is typed into, without a submit. |
| +// |
| +// Paul's brief was "remove the 'search documents' button .. the field |
| +// is enough -> can it live search?". Removing the button is only |
| +// honest if the field does something on its own, which is what this |
| +// module is; without it the field would still work (Enter submits a |
| +// single-input GET form natively) but the control that told you so |
| +// would be gone. |
| +// |
| +// THE SERVER RENDERS THE RESULTS. This module fetches GET /?q=… — the |
| +// same URL the form submits to, the same handler, the same template — |
| +// and swaps in that response's [docs-results] region. It builds no |
| +// rows, formats no dates and holds no copy. |
| +// |
| +// That is the whole design, and the alternative is what makes it worth |
| +// stating. GET /documents/search already returns JSON and the palette |
| +// (fixtures/jump-to.mjs) reads it; a live list built from that would |
| +// need "No documents match", the recency group labels ("Today", |
| +// "Earlier"), and the relative-time strings — in twelve locales — to |
| +// exist inside a JavaScript bundle. AGENTS.md forbids a literal in the |
| +// codebase for exactly that reason, and a bundle that fetched its copy |
| +// as data would still be a second renderer capable of disagreeing with |
| +// the first. Fetching the region means the live results and the |
| +// submitted results are not merely consistent: they are the same bytes |
| +// from the same handler, and a change to list.html changes both. |
| +// |
| +// What this module does NOT do is prevent the form's submit. Pressing |
| +// Enter navigates for real, which is what puts the query in the |
| +// browser's history and makes the back button work. The live update |
| +// uses replaceState instead, so a person typing "min", "minu", |
| +// "minut" does not have to press back four times to get out of it. |
| + |
| +// How long the field must be still before a query goes out. |
| +// |
| +// 180ms is chosen against typing speed rather than against a network: |
| +// a fast typist's inter-key gap is around 120-200ms, so this fires |
| +// between words and in the pause before you look at the screen, and |
| +// not once per character in the middle of a burst. Lower and a |
| +// six-letter word costs six requests; much higher and the list feels |
| +// like it is lagging behind the field, which is the one thing live |
| +// search must never feel like. |
| +export const DEBOUNCE_MS = 180 |
| + |
| +// buildURL is the query's URL, and it is the same URL the <form> would |
| +// navigate to — action plus ?q=. Built from the form's own action so |
| +// there is one statement of where the list lives. |
| +// |
| +// An empty query drops the parameter entirely rather than sending |
| +// ?q=, so clearing the field lands on "/" — the unfiltered, grouped |
| +// list — which is exactly where the "Clear search" link goes. Two ways |
| +// to reach one page, not two pages. |
| +export function buildURL(action, query) { |
| + const url = new URL(action, 'http://example.invalid') |
| + const q = query.trim() |
| + if (q) url.searchParams.set('q', q) |
| + else url.searchParams.delete('q') |
| + return url.pathname + url.search |
| +} |
| + |
| +// extractResults pulls the [docs-results] region out of a fetched |
| +// page. |
| +// |
| +// DOMParser rather than assigning the whole response to innerHTML |
| +// anywhere: the response is a full document, and a parsed document is |
| +// inert — its scripts are flagged as already-started and never run, |
| +// and its images and stylesheets are not fetched. So this walks a |
| +// detached tree and imports one subtree from it. |
| +// |
| +// Returns null when the region is missing, which is what a redirect to |
| +// /signin looks like after a session expires. The caller leaves the |
| +// current results alone in that case rather than blanking the list, |
| +// because a person whose session has ended is about to be sent to the |
| +// sign-in page by their next real navigation anyway, and an emptied |
| +// list in the meantime tells them their documents are gone. |
| +export function extractResults(html, parser) { |
| + const parsed = parser.parseFromString(html, 'text/html') |
| + return parsed.querySelector('[docs-results]') |
| +} |
| + |
| +// createLiveSearch wires one search form to one results region. |
| +// |
| +// Every collaborator is injected so this is testable with no jsdom and |
| +// no network — fixtures/live-search.test.mjs runs it against a small |
| +// fake DOM, the same way menu.test.mjs does. |
| +export function createLiveSearch({ |
| + form, |
| + results, |
| + fetchImpl, |
| + parser, |
| + history, |
| + setTimeoutImpl = setTimeout, |
| + clearTimeoutImpl = clearTimeout, |
| +} = {}) { |
| + if (!form || !results) return null |
| + const input = form.querySelector('[docs-search-input]') |
| + if (!input) return null |
| + |
| + const action = form.getAttribute('action') || '/' |
| + let timer = null |
| + // The last query whose results are on screen. Starts as whatever the |
| + // server rendered, so the first keystroke that merely re-types the |
| + // existing query costs nothing. |
| + let shown = input.value.trim() |
| + // A monotonic id, and it is what actually orders the responses. |
| + // |
| + // AbortController is not enough on its own and it is worth saying |
| + // why, because "we abort the old one" reads like it settles this. |
| + // Abort is a request to stop; a response already in the microtask |
| + // queue is delivered regardless. Two queries in flight — "a" slow, |
| + // "ab" fast — can therefore resolve in the order "ab" then "a", and |
| + // the field says "ab" while the list below it answers "a". A |
| + // sequence number makes a stale resolution a no-op no matter when it |
| + // arrives. |
| + let seq = 0 |
| + let inFlight = null |
| + |
| + function apply(html) { |
| + const fresh = extractResults(html, parser) |
| + if (!fresh) return false |
| + const imported = [] |
| + for (const node of Array.from(fresh.childNodes)) { |
| + imported.push(results.ownerDocument.importNode(node, true)) |
| + } |
| + results.replaceChildren(...imported) |
| + return true |
| + } |
| + |
| + async function run(query) { |
| + const mine = ++seq |
| + const url = buildURL(action, query) |
| + if (inFlight) inFlight.abort() |
| + const controller = typeof AbortController === 'function' ? new AbortController() : null |
| + inFlight = controller |
| + results.setAttribute('aria-busy', 'true') |
| + try { |
| + const response = await fetchImpl(url, { |
| + signal: controller ? controller.signal : undefined, |
| + // Same-origin only, and credentials included because the list |
| + // is behind RequireSession — a fetch without them gets the |
| + // sign-in redirect, whose page has no [docs-results] and would |
| + // therefore be dropped by extractResults every single time. |
| + credentials: 'same-origin', |
| + headers: { Accept: 'text/html' }, |
| + }) |
| + if (mine !== seq) return |
| + if (!response.ok) return |
| + const html = await response.text() |
| + // The SECOND check, and it is not redundant even though the |
| + // test suite passes without it. `text()` on a real Response |
| + // reads a stream: the body can still be arriving here long |
| + // after the headers resolved above, which is exactly the window |
| + // a newer query overtakes an older one in. The fake in |
| + // live-search.test.mjs resolves text() immediately, so removing |
| + // this line alone leaves every test green — mutation-tested, and |
| + // recorded here so the next person to find it "uncovered" knows |
| + // that means the fake is small, not that the guard is spare. |
| + // Removing BOTH does turn the ordering test red. |
| + if (mine !== seq) return |
| + if (!apply(html)) return |
| + shown = query.trim() |
| + // replaceState, not pushState: see the head comment. The URL |
| + // stays truthful — copying it out of the bar gives the results |
| + // on screen — without turning every keystroke into a history |
| + // entry to press back through. |
| + if (history && typeof history.replaceState === 'function') { |
| + history.replaceState(history.state ?? null, '', url) |
| + } |
| + } catch (err) { |
| + // An aborted request is the normal case, not a failure: it means |
| + // a newer query replaced this one. Anything else is a network or |
| + // parse problem, and the response to all of them is the same — |
| + // leave the results that are on screen, and let the form's own |
| + // submit be the way through. Nothing here is worth a message: a |
| + // person who types faster than the network sees the previous |
| + // answer for a moment longer, which is what they would see |
| + // anyway. |
| + if (!(err && err.name === 'AbortError')) console.error(err) |
| + } finally { |
| + if (mine === seq) { |
| + results.removeAttribute('aria-busy') |
| + inFlight = null |
| + } |
| + } |
| + } |
| + |
| + function schedule() { |
| + const query = input.value.trim() |
| + if (timer !== null) clearTimeoutImpl(timer) |
| + if (query === shown) { |
| + // Back where we started — a character typed and deleted. Cancel |
| + // the pending query rather than sending one that would return |
| + // what is already on screen. Also cancels the request in flight, |
| + // for the same reason. |
| + seq++ |
| + if (inFlight) inFlight.abort() |
| + results.removeAttribute('aria-busy') |
| + return |
| + } |
| + timer = setTimeoutImpl(() => { |
| + timer = null |
| + run(query) |
| + }, DEBOUNCE_MS) |
| + } |
| + |
| + input.addEventListener('input', schedule) |
| + // type="search" fires `search` for the platform's own clear control |
| + // (the ⨯ inside the field, and Escape on some platforms). It fires |
| + // `input` too in current browsers, but has not always, and a clear |
| + // that does not clear the list is worse than a redundant listener — |
| + // schedule() is idempotent for an unchanged value. |
| + input.addEventListener('search', schedule) |
| + |
| + return { schedule, pending: () => timer !== null } |
| +} |
| + |
| +// mountLiveSearch is the page's entry: find the form, find the region |
| +// it names, wire them. Returns null when either is absent, which is |
| +// every screen that is not the list. |
| +export function mountLiveSearch(doc = document) { |
| + const form = doc.querySelector('[docs-search-form]') |
| + if (!form) return null |
| + const selector = form.getAttribute('data-docs-results') |
| + if (!selector) return null |
| + const results = doc.querySelector(selector) |
| + if (!results) return null |
| + return createLiveSearch({ |
| + form, |
| + results, |
| + fetchImpl: (...args) => fetch(...args), |
| + parser: new DOMParser(), |
| + history: doc.defaultView ? doc.defaultView.history : undefined, |
| + }) |
| +} |
diff --git a/fixtures/live-search.test.mjs b/fixtures/live-search.test.mjs| new file mode 100644 |
| index 0000000..fd12559 |
| --- /dev/null |
| +++ b/fixtures/live-search.test.mjs |
| @@ -0,0 +1,257 @@ |
| +// fixtures/live-search.test.mjs — the live document list. |
| +// |
| +// The properties worth pinning here are all about ORDER and about what |
| +// happens when the network misbehaves, because those are the two |
| +// things a live search gets wrong in ways that look like a working |
| +// feature: a stale response overwriting a fresh one leaves the field |
| +// saying one thing and the list saying another, and a failed request |
| +// that blanks the list tells a person their documents are gone. |
| +// |
| +// Same shape as menu.test.mjs: a small fake DOM, no jsdom, no network, |
| +// no browser. The fake answers exactly what live-search.mjs touches. |
| +// Layout, focus and the real fetch belong in the browser drive. |
| + |
| +import { test } from 'node:test' |
| +import assert from 'node:assert/strict' |
| + |
| +import { buildURL, createLiveSearch, extractResults, DEBOUNCE_MS } from './live-search.mjs' |
| + |
| +// A node with children and the handful of methods this module uses. |
| +function node(tag = 'div', attrs = {}) { |
| + const self = { |
| + tag, |
| + attrs: { ...attrs }, |
| + childNodes: [], |
| + value: '', |
| + listeners: {}, |
| + ownerDocument: { importNode: (n) => n }, |
| + getAttribute: (k) => (k in self.attrs ? self.attrs[k] : null), |
| + setAttribute: (k, v) => { |
| + self.attrs[k] = String(v) |
| + }, |
| + removeAttribute: (k) => { |
| + delete self.attrs[k] |
| + }, |
| + replaceChildren: (...kids) => { |
| + self.childNodes = kids |
| + }, |
| + addEventListener: (name, fn) => { |
| + ;(self.listeners[name] ||= []).push(fn) |
| + }, |
| + fire: (name) => { |
| + for (const fn of self.listeners[name] || []) fn() |
| + }, |
| + querySelector: (sel) => (sel === '[docs-search-input]' ? self.input : null), |
| + } |
| + return self |
| +} |
| + |
| +// A form holding one search input, plus the results region. |
| +function harness({ initial = '' } = {}) { |
| + const form = node('form', { action: '/' }) |
| + const input = node('input') |
| + input.value = initial |
| + form.input = input |
| + const results = node('div') |
| + results.childNodes = [node('p', { 'data-orig': '1' })] |
| + |
| + // Every fetch is resolved by hand so a test decides the order. |
| + const calls = [] |
| + const fetchImpl = (url) => { |
| + const pending = {} |
| + pending.promise = new Promise((resolve, reject) => { |
| + pending.resolve = resolve |
| + pending.reject = reject |
| + }) |
| + calls.push({ url, ...pending }) |
| + return pending.promise |
| + } |
| + |
| + // A parser that returns a region whose one child records the query |
| + // the server was asked for, so a test can tell which response landed. |
| + const parser = { |
| + parseFromString: (html) => ({ |
| + querySelector: (sel) => { |
| + if (sel !== '[docs-results]') return null |
| + if (html === null) return null |
| + const region = node('div') |
| + region.childNodes = [node('p', { 'data-answer': html })] |
| + return region |
| + }, |
| + }), |
| + } |
| + |
| + const historyCalls = [] |
| + const history = { state: null, replaceState: (s, t, url) => historyCalls.push(url) } |
| + |
| + // A timer this test drives: schedule() records, run() fires. |
| + let scheduled = null |
| + const setTimeoutImpl = (fn) => { |
| + scheduled = fn |
| + return 1 |
| + } |
| + const clearTimeoutImpl = () => { |
| + scheduled = null |
| + } |
| + |
| + createLiveSearch({ |
| + form, |
| + results, |
| + fetchImpl, |
| + parser, |
| + history, |
| + setTimeoutImpl, |
| + clearTimeoutImpl, |
| + }) |
| + |
| + return { |
| + form, |
| + input, |
| + results, |
| + calls, |
| + historyCalls, |
| + tick: () => { |
| + const fn = scheduled |
| + scheduled = null |
| + if (fn) fn() |
| + }, |
| + isScheduled: () => scheduled !== null, |
| + answer: (i, body) => calls[i].resolve({ ok: true, text: async () => body }), |
| + fail: (i, err) => calls[i].reject(err), |
| + shown: () => results.childNodes.map((c) => c.getAttribute('data-answer')), |
| + unchanged: () => results.childNodes.length === 1 && results.childNodes[0].getAttribute('data-orig') === '1', |
| + } |
| +} |
| + |
| +const settle = () => new Promise((r) => setImmediate(r)) |
| + |
| +test('typing fetches the list page for that query and swaps the region in', async () => { |
| + const h = harness() |
| + h.input.value = 'minutes' |
| + h.input.fire('input') |
| + h.tick() |
| + assert.equal(h.calls.length, 1) |
| + assert.equal(h.calls[0].url, '/?q=minutes') |
| + h.answer(0, 'minutes') |
| + await settle() |
| + assert.deepEqual(h.shown(), ['minutes']) |
| + // The URL follows, so copying it out of the bar gives what is on |
| + // screen — but through replaceState, not a history entry per key. |
| + assert.deepEqual(h.historyCalls, ['/?q=minutes']) |
| +}) |
| + |
| +// THE ONE THIS FILE EXISTS FOR. Two queries in flight, the older one |
| +// resolving last: without a sequence number the list ends up answering |
| +// "min" while the field says "minutes", and nothing about the DOM looks |
| +// wrong afterwards. |
| +test('a slow earlier response cannot overwrite a newer one', async () => { |
| + const h = harness() |
| + h.input.value = 'min' |
| + h.input.fire('input') |
| + h.tick() |
| + h.input.value = 'minutes' |
| + h.input.fire('input') |
| + h.tick() |
| + assert.equal(h.calls.length, 2) |
| + |
| + // Out of order, on purpose: the newer answer lands first. |
| + h.answer(1, 'minutes') |
| + await settle() |
| + h.answer(0, 'min') |
| + await settle() |
| + |
| + assert.deepEqual(h.shown(), ['minutes'], 'the stale response must be dropped') |
| +}) |
| + |
| +// The module logs a network failure, which is right in a browser and is |
| +// noise in a test run. Silenced around the one test that provokes it, |
| +// rather than in the module: an error that vanished silently would be |
| +// genuinely worse than a noisy test. |
| +async function quietly(fn) { |
| + const realError = console.error |
| + console.error = () => {} |
| + try { |
| + await fn() |
| + } finally { |
| + console.error = realError |
| + } |
| +} |
| + |
| +test('a failed request leaves the results that are on screen', async () => { |
| + await quietly(async () => { |
| + const h = harness() |
| + h.input.value = 'x' |
| + h.input.fire('input') |
| + h.tick() |
| + h.fail(0, new TypeError('network')) |
| + await settle() |
| + assert.ok(h.unchanged(), 'a network failure must not blank the list') |
| + assert.equal(h.results.getAttribute('aria-busy'), null, 'and must clear the busy state') |
| + }) |
| +}) |
| + |
| +// A session that expired redirects to /signin, whose page has no |
| +// results region at all. Blanking the list would tell a person their |
| +// documents are gone at the exact moment they are only signed out. |
| +test('a response with no results region leaves the list alone', async () => { |
| + const h = harness() |
| + h.input.value = 'x' |
| + h.input.fire('input') |
| + h.tick() |
| + h.answer(0, null) |
| + await settle() |
| + assert.ok(h.unchanged()) |
| +}) |
| + |
| +test('typing a character and deleting it sends nothing', async () => { |
| + const h = harness({ initial: 'notes' }) |
| + h.input.value = 'notes!' |
| + h.input.fire('input') |
| + assert.ok(h.isScheduled(), 'a changed value schedules a query') |
| + h.input.value = 'notes' |
| + h.input.fire('input') |
| + assert.ok(!h.isScheduled(), 'and returning to the rendered query cancels it') |
| + assert.equal(h.calls.length, 0) |
| +}) |
| + |
| +test('the field is debounced rather than firing per keystroke', () => { |
| + const h = harness() |
| + for (const v of ['m', 'mi', 'min']) { |
| + h.input.value = v |
| + h.input.fire('input') |
| + } |
| + assert.equal(h.calls.length, 0, 'nothing goes out until the field is still') |
| + h.tick() |
| + assert.equal(h.calls.length, 1) |
| + assert.equal(h.calls[0].url, '/?q=min') |
| + assert.ok(DEBOUNCE_MS > 0) |
| +}) |
| + |
| +// The platform's own clear control inside a type="search" field. It |
| +// fires `search`, and has not always also fired `input`. |
| +test('the native clear control clears the list', () => { |
| + const h = harness({ initial: 'notes' }) |
| + h.input.value = '' |
| + h.input.fire('search') |
| + h.tick() |
| + assert.equal(h.calls[0].url, '/', 'an empty query drops ?q= and lands on the grouped list') |
| +}) |
| + |
| +test('buildURL drops an empty or whitespace query', () => { |
| + assert.equal(buildURL('/', ''), '/') |
| + assert.equal(buildURL('/', ' '), '/') |
| + assert.equal(buildURL('/', 'a b'), '/?q=a+b') |
| + assert.equal(buildURL('/', '&=?'), '/?q=%26%3D%3F') |
| +}) |
| + |
| +test('extractResults returns null when the page has no region', () => { |
| + const parser = { parseFromString: () => ({ querySelector: () => null }) } |
| + assert.equal(extractResults('<html></html>', parser), null) |
| +}) |
| + |
| +test('mounting is a no-op without a form or a results region', () => { |
| + assert.equal(createLiveSearch({ form: null, results: node() }), null) |
| + const form = node('form') |
| + form.input = null |
| + assert.equal(createLiveSearch({ form, results: node() }), null) |
| +}) |
diff --git a/fixtures/title.mjs b/fixtures/title.mjs| new file mode 100644 |
| index 0000000..4e0c399 |
| --- /dev/null |
| +++ b/fixtures/title.mjs |
| @@ -0,0 +1,180 @@ |
| +// fixtures/title.mjs — renaming a document from its own title bar. |
| +// |
| +// The markup (show.html's "brand" block) is a real <form> around a real |
| +// <input> posting to POST /d/{sqid}/title, and it renames a document |
| +// with no JavaScript at all: type, and the submit button that |
| +// :focus-within reveals posts and 303s back. This module does not make |
| +// renaming possible. It makes it feel like a title bar rather than a |
| +// form — Enter and blur save without leaving the page, Escape puts |
| +// back what was there. |
| +// |
| +// # The server decides what the title is |
| +// |
| +// Every save reads the stored title back out of the response and puts |
| +// THAT in the field, rather than assuming the submitted string was |
| +// kept. It is routinely not: handlers.go's rename substitutes |
| +// docs.document.untitled for anything that normalises to nothing, and |
| +// title.go collapses whitespace runs and caps the length. A client that |
| +// trusted its own string would show a name the server does not have, |
| +// and would go on showing it until the next reload — the kind of |
| +// disagreement that gets noticed days later as "it renamed itself". |
| +// |
| +// # A failed save falls back to the form |
| +// |
| +// Not to a message, and not to silently reverting: on a failed fetch |
| +// this submits the form for real. That is a navigation, and it either |
| +// succeeds — in which case the rename happened and the page reloads |
| +// with the new name — or it lands on the server's own error page, |
| +// which is a thing a person can see and act on. The scriptless path is |
| +// the fallback for the scripted one, which is the arrangement that |
| +// makes writing the scriptless path first worth anything. |
| +// |
| +// # It does not tell collaborators |
| +// |
| +// A title is a row on the documents table, not part of the CRDT, so |
| +// there is nothing for the live-update stream to carry. Somebody else |
| +// with the document open sees the old name until they reload. That is |
| +// stated in Store.Rename's comment as a design boundary rather than an |
| +// oversight: making the title collaborative means putting it in the Yjs |
| +// document, which has real questions in it. |
| + |
| +// createTitleEditor wires one title form. Everything it talks to is |
| +// injected so fixtures/title.test.mjs can drive it with no DOM and no |
| +// network. |
| +export function createTitleEditor({ form, input, fetchImpl, setPageTitle, submitForm } = {}) { |
| + if (!form || !input) return null |
| + |
| + // The last title the SERVER confirmed. Seeded from the rendered |
| + // value, which the server just wrote, and only ever moved by a |
| + // successful response. Every "has this changed?" question below is |
| + // asked against this rather than against the field's own history, |
| + // which is what makes a save idempotent: blurring a field nobody |
| + // edited sends nothing, and so does blurring one edited back to what |
| + // it was. |
| + let saved = input.value |
| + let inFlight = false |
| + |
| + function markValid() { |
| + input.removeAttribute('aria-invalid') |
| + } |
| + |
| + async function save() { |
| + const wanted = input.value |
| + if (wanted === saved) { |
| + markValid() |
| + return |
| + } |
| + // One save at a time. A second one starting mid-flight would race |
| + // the first, and the loser's response would put a stale title in |
| + // the field — the same ordering problem live-search.mjs solves |
| + // with a sequence number, except that here the right answer is |
| + // simply not to start: a title is one short string a person is |
| + // typing, not a stream of independent queries, and the blur that |
| + // triggers this cannot happen twice without a focus in between. |
| + if (inFlight) return |
| + inFlight = true |
| + try { |
| + const body = new URLSearchParams() |
| + body.set('title', wanted) |
| + const response = await fetchImpl(form.getAttribute('action'), { |
| + method: 'POST', |
| + credentials: 'same-origin', |
| + headers: { |
| + // Accept is what selects the JSON representation over the |
| + // 303 redirect — see handlers.go's wantsJSON. Without it |
| + // this fetch would follow the redirect, download the whole |
| + // document page and throw it away. |
| + Accept: 'application/json', |
| + 'Content-Type': 'application/x-www-form-urlencoded', |
| + }, |
| + body: body.toString(), |
| + }) |
| + if (!response.ok) throw new Error(`rename: ${response.status}`) |
| + const data = await response.json() |
| + const stored = typeof data.title === 'string' ? data.title : wanted |
| + saved = stored |
| + // Only write back when the server changed something. Assigning |
| + // input.value unconditionally would move the caret to the end of |
| + // the field, which is visible and wrong if the person has |
| + // already focused it again and started typing. |
| + if (input.value !== stored) input.value = stored |
| + markValid() |
| + if (setPageTitle) setPageTitle(stored) |
| + } catch (err) { |
| + console.error(err) |
| + input.setAttribute('aria-invalid', 'true') |
| + // The real submit. See the head comment: this is a navigation, |
| + // and it is the honest end of a failed save rather than a |
| + // message about one. |
| + if (submitForm) submitForm() |
| + } finally { |
| + inFlight = false |
| + } |
| + } |
| + |
| + function revert() { |
| + input.value = saved |
| + markValid() |
| + } |
| + |
| + input.addEventListener('blur', () => { |
| + save() |
| + }) |
| + |
| + input.addEventListener('keydown', (event) => { |
| + if (event.key === 'Enter') { |
| + // preventDefault so the form does not navigate, then blur, which |
| + // saves through the listener above. Blurring rather than calling |
| + // save() directly is deliberate: Enter in a title bar means "I |
| + // am done with this field", and leaving focus in a field that |
| + // has been committed invites a second Enter that does nothing. |
| + event.preventDefault() |
| + input.blur() |
| + return |
| + } |
| + if (event.key === 'Escape') { |
| + // Escape puts back the stored title and leaves. It must NOT |
| + // save, and it must stop the event: the editor binds Escape to |
| + // move focus to the toolbar (editor-entry.js's keymap), and a |
| + // person abandoning a rename is not asking for that. |
| + event.preventDefault() |
| + event.stopPropagation() |
| + revert() |
| + input.blur() |
| + } |
| + }) |
| + |
| + form.addEventListener('submit', (event) => { |
| + // The revealed submit button, with scripts working. Handled here |
| + // rather than left to navigate so that clicking it behaves like |
| + // Enter — the same save, in place. The blur that clicking the |
| + // button causes has usually already saved by the time this fires, |
| + // and save() returns immediately in that case because the value |
| + // matches `saved`. |
| + event.preventDefault() |
| + save() |
| + }) |
| + |
| + return { save, revert, current: () => saved } |
| +} |
| + |
| +// mountTitleEditor is the page's entry. Returns null on any screen |
| +// without a title form, which is every screen but a document. |
| +export function mountTitleEditor(doc = document) { |
| + const form = doc.querySelector('[docs-title-form]') |
| + if (!form) return null |
| + const input = form.querySelector('[docs-title-input]') |
| + if (!input) return null |
| + return createTitleEditor({ |
| + form, |
| + input, |
| + fetchImpl: (...args) => fetch(...args), |
| + setPageTitle: (t) => { |
| + doc.title = t |
| + }, |
| + // HTMLFormElement.submit(), not requestSubmit(): submit() does not |
| + // fire the submit event, so this cannot re-enter the handler above |
| + // and loop. |
| + submitForm: () => form.submit(), |
| + }) |
| +} |
diff --git a/fixtures/title.test.mjs b/fixtures/title.test.mjs| new file mode 100644 |
| index 0000000..6e6e98b |
| --- /dev/null |
| +++ b/fixtures/title.test.mjs |
| @@ -0,0 +1,228 @@ |
| +// fixtures/title.test.mjs — renaming a document from its title bar. |
| +// |
| +// The properties here are the ones that make a rename either trusted |
| +// or quietly wrong. A field that shows a name the server did not store |
| +// is the worst outcome available: nothing looks broken, and the |
| +// disagreement only surfaces on the next reload. So most of this file |
| +// is about the field ending up holding what came BACK, not what went |
| +// out. |
| +// |
| +// Small fake DOM, no jsdom, no network — the same shape as |
| +// menu.test.mjs and live-search.test.mjs. |
| + |
| +import { test } from 'node:test' |
| +import assert from 'node:assert/strict' |
| + |
| +import { createTitleEditor } from './title.mjs' |
| + |
| +function el(attrs = {}) { |
| + const self = { |
| + attrs: { ...attrs }, |
| + value: '', |
| + listeners: {}, |
| + getAttribute: (k) => (k in self.attrs ? self.attrs[k] : null), |
| + setAttribute: (k, v) => { |
| + self.attrs[k] = String(v) |
| + }, |
| + removeAttribute: (k) => { |
| + delete self.attrs[k] |
| + }, |
| + addEventListener: (name, fn) => { |
| + ;(self.listeners[name] ||= []).push(fn) |
| + }, |
| + blur: () => self.fire('blur'), |
| + fire: (name, event = {}) => { |
| + const ev = { preventDefault() {}, stopPropagation() {}, ...event } |
| + for (const fn of self.listeners[name] || []) fn(ev) |
| + return ev |
| + }, |
| + } |
| + return self |
| +} |
| + |
| +function harness({ rendered = 'Notes' } = {}) { |
| + const form = el({ action: '/d/abc/title' }) |
| + const input = el() |
| + input.value = rendered |
| + |
| + const posts = [] |
| + let reply = { ok: true, json: async () => ({ title: 'Notes' }) } |
| + |
| + const submits = [] |
| + const pageTitles = [] |
| + |
| + createTitleEditor({ |
| + form, |
| + input, |
| + fetchImpl: async (url, init) => { |
| + posts.push({ url, body: init.body, accept: init.headers.Accept }) |
| + if (reply instanceof Error) throw reply |
| + return reply |
| + }, |
| + setPageTitle: (t) => pageTitles.push(t), |
| + submitForm: () => submits.push(true), |
| + }) |
| + |
| + return { |
| + form, |
| + input, |
| + posts, |
| + submits, |
| + pageTitles, |
| + answer: (title) => { |
| + reply = { ok: true, json: async () => ({ title }) } |
| + }, |
| + status: (code) => { |
| + reply = { ok: false, status: code, json: async () => ({}) } |
| + }, |
| + boom: (err) => { |
| + reply = err |
| + }, |
| + } |
| +} |
| + |
| +const settle = () => new Promise((r) => setImmediate(r)) |
| + |
| +async function quietly(fn) { |
| + const realError = console.error |
| + console.error = () => {} |
| + try { |
| + await fn() |
| + } finally { |
| + console.error = realError |
| + } |
| +} |
| + |
| +test('blurring an edited field posts the new title', async () => { |
| + const h = harness() |
| + h.input.value = 'Minutes' |
| + h.answer('Minutes') |
| + h.input.fire('blur') |
| + await settle() |
| + assert.equal(h.posts.length, 1) |
| + assert.equal(h.posts[0].url, '/d/abc/title') |
| + assert.equal(h.posts[0].body, 'title=Minutes') |
| + assert.equal(h.posts[0].accept, 'application/json', 'must ask for JSON, or it follows the 303') |
| + assert.deepEqual(h.pageTitles, ['Minutes'], 'the browser tab follows too') |
| +}) |
| + |
| +test('blurring an untouched field posts nothing', async () => { |
| + const h = harness() |
| + h.input.fire('blur') |
| + await settle() |
| + assert.equal(h.posts.length, 0) |
| +}) |
| + |
| +test('editing a title back to what it was posts nothing', async () => { |
| + const h = harness({ rendered: 'Notes' }) |
| + h.input.value = 'Notes!' |
| + h.input.value = 'Notes' |
| + h.input.fire('blur') |
| + await settle() |
| + assert.equal(h.posts.length, 0) |
| +}) |
| + |
| +// THE ONE THAT MATTERS. The server normalises: an empty title becomes |
| +// "Untitled document", whitespace runs collapse, length is capped. |
| +// Trusting the submitted string leaves the field showing a name that |
| +// was never stored. |
| +test('the field shows what the server stored, not what was typed', async () => { |
| + const h = harness() |
| + h.input.value = ' ' |
| + h.answer('Untitled document') |
| + h.input.fire('blur') |
| + await settle() |
| + assert.equal(h.input.value, 'Untitled document') |
| + assert.deepEqual(h.pageTitles, ['Untitled document']) |
| +}) |
| + |
| +test('a second save after that one sends nothing, because it matches', async () => { |
| + const h = harness() |
| + h.input.value = ' Notes again ' |
| + h.answer('Notes again') |
| + h.input.fire('blur') |
| + await settle() |
| + assert.equal(h.input.value, 'Notes again') |
| + h.input.fire('blur') |
| + await settle() |
| + assert.equal(h.posts.length, 1, 'the collapsed title is now the baseline') |
| +}) |
| + |
| +test('a refused save falls back to submitting the form for real', async () => { |
| + await quietly(async () => { |
| + const h = harness() |
| + h.input.value = 'Minutes' |
| + h.status(500) |
| + h.input.fire('blur') |
| + await settle() |
| + assert.deepEqual(h.submits, [true]) |
| + assert.equal(h.input.attrs['aria-invalid'], 'true') |
| + assert.equal(h.input.value, 'Minutes', 'and keeps what was typed') |
| + }) |
| +}) |
| + |
| +test('a network failure does the same', async () => { |
| + await quietly(async () => { |
| + const h = harness() |
| + h.input.value = 'Minutes' |
| + h.boom(new TypeError('network')) |
| + h.input.fire('blur') |
| + await settle() |
| + assert.deepEqual(h.submits, [true]) |
| + }) |
| +}) |
| + |
| +test('Enter saves without navigating', async () => { |
| + const h = harness() |
| + h.input.value = 'Minutes' |
| + h.answer('Minutes') |
| + let prevented = false |
| + h.input.fire('keydown', { key: 'Enter', preventDefault: () => (prevented = true) }) |
| + await settle() |
| + assert.ok(prevented, 'the form must not navigate') |
| + assert.equal(h.posts.length, 1) |
| +}) |
| + |
| +// Escape must revert AND must not reach the editor's own Escape |
| +// binding, which moves focus to the formatting bar. Abandoning a |
| +// rename is not a request to leave the document. |
| +test('Escape reverts and is not passed on', async () => { |
| + const h = harness({ rendered: 'Notes' }) |
| + h.input.value = 'Minutes' |
| + let stopped = false |
| + h.input.fire('keydown', { key: 'Escape', stopPropagation: () => (stopped = true) }) |
| + await settle() |
| + assert.equal(h.input.value, 'Notes') |
| + assert.equal(h.posts.length, 0, 'reverting must not save') |
| + assert.ok(stopped) |
| +}) |
| + |
| +test('submitting the form saves in place rather than navigating', async () => { |
| + const h = harness() |
| + h.input.value = 'Minutes' |
| + h.answer('Minutes') |
| + let prevented = false |
| + h.form.fire('submit', { preventDefault: () => (prevented = true) }) |
| + await settle() |
| + assert.ok(prevented) |
| + assert.equal(h.posts.length, 1) |
| +}) |
| + |
| +// Clicking the revealed submit button blurs the field first, so the |
| +// blur save and the submit save both fire. The second must be a no-op |
| +// rather than a second POST. |
| +test('blur then submit is one save, not two', async () => { |
| + const h = harness() |
| + h.input.value = 'Minutes' |
| + h.answer('Minutes') |
| + h.input.fire('blur') |
| + await settle() |
| + h.form.fire('submit') |
| + await settle() |
| + assert.equal(h.posts.length, 1) |
| +}) |
| + |
| +test('mounting is a no-op without a form or a field', () => { |
| + assert.equal(createTitleEditor({ form: null, input: el() }), null) |
| + assert.equal(createTitleEditor({ form: el(), input: null }), null) |
| +}) |
diff --git a/internal/docs/app.go b/internal/docs/app.go| index 757f415..adcb99d 100644 |
| --- a/internal/docs/app.go |
| +++ b/internal/docs/app.go |
| @@ -449,6 +449,17 @@ func New(cfg Config) (*App, error) { |
| // check of its own — a person's document titles are theirs. |
| r.Get("/documents/search", h.searchJSON) |
| r.Get("/d/{sqid}", h.show) |
| + // The document's own name (title.go, handlers.go's rename). |
| + // A POST, so csrf.Protect above covers it exactly as it covers |
| + // /documents and /signout — and it is inside this group, so a |
| + // signed-out attempt gets the same treatment every other |
| + // document route gives it. |
| + // |
| + // A separate path rather than a PATCH on /d/{sqid}: a plain |
| + // <form> can only GET or POST, and the scriptless path is the |
| + // one this route exists to keep working. Naming the field in |
| + // the URL is also what makes it readable in a log. |
| + r.Post("/d/{sqid}/title", h.rename) |
| // Task 5: the update log, what the browser's editor reads to |
| // build its Yjs doc (GET) and posts new edits back through |
| // (POST) — see handlers.go's updatesGet/updatesPost. Neither |
diff --git a/internal/docs/handlers.go b/internal/docs/handlers.go| index d7aceaa..fcc6677 100644 |
| --- a/internal/docs/handlers.go |
| +++ b/internal/docs/handlers.go |
| @@ -1,6 +1,7 @@ |
| package docs |
| import ( |
| + "encoding/json" |
| "errors" |
| "html/template" |
| "io" |
| @@ -59,7 +60,7 @@ type handlers struct { |
| // a row's Title and When are the document's own data, not authored |
| // copy, so list.html prints them directly rather than through T. |
| // |
| -// CreateLabel and CreatePlaceholder back the create form list.html |
| +// CreateLabel backs the create form list.html |
| // renders in its page header — present regardless of Groups, so it |
| // reaches a person with no documents yet exactly as it reaches |
| // everyone else (fix round 1: the original version of this screen |
| @@ -74,13 +75,12 @@ type handlers struct { |
| // (rastrillo/auth.DefaultSessionTTL); a page with no way to end one |
| // on purpose is a real gap on something public, not a cosmetic one. |
| type listPage struct { |
| - Title string |
| - Groups []docGroup |
| - EmptyTitle string |
| - EmptyBody string |
| - CreateLabel string |
| - CreatePlaceholder string |
| - SignOutLabel string |
| + Title string |
| + Groups []docGroup |
| + EmptyTitle string |
| + EmptyBody string |
| + CreateLabel string |
| + SignOutLabel string |
| // Query is what was searched for, echoed back into the field so a |
| // result page is a page a person can edit and re-submit rather |
| @@ -172,20 +172,19 @@ func (h *handlers) list(w http.ResponseWriter, r *http.Request) { |
| query := strings.TrimSpace(r.URL.Query().Get("q")) |
| data := listPage{ |
| - Title: rastrillo.T(r, "docs.list.title"), |
| - EmptyTitle: rastrillo.T(r, "docs.list.empty.title"), |
| - EmptyBody: rastrillo.T(r, "docs.list.empty.body"), |
| - CreateLabel: rastrillo.T(r, "docs.list.create.label"), |
| - CreatePlaceholder: rastrillo.T(r, "docs.list.create.placeholder"), |
| - SignOutLabel: rastrillo.T(r, "docs.signout.label"), |
| - Query: query, |
| - SearchLabel: rastrillo.T(r, "docs.list.search.label"), |
| - SearchPlace: rastrillo.T(r, "docs.list.search.placeholder"), |
| - NoResultsHead: rastrillo.T(r, "docs.list.search.none.title"), |
| - NoResultsBody: rastrillo.T(r, "docs.list.search.none.body"), |
| - ClearLabel: rastrillo.T(r, "docs.list.search.clear"), |
| - JumpTo: newJumpToView(r), |
| - JumpToScript: jumpToScriptPath(), |
| + Title: rastrillo.T(r, "docs.list.title"), |
| + EmptyTitle: rastrillo.T(r, "docs.list.empty.title"), |
| + EmptyBody: rastrillo.T(r, "docs.list.empty.body"), |
| + CreateLabel: rastrillo.T(r, "docs.list.create.label"), |
| + SignOutLabel: rastrillo.T(r, "docs.signout.label"), |
| + Query: query, |
| + SearchLabel: rastrillo.T(r, "docs.list.search.label"), |
| + SearchPlace: rastrillo.T(r, "docs.list.search.placeholder"), |
| + NoResultsHead: rastrillo.T(r, "docs.list.search.none.title"), |
| + NoResultsBody: rastrillo.T(r, "docs.list.search.none.body"), |
| + ClearLabel: rastrillo.T(r, "docs.list.search.clear"), |
| + JumpTo: newJumpToView(r), |
| + JumpToScript: jumpToScriptPath(), |
| } |
| if query != "" { |
| @@ -241,7 +240,13 @@ func (h *handlers) create(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| - title := strings.TrimSpace(r.PostFormValue("title")) |
| + // The list header no longer HAS a title field — a new document is |
| + // created untitled and named on its own title bar (title.go, and |
| + // list.html's own comment on the create form). The read stays, |
| + // because POST /documents is a public URL and a request that sends |
| + // `title` is not wrong; it is just no longer the way anyone gets |
| + // here from the UI. |
| + title := normaliseTitle(r.PostFormValue("title")) |
| if title == "" { |
| title = rastrillo.T(r, "docs.document.untitled") |
| } |
| @@ -255,6 +260,90 @@ func (h *handlers) create(w http.ResponseWriter, r *http.Request) { |
| http.Redirect(w, r, "/d/"+doc.Sqid, http.StatusSeeOther) |
| } |
| +// rename serves POST /d/{sqid}/title: the document's own name, |
| +// changed from the field the title bar renders (show.html's |
| +// [docs-title-form], fixtures/title.mjs). |
| +// |
| +// It answers in one of two ways, chosen by the request's Accept |
| +// header, and the pair is the whole reason this is one route rather |
| +// than two: |
| +// |
| +// - An ordinary form post — no JavaScript, or JavaScript that failed |
| +// to load — gets 303 See Other back to the document, exactly like |
| +// create above. The page reloads with the new name in the bar, in |
| +// the browser tab, and in <title>. Nothing about the feature needs |
| +// a script. |
| +// - A fetch that asks for JSON gets the stored title back as JSON and |
| +// stays on the page. It has to be the STORED title rather than |
| +// nothing, because what was submitted and what was saved can |
| +// differ: an empty submit becomes docs.document.untitled, and |
| +// normaliseTitle collapses whitespace and caps length. A client |
| +// that assumed its own string had been stored would show a title |
| +// the server does not have. |
| +// |
| +// Accept, rather than a header of this app's own invention, because |
| +// this is exactly what content negotiation is: the same resource, two |
| +// representations. It also means the scriptless path is the DEFAULT — |
| +// anything that does not ask for JSON gets the redirect — which is the |
| +// right way round for a fallback. |
| +func (h *handlers) rename(w http.ResponseWriter, r *http.Request) { |
| + sqid := r.PathValue("sqid") |
| + |
| + owner, err := h.owner(r) |
| + if err != nil { |
| + h.logger.Error("docs: resolve signed-in person", "err", err) |
| + http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError) |
| + return |
| + } |
| + |
| + title := normaliseTitle(r.PostFormValue("title")) |
| + if title == "" { |
| + title = rastrillo.T(r, "docs.document.untitled") |
| + } |
| + |
| + doc, err := h.store.Rename(r.Context(), owner, sqid, title) |
| + if err != nil { |
| + if errors.Is(err, gorm.ErrRecordNotFound) { |
| + // The same 404 show gives, for the same reason: a document |
| + // somebody else owns must be indistinguishable from one |
| + // that does not exist (spec §4.2). Store.Rename scopes by |
| + // owner in its WHERE clause, so this branch covers both |
| + // without either being identified. |
| + http.Error(w, rastrillo.T(r, "docs.error.notfound"), http.StatusNotFound) |
| + return |
| + } |
| + h.logger.Error("docs: rename document", "sqid", sqid, "err", err) |
| + http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError) |
| + return |
| + } |
| + |
| + if wantsJSON(r) { |
| + w.Header().Set("Content-Type", "application/json; charset=utf-8") |
| + if err := json.NewEncoder(w).Encode(struct { |
| + Title string `json:"title"` |
| + }{Title: doc.Title}); err != nil { |
| + h.logger.Error("docs: encode rename response", "sqid", sqid, "err", err) |
| + } |
| + return |
| + } |
| + http.Redirect(w, r, "/d/"+doc.Sqid, http.StatusSeeOther) |
| +} |
| + |
| +// wantsJSON reports whether the caller asked for the JSON |
| +// representation. |
| +// |
| +// A substring check on Accept rather than a full media-type parse, and |
| +// the looseness is bounded: the only thing this decides is which of two |
| +// representations of the same successful write to send back. A request |
| +// that says application/json anywhere in its Accept is asking for JSON; |
| +// everything else — including the browser's own |
| +// "text/html,application/xhtml+xml,..." — takes the redirect. There is |
| +// no authorisation, no side effect and no parsing downstream that a |
| +// wrong guess here could reach. |
| +func wantsJSON(r *http.Request) bool { |
| + return strings.Contains(r.Header.Get("Accept"), "application/json") |
| +} |
| + |
| // showPage is GET /d/{sqid}'s view data (spec §6.1/§6.2). Title is the |
| // document's own title — data, not authored copy, printed as-is the |
| // same way docRow.Title is on the list. HTML is Store.HTML's rendered |
| @@ -287,8 +376,14 @@ type showPage struct { |
| // to happen in eleven locales nobody in this repo reads. One key, |
| // one string, and the link can never disagree with the page it |
| // leads to. |
| - BackLabel string |
| - Sqid string // views/show.html's [docs-editor] mount point's data-sqid, the editor bundle's own base for /d/{sqid}/updates |
| + BackLabel string |
| + Sqid string |
| + // TitleLabel and TitleSave back the editable title in the shell |
| + // bar — the field's accessible name, and its scriptless submit |
| + // button. Title above is the document's own name and is data; |
| + // these two are copy. |
| + TitleLabel string |
| + TitleSave string // views/show.html's [docs-editor] mount point's data-sqid, the editor bundle's own base for /d/{sqid}/updates |
| EditorScript string // views/show.html's <script src>, from editorScriptPath() — never a literal, so it can never drift from what staticHandler actually serves |
| // SaveRetrying/SaveFailed are the editor's own save-status copy |
| @@ -436,6 +531,8 @@ func (h *handlers) show(w http.ResponseWriter, r *http.Request) { |
| SignOutLabel: rastrillo.T(r, "docs.signout.label"), |
| BackLabel: rastrillo.T(r, "docs.list.title"), |
| Sqid: doc.Sqid, |
| + TitleLabel: rastrillo.T(r, "docs.editor.title.label"), |
| + TitleSave: rastrillo.T(r, "docs.editor.title.save"), |
| EditorScript: editorScriptPath(), |
| SaveRetrying: rastrillo.T(r, "docs.editor.status.retrying"), |
| SaveFailed: rastrillo.T(r, "docs.editor.status.failed"), |
diff --git a/internal/docs/render.go b/internal/docs/render.go| index e18cbe5..674d5f7 100644 |
| --- a/internal/docs/render.go |
| +++ b/internal/docs/render.go |
| @@ -77,10 +77,13 @@ var Catalog = rastrillo.Catalog{ |
| // header, present whether or not Groups is empty (fix round 1: |
| // the plan had scoped a create *control* out of every task, so the |
| // screen this plan exists for could not be driven from a browser |
| - // at all). label is both the submit button's text and the title |
| - // field's accessible name; placeholder is that field's hint text. |
| - "docs.list.create.label": "New document", |
| - "docs.list.create.placeholder": "Title", |
| + // at all). One key, because the control is one button: this was |
| + // a labelled title field beside that button, and the field is |
| + // gone (Paul: "Remove the 'title' beside new document"). A new |
| + // document is created untitled and named on its own title bar, |
| + // which is the only surface that can also RENAME one — see |
| + // docs.editor.title.* below. |
| + "docs.list.create.label": "New document", |
| // docs.list.search.* — finding a document by its title (spec |
| // §15.2). The field is a GET form, so a search is a link and works |
| @@ -277,6 +280,22 @@ var Catalog = rastrillo.Catalog{ |
| "docs.editor.menu.format": "Format", |
| "docs.editor.menu.tools": "Tools", |
| + // docs.editor.title.* — the document's own name, as an editable |
| + // field in the shell bar (show.html's [docs-title-form], |
| + // handlers.go's rename). label is the field's accessible name and |
| + // is the only thing that says in words what the field is for, |
| + // since the control deliberately does not look like a form field |
| + // until it is hovered or focused; save is the submit button that |
| + // :focus-within reveals, which is the whole rename control for |
| + // somebody with no JavaScript. |
| + // |
| + // "Rename", not "Save": the button is beside a field that already |
| + // holds the current name, and "Save" beside a document's title |
| + // reads as saving the DOCUMENT, which this app does continuously |
| + // and never with a button. |
| + "docs.editor.title.label": "Document name", |
| + "docs.editor.title.save": "Rename", |
| + |
| "docs.editor.menu.file.new": "New document", |
| "docs.editor.menu.file.open": "Open a document", |
| "docs.editor.menu.file.print": "Print", |
diff --git a/internal/docs/title.go b/internal/docs/title.go| new file mode 100644 |
| index 0000000..9b8a4ca |
| --- /dev/null |
| +++ b/internal/docs/title.go |
| @@ -0,0 +1,151 @@ |
| +package docs |
| + |
| +// A document's title: how one is cleaned up, and how one is changed. |
| +// |
| +// Until this file there was no way to change a title at all. It was set |
| +// once by a field beside "New document" on the list header and was then |
| +// permanent — a typo in a name was permanent, and a document whose |
| +// subject had moved on kept the name of what it used to be. That field |
| +// is gone (list.html's own comment), and this is what replaces it: the |
| +// title is edited where it is displayed, on the document itself. |
| +// |
| +// The two halves here are separate on purpose. normaliseTitle is what |
| +// makes a submitted string safe to store and sane to display, and BOTH |
| +// entry points run it — Store.Create through handlers.go's create, and |
| +// Store.Rename through rename. A cleaning rule that only one caller |
| +// applies is a rule the other caller's rows do not follow, and the |
| +// difference shows up months later as one document that behaves |
| +// differently from every other. |
| + |
| +import ( |
| + "context" |
| + "fmt" |
| + "strings" |
| + "time" |
| + "unicode" |
| + |
| + "gorm.io/gorm" |
| +) |
| + |
| +// maxTitleRunes caps a stored title. |
| +// |
| +// RUNES, not bytes, because the cap is about how long a name is and |
| +// twelve locales disagree wildly about how many bytes that takes: 200 |
| +// bytes is 200 Latin characters and about 66 Japanese ones, which |
| +// would make the same rule mean three different things depending on |
| +// what language somebody names their document in. |
| +// |
| +// 200 is generous rather than tight, deliberately. This is not a |
| +// storage limit — the column has none — it is a guard against a title |
| +// that is really a pasted paragraph, which breaks the shell bar's |
| +// layout, the browser's tab, the list rows and the PDF header all at |
| +// once. Anything a person would actually call a document is far under |
| +// it, so the cap should never be reached by ordinary use, and a title |
| +// that reaches it is truncated rather than refused: rejecting the |
| +// paste would lose the whole thing, and truncating it keeps the part |
| +// that was probably the name. |
| +const maxTitleRunes = 200 |
| + |
| +// normaliseTitle turns a submitted title into the one that gets |
| +// stored, and returns "" for anything that amounts to no title at all. |
| +// The caller substitutes its own fallback copy — docs.document.untitled |
| +// — because that string is catalog copy resolved against a request's |
| +// locale, and this function has no request. |
| +// |
| +// Three things happen, in this order, and the order matters: |
| +// |
| +// 1. Every control character and line break becomes a space. A title |
| +// is a single line everywhere it appears — the shell bar, the |
| +// browser tab, a list row, the PDF's header — and none of those |
| +// places has anywhere to put a second one. A newline in a <title> |
| +// is not a security problem (html/template escapes it like |
| +// anything else); it is a layout problem, and it arrives easily, |
| +// because pasting two lines into a single-line field is something |
| +// browsers happily allow through the paste event. |
| +// |
| +// 2. Runs of whitespace collapse to one space. Otherwise "Notes — |
| +// draft" and "Notes — draft" are two different titles that look |
| +// identical in every place either is displayed, which makes search |
| +// results and the list order inexplicable. |
| +// |
| +// 3. The result is trimmed and capped. Trimmed after collapsing, so a |
| +// title that was nothing but whitespace and control characters |
| +// ends as "" rather than as " ". |
| +func normaliseTitle(raw string) string { |
| + var b strings.Builder |
| + b.Grow(len(raw)) |
| + lastWasSpace := false |
| + for _, r := range raw { |
| + // unicode.IsControl covers \n, \r, \t and the C0/C1 ranges. |
| + // Treated as spaces rather than dropped: dropping them would |
| + // join the words either side of a line break into one. |
| + if unicode.IsControl(r) || unicode.IsSpace(r) { |
| + if !lastWasSpace { |
| + b.WriteByte(' ') |
| + lastWasSpace = true |
| + } |
| + continue |
| + } |
| + b.WriteRune(r) |
| + lastWasSpace = false |
| + } |
| + title := strings.TrimSpace(b.String()) |
| + |
| + // Counted and cut by rune. Slicing a string by byte here would cut |
| + // a multi-byte character in half and store invalid UTF-8, which |
| + // then reaches a template, a PDF and a JSON response. |
| + runes := []rune(title) |
| + if len(runes) > maxTitleRunes { |
| + title = strings.TrimSpace(string(runes[:maxTitleRunes])) |
| + } |
| + return title |
| +} |
| + |
| +// Rename changes one document's title, scoped to its owner. |
| +// |
| +// Owner-scoped in the WHERE clause rather than by reading the row |
| +// first and comparing: the same discipline Store.Get and Store.List |
| +// follow through owned(). A document somebody else owns is not |
| +// "found and refused", it is never selected, so this returns |
| +// gorm.ErrRecordNotFound for a stranger's document and for a |
| +// nonexistent one alike — and the handler turns both into the same |
| +// 404. A 403 would confirm the document exists. |
| +// |
| +// It bumps UpdatedAt, and that is a real decision rather than an |
| +// artefact of the UPDATE. UpdatedAt is what the list orders by and |
| +// what groupByRecency buckets into "Today" — see Store.List's comment |
| +// on the round where ordering by CreatedAt let the list and its own |
| +// headings disagree. Naming a document is touching it: a person who |
| +// renames three documents and goes back to the list expects to find |
| +// them at the top, where everything else they just did is. |
| +// |
| +// It does NOT write to the CRDT, and a collaborator with the document |
| +// open does not see the new name until they reload. The title is not |
| +// document content — it is a row on the documents table, outside the |
| +// update log entirely — so there is nothing for the live-update stream |
| +// (stream.go) to carry it on. Making the title collaborative means |
| +// putting it in the Yjs document, which is a design with real |
| +// questions in it (two people renaming at once, and what an export's |
| +// header then says), and is not something to do as a side effect of |
| +// making the field editable. |
| +func (s *Store) Rename(ctx context.Context, owner int64, sqid string, title string) (*Document, error) { |
| + now := time.Now().UTC() |
| + res := owned(s.db.WithContext(ctx), owner). |
| + Model(&Document{}). |
| + Where("sqid = ? AND deleted_at IS NULL", sqid). |
| + Updates(map[string]any{"title": title, "updated_at": now}) |
| + if res.Error != nil { |
| + return nil, fmt.Errorf("docs: rename document %q: %w", sqid, res.Error) |
| + } |
| + if res.RowsAffected == 0 { |
| + // Not an error the caller has to distinguish: RowsAffected 0 |
| + // means the owner-scoped WHERE matched nothing, which is the |
| + // same "not yours or not there" the Get path reports. |
| + return nil, fmt.Errorf("docs: rename document %q: %w", sqid, gorm.ErrRecordNotFound) |
| + } |
| + // Read back rather than constructing a Document from what was |
| + // written: the row carries fields this update did not touch, and a |
| + // caller that renders from a half-built struct renders a document |
| + // with no Sqid and no owner. |
| + return s.Get(ctx, owner, sqid) |
| +} |
diff --git a/internal/docs/title_internal_test.go b/internal/docs/title_internal_test.go| new file mode 100644 |
| index 0000000..df62ea8 |
| --- /dev/null |
| +++ b/internal/docs/title_internal_test.go |
| @@ -0,0 +1,87 @@ |
| +package docs |
| + |
| +// normaliseTitle's own rules, tested where it lives because it is |
| +// unexported. What a title becomes matters in more places than it |
| +// looks: the shell bar, the browser tab, the list rows, search |
| +// ranking (search.go normalises again for matching, but orders the |
| +// STORED string) and the PDF's header all render whatever this |
| +// returns. |
| + |
| +import ( |
| + "strings" |
| + "testing" |
| +) |
| + |
| +func TestNormaliseTitleCollapsesAndTrims(t *testing.T) { |
| + cases := []struct { |
| + name string |
| + in string |
| + want string |
| + }{ |
| + {"plain", "Board minutes", "Board minutes"}, |
| + {"leading and trailing space", " Notes ", "Notes"}, |
| + {"collapses runs", "Notes — draft", "Notes — draft"}, |
| + {"tabs are whitespace", "a\tb", "a b"}, |
| + {"newlines become one space", "first\nsecond", "first second"}, |
| + {"CRLF is one space, not two", "first\r\nsecond", "first second"}, |
| + {"empty", "", ""}, |
| + {"whitespace only", " \t\n ", ""}, |
| + {"non-breaking space is still a space", "a b", "a b"}, |
| + {"keeps interior punctuation", "Q3/Q4 — plan (v2)", "Q3/Q4 — plan (v2)"}, |
| + {"keeps non-Latin scripts intact", "会議の議事録", "会議の議事録"}, |
| + } |
| + for _, c := range cases { |
| + t.Run(c.name, func(t *testing.T) { |
| + if got := normaliseTitle(c.in); got != c.want { |
| + t.Errorf("normaliseTitle(%q) = %q, want %q", c.in, got, c.want) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// A control character in a title is a layout problem rather than a |
| +// security one — html/template escapes whatever reaches a template — |
| +// but a title with a line break in it has nowhere to go in a single-row |
| +// shell bar or a browser tab. |
| +func TestNormaliseTitleRemovesControlCharacters(t *testing.T) { |
| + in := "before\x00\x07\x1bafter" |
| + got := normaliseTitle(in) |
| + for _, r := range got { |
| + if r < 0x20 || r == 0x7f { |
| + t.Fatalf("normaliseTitle(%q) = %q, which still holds a control character", in, got) |
| + } |
| + } |
| + if got != "before after" { |
| + t.Errorf("normaliseTitle(%q) = %q, want %q", in, got, "before after") |
| + } |
| +} |
| + |
| +// The cap counts RUNES. Slicing by byte would cut a multi-byte |
| +// character in half and store invalid UTF-8, which then reaches a |
| +// template, a PDF and a JSON response. |
| +func TestNormaliseTitleCapsByRuneNotByte(t *testing.T) { |
| + long := strings.Repeat("あ", maxTitleRunes+50) |
| + got := normaliseTitle(long) |
| + if n := len([]rune(got)); n != maxTitleRunes { |
| + t.Errorf("capped title is %d runes, want %d", n, maxTitleRunes) |
| + } |
| + if !isValidUTF8(got) { |
| + t.Error("capping produced invalid UTF-8 — the cut was by byte, not by rune") |
| + } |
| +} |
| + |
| +func TestNormaliseTitleLeavesShortTitlesAlone(t *testing.T) { |
| + in := strings.Repeat("a", maxTitleRunes) |
| + if got := normaliseTitle(in); got != in { |
| + t.Errorf("a title exactly at the cap was changed: %d runes in, %d out", len(in), len(got)) |
| + } |
| +} |
| + |
| +func isValidUTF8(s string) bool { |
| + for _, r := range s { |
| + if r == '�' { |
| + return false |
| + } |
| + } |
| + return true |
| +} |
diff --git a/internal/docs/title_test.go b/internal/docs/title_test.go| new file mode 100644 |
| index 0000000..9f5fe64 |
| --- /dev/null |
| +++ b/internal/docs/title_test.go |
| @@ -0,0 +1,291 @@ |
| +package docs_test |
| + |
| +// POST /d/{sqid}/title — renaming a document. |
| +// |
| +// The route is the only way a title can change after a document is |
| +// created, since the list header's title field is gone, so the two |
| +// things this file is really holding are that it WORKS with a plain |
| +// form post and that it cannot be used to rename somebody else's |
| +// document. |
| +// |
| +// One sign-in per test, reused across requests: the sign-in path is |
| +// rate limited, and a test that signs in per request starts getting |
| +// ?err=rate back partway through and then fails somewhere else |
| +// entirely. |
| + |
| +import ( |
| + "context" |
| + "encoding/json" |
| + "net/http" |
| + "net/http/httptest" |
| + "net/url" |
| + "strings" |
| + "testing" |
| + |
| + "github.com/carlosframework/docs/internal/docs" |
| +) |
| + |
| +// renamePost builds the request a browser's own form submit sends: |
| +// same-origin, form-encoded, with the session cookies. |
| +func renamePost(t *testing.T, cookies []*http.Cookie, sqid, title string, accept string) *http.Request { |
| + t.Helper() |
| + form := url.Values{} |
| + form.Set("title", title) |
| + req := httptest.NewRequest(http.MethodPost, "/d/"+sqid+"/title", strings.NewReader(form.Encode())) |
| + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| + req.Header.Set("Sec-Fetch-Site", "same-origin") |
| + if accept != "" { |
| + req.Header.Set("Accept", accept) |
| + } |
| + for _, c := range cookies { |
| + req.AddCookie(c) |
| + } |
| + return req |
| +} |
| + |
| +func TestRenameChangesTheTitle(t *testing.T) { |
| + app, _ := docs.NewForTest(t) |
| + ctx := context.Background() |
| + owner := app.TestPerson(t, "owner@example.com") |
| + doc, err := app.Store().Create(ctx, owner, "Untitled document") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + cookies := signInCookies(t, app, "owner@example.com") |
| + |
| + rec := httptest.NewRecorder() |
| + app.Handler().ServeHTTP(rec, renamePost(t, cookies, doc.Sqid, "Board minutes", "")) |
| + |
| + // 303 See Other, not 302: a reload of the response replays the GET |
| + // it points at, never the POST. |
| + if rec.Code != http.StatusSeeOther { |
| + t.Fatalf("status %d, want %d", rec.Code, http.StatusSeeOther) |
| + } |
| + if got := rec.Header().Get("Location"); got != "/d/"+doc.Sqid { |
| + t.Errorf("Location = %q, want %q", got, "/d/"+doc.Sqid) |
| + } |
| + |
| + after, err := app.Store().Get(ctx, owner, doc.Sqid) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if after.Title != "Board minutes" { |
| + t.Errorf("stored title = %q, want %q", after.Title, "Board minutes") |
| + } |
| +} |
| + |
| +// The whole point of the scriptless path: the new name has to be on |
| +// the page the redirect lands on, in the field and in <title>. |
| +func TestRenameShowsOnTheDocumentPage(t *testing.T) { |
| + app, _ := docs.NewForTest(t) |
| + ctx := context.Background() |
| + owner := app.TestPerson(t, "owner@example.com") |
| + doc, err := app.Store().Create(ctx, owner, "Untitled document") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + cookies := signInCookies(t, app, "owner@example.com") |
| + |
| + rec := httptest.NewRecorder() |
| + app.Handler().ServeHTTP(rec, renamePost(t, cookies, doc.Sqid, "Q3 plan", "")) |
| + if rec.Code != http.StatusSeeOther { |
| + t.Fatalf("rename status %d", rec.Code) |
| + } |
| + |
| + get := httptest.NewRequest(http.MethodGet, "/d/"+doc.Sqid, nil) |
| + for _, c := range cookies { |
| + get.AddCookie(c) |
| + } |
| + page := httptest.NewRecorder() |
| + app.Handler().ServeHTTP(page, get) |
| + if page.Code != http.StatusOK { |
| + t.Fatalf("show status %d", page.Code) |
| + } |
| + body := page.Body.String() |
| + if !strings.Contains(body, `value="Q3 plan"`) { |
| + t.Error("the renamed title is not in the title field") |
| + } |
| + if !strings.Contains(body, "<title>Q3 plan") { |
| + t.Error("the renamed title is not in the page title") |
| + } |
| +} |
| + |
| +// The JSON representation, which is what fixtures/title.mjs uses so a |
| +// rename does not navigate. It must answer with the STORED title: |
| +// title.go normalises, and the client shows whatever comes back. |
| +func TestRenameAnswersJSONWhenAsked(t *testing.T) { |
| + app, _ := docs.NewForTest(t) |
| + ctx := context.Background() |
| + owner := app.TestPerson(t, "owner@example.com") |
| + doc, err := app.Store().Create(ctx, owner, "Untitled document") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + cookies := signInCookies(t, app, "owner@example.com") |
| + |
| + rec := httptest.NewRecorder() |
| + app.Handler().ServeHTTP(rec, renamePost(t, cookies, doc.Sqid, " Notes again ", "application/json")) |
| + if rec.Code != http.StatusOK { |
| + t.Fatalf("status %d, want 200", rec.Code) |
| + } |
| + var got struct { |
| + Title string `json:"title"` |
| + } |
| + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { |
| + t.Fatalf("decode: %v (body %q)", err, rec.Body.String()) |
| + } |
| + if got.Title != "Notes again" { |
| + t.Errorf("title = %q, want the normalised %q", got.Title, "Notes again") |
| + } |
| +} |
| + |
| +// An empty title is not an empty title: it becomes the catalog's own |
| +// "Untitled document", the same fallback POST /documents uses. |
| +func TestRenameToNothingGetsTheUntitledFallback(t *testing.T) { |
| + app, _ := docs.NewForTest(t) |
| + ctx := context.Background() |
| + owner := app.TestPerson(t, "owner@example.com") |
| + doc, err := app.Store().Create(ctx, owner, "Board minutes") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + cookies := signInCookies(t, app, "owner@example.com") |
| + |
| + rec := httptest.NewRecorder() |
| + app.Handler().ServeHTTP(rec, renamePost(t, cookies, doc.Sqid, " ", "application/json")) |
| + if rec.Code != http.StatusOK { |
| + t.Fatalf("status %d", rec.Code) |
| + } |
| + var got struct { |
| + Title string `json:"title"` |
| + } |
| + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { |
| + t.Fatal(err) |
| + } |
| + if got.Title == "" || strings.TrimSpace(got.Title) == "" { |
| + t.Errorf("title = %q — an empty rename must fall back to the untitled copy", got.Title) |
| + } |
| +} |
| + |
| +// Spec §4.2: a document you cannot see is indistinguishable from one |
| +// that does not exist. Store.Rename scopes by owner in its WHERE, so |
| +// there is no branch here that could 403 by mistake — and this test is |
| +// what would notice if one appeared. |
| +func TestRenameSomebodyElsesDocumentIs404(t *testing.T) { |
| + app, _ := docs.NewForTest(t) |
| + ctx := context.Background() |
| + owner := app.TestPerson(t, "owner@example.com") |
| + doc, err := app.Store().Create(ctx, owner, "Private") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + stranger := signInCookies(t, app, "stranger@example.com") |
| + |
| + rec := httptest.NewRecorder() |
| + app.Handler().ServeHTTP(rec, renamePost(t, stranger, doc.Sqid, "Mine now", "")) |
| + if rec.Code != http.StatusNotFound { |
| + t.Fatalf("status %d, want 404 — a stranger's document must not be distinguishable from a missing one", rec.Code) |
| + } |
| + |
| + after, err := app.Store().Get(ctx, owner, doc.Sqid) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if after.Title != "Private" { |
| + t.Errorf("the document was renamed anyway: %q", after.Title) |
| + } |
| +} |
| + |
| +func TestRenameAnUnknownDocumentIs404(t *testing.T) { |
| + app, _ := docs.NewForTest(t) |
| + cookies := signInCookies(t, app, "owner@example.com") |
| + rec := httptest.NewRecorder() |
| + app.Handler().ServeHTTP(rec, renamePost(t, cookies, "nosuchdoc", "Anything", "")) |
| + if rec.Code != http.StatusNotFound { |
| + t.Fatalf("status %d, want 404", rec.Code) |
| + } |
| +} |
| + |
| +// csrf.Protect is mounted above every route (app.go). A cross-site POST |
| +// must not rename anything — this is the check that the new route sits |
| +// inside that protection rather than beside it. |
| +func TestRenameRefusesACrossSitePost(t *testing.T) { |
| + app, _ := docs.NewForTest(t) |
| + ctx := context.Background() |
| + owner := app.TestPerson(t, "owner@example.com") |
| + doc, err := app.Store().Create(ctx, owner, "Board minutes") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + cookies := signInCookies(t, app, "owner@example.com") |
| + |
| + form := url.Values{} |
| + form.Set("title", "Renamed from elsewhere") |
| + req := httptest.NewRequest(http.MethodPost, "/d/"+doc.Sqid+"/title", strings.NewReader(form.Encode())) |
| + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| + req.Header.Set("Sec-Fetch-Site", "cross-site") |
| + req.Header.Set("Origin", "https://evil.example") |
| + for _, c := range cookies { |
| + req.AddCookie(c) |
| + } |
| + rec := httptest.NewRecorder() |
| + app.Handler().ServeHTTP(rec, req) |
| + |
| + if rec.Code == http.StatusSeeOther || rec.Code == http.StatusOK { |
| + t.Fatalf("a cross-site rename was accepted (status %d)", rec.Code) |
| + } |
| + after, err := app.Store().Get(ctx, owner, doc.Sqid) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if after.Title != "Board minutes" { |
| + t.Errorf("the document was renamed cross-site: %q", after.Title) |
| + } |
| +} |
| + |
| +// Renaming is touching: the list orders by UpdatedAt and groups by it, |
| +// so a document you just named has to be where everything else you just |
| +// did is. |
| +func TestRenameBumpsUpdatedAt(t *testing.T) { |
| + app, _ := docs.NewForTest(t) |
| + ctx := context.Background() |
| + owner := app.TestPerson(t, "owner@example.com") |
| + doc, err := app.Store().Create(ctx, owner, "One") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := app.Store().Create(ctx, owner, "Two"); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + list, err := app.Store().List(ctx, owner) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(list) != 2 || list[0].Title != "Two" { |
| + t.Fatalf("setup: expected Two newest first, got %v", titlesOf(list)) |
| + } |
| + |
| + renamed, err := app.Store().Rename(ctx, owner, doc.Sqid, "One, renamed") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if !renamed.UpdatedAt.After(doc.UpdatedAt) { |
| + t.Errorf("UpdatedAt did not move: %v -> %v", doc.UpdatedAt, renamed.UpdatedAt) |
| + } |
| + list, err = app.Store().List(ctx, owner) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if list[0].Title != "One, renamed" { |
| + t.Errorf("the renamed document is not first: %v", titlesOf(list)) |
| + } |
| +} |
| + |
| +func titlesOf(docs []*docs.Document) []string { |
| + out := make([]string, 0, len(docs)) |
| + for _, d := range docs { |
| + out = append(out, d.Title) |
| + } |
| + return out |
| +} |
diff --git a/internal/docs/views/list.html b/internal/docs/views/list.html| index 929e38a..afc5567 100644 |
| --- a/internal/docs/views/list.html |
| +++ b/internal/docs/views/list.html |
| @@ -121,103 +121,60 @@ |
| align-items: center; |
| gap: var(--rst-sp-3); |
| } |
| - /* [docs-create-form]: the title field and its submit button as one |
| - inline row — nowrap, deliberately, and this is the part that took |
| - two attempts. |
| - A wrapping flex line does not shrink its items to fit; it breaks |
| - first and shrinks only what still does not fit afterwards. So |
| - with wrap on, the button dropped to a second line and the field — |
| - now alone on the first one, with room to grow — sprang back to |
| - its 14rem maximum, which is how a fix aimed at making the header |
| - shorter measured exactly as tall as before it (175px, unchanged). |
| - nowrap is what forces the field to give up width to its button |
| - instead of the button giving up the line. |
| - The button itself is flex: none so the whole shortfall comes out |
| - of the field. Shared proportionally, both would shrink, and |
| - "New document" would start wrapping or clipping inside a control |
| - that had no reason to be narrower. |
| - One thing to watch when the other eleven catalogs land: nowrap |
| - plus a non-shrinking button means a locale whose |
| - docs.list.create.label is wider than about 270px overflows the |
| - 320px column rather than wrapping out of it. English is 131px and |
| - is the only catalog today, so this is a note rather than a bug — |
| - but it is a real edge, and the fix when it arrives is a |
| - wrap allowance keyed to the label, not a smaller field. */ |
| + /* [docs-create-form] holds one button and needs no layout rule of |
| + its own beyond not stretching. It used to hold a title field |
| + beside that button and carried thirty lines about nowrap, |
| + flex-basis and which item gives up width first; all of that was |
| + about a field that is gone, and none of it is true of a button |
| + that sizes to its label. The history is worth one sentence rather |
| + than the thirty: a fixed-width field and a non-shrinking button |
| + could not share a 288px column, the field would not shrink |
| + because a flex item's automatic minimum is its content, and the |
| + header stood 175px tall at 320px as a result. A lone button has |
| + no such floor. |
| + The one thing that DID survive from that comment is the locale |
| + caveat, and it survived in weaker form: docs.list.create.label is |
| + the only thing in this form now, so a wide catalog makes the |
| + button wide and the header wraps to a second line, which is what |
| + [docs-header-actions]'s flex-wrap is for. Nothing overflows. */ |
| [docs-create-form] { |
| display: flex; |
| - flex-wrap: nowrap; |
| - gap: var(--rst-sp-2); |
| - } |
| - [docs-create-form] button { |
| flex: none; |
| } |
| - /* The title field shares its line with the submit button instead of |
| - holding a fixed 14rem and pushing it onto a line of its own. |
| - Measured at 320px — the reflow width spec §10.3 commits to — |
| - before this changed: a fixed 14rem (224px) field could not sit |
| - beside a 131px button in a 288px content column, so the field, |
| - "New document" and "Sign out" each took a full row and the header |
| - stood 175px tall, putting the first document 223px down a 720px |
| - viewport. Just under a third of the screen spent on chrome before |
| - a single thing a person came here to read. |
| - flex-basis 10rem with room to shrink is what lets the field and |
| - its button share one line in that column; the 14rem cap keeps the |
| - desktop header exactly as it was, since the form still sizes to |
| - its max-content there. min-inline-size: 0 is load-bearing — a |
| - flex item's automatic minimum size is its content, so without it |
| - an <input> refuses to shrink below its default size and the row |
| - overflows rather than fitting. */ |
| - [docs-create-form] [rst-input] { |
| - flex: 1 1 10rem; |
| - min-inline-size: 0; |
| - max-inline-size: 14rem; |
| - } |
| - /* [docs-search-form] is the same inline row as the create form, with |
| - one difference: it does NOT wrap. The header carries several |
| - controls, and with wrapping allowed the search button dropped onto |
| - a line of its own under its own field while the create form stayed |
| - whole — which reads as two rows of unrelated chrome rather than as |
| - one field and its button. |
| - |
| - Staying on one row is only safe because the FIELD shrinks, and |
| - that is the part this rule got wrong for a while. It previously |
| - carried a comment asserting that the group wrapping at |
| - [docs-header-actions] meant "a 320px viewport puts each form on |
| - its own line intact rather than breaking one of them in half". |
| - That was never measured and was wrong. The field held a fixed |
| - 11rem, its button holds a label that does not wrap, and |
| - max-inline-size: 100% resolves against a form whose own width is |
| - its content — so the form set a min-content floor of about 334px |
| - and simply stood wider than the 288px column. Measured on |
| - origin/main at 7507a33 with scripts/browser-drive.sh: the list |
| - page scrolled sideways at 320px, scrollWidth 356 against a 320 |
| - viewport, with this form's button the innermost offender at |
| - x=356. |
| - |
| - That is a WCAG 2.2 SC 1.4.10 failure, which AGENTS.md allows no |
| - exemption from, traded for a desktop layout nicety by a comment |
| - that sounded like it had been checked. |
| + /* [docs-search-form] is now a field and nothing else — the submit |
| + button is gone (Paul: "the field is enough"), and with it the |
| + whole reason this rule was hard. |
| - Note what the floor did to its NEIGHBOUR. [docs-create-form] was |
| - measured over the edge too, at x=350, and it is correctly written |
| - — it shrinks exactly as it should. It overflowed because a |
| - wrapping flex container is at least as wide as its widest item's |
| - min-content, so this form's 334px floor widened every line in the |
| - group, including the one the create form had to itself. Fixing |
| - the field below fixes both; there was never a second bug. Worth |
| - knowing before someone "fixes" the create form as well. |
| + What the button was for, and where each part of it went, because |
| + removing a control that had two justifications should not lose |
| + both. It was there for a pointer, and for a screen reader: its |
| + label was also the field's accessible name and the two agreeing |
| + was worth the width. The accessible name is untouched — it is |
| + still docs.list.search.label, still "Search documents", now on |
| + the input's own aria-label where it always also was. What a |
| + pointer lost is a click target that submitted; what it gained is |
| + that it no longer needs one, because fixtures/live-search.mjs |
| + updates the list as the field is typed into. With scripts off, |
| + Enter in a single-input form submits it natively — which is not |
| + a fallback added for this change but the behaviour a lone text |
| + input in a GET form has always had, and is what made the button |
| + optional in the first place. |
| - The fix is that the field takes what is left after the button |
| - rather than claiming a width: flex 1 1 with a small basis, and |
| - min-inline-size 0 to lift a flex item's automatic minimum, which |
| - is its content size and is what an <input> otherwise refuses to |
| - go below. The 11rem cap keeps the desktop header exactly as it |
| - was. */ |
| + The rule that remains is the interesting half of the old one. |
| + flex: 1 1 with min-inline-size: 0 was written to make the field |
| + give up width to a button that would not shrink; with no button |
| + it is what stops the field claiming a fixed width in a narrow |
| + column, which is the same 320px reflow failure (WCAG 2.2 SC |
| + 1.4.10) measured on main at 7507a33 — scrollWidth 356 against a |
| + 320 viewport, this form's own contents the innermost offender. |
| + An <input> refuses to go below its content size without |
| + min-inline-size: 0, so that line is load-bearing whether or not |
| + anything sits beside it. The 11rem cap keeps the desktop header |
| + the width it has been throughout. */ |
| [docs-search-form] { |
| display: flex; |
| flex-wrap: nowrap; |
| align-items: center; |
| - gap: var(--rst-sp-2); |
| max-inline-size: 100%; |
| } |
| [docs-search-form] [rst-input] { |
| @@ -225,32 +182,27 @@ |
| min-inline-size: 0; |
| max-inline-size: 11rem; |
| } |
| - /* The submit button is for a pointer and for a screen reader; a |
| - keyboard user submits with Enter from the field, which a |
| - single-input form does natively. It stays visible rather than |
| - becoming an icon because "Search documents" is also the field's |
| - accessible name and the two agreeing is worth more than the |
| - width. |
| - flex: 0 0 auto for the same reason: the whole shortfall comes out |
| - of the field, so the button keeps its whole label. It is the |
| - words that are load-bearing here, not the box. |
| - The locale caveat, stated as the sequence it actually is rather |
| - than as a single threshold. Because the field gives way first and |
| - the button never shrinks, a wider label eats the FIELD long |
| - before anything overflows: a ~200px label leaves about 80px of |
| - field, ~270px leaves a sliver, and only past ~280px — the 288px |
| - column less the gap — does anything cross the edge. So the first |
| - failure a wide catalog produces is a search box too narrow to |
| - read what you typed, which is a usability problem and not a |
| - 1.4.10 one; the overflow comes later. English is 131px and is the |
| - only catalog today. The fix when it arrives is a wrap allowance |
| - keyed to the label, not a smaller field. |
| - ([docs-create-form]'s own comment above states the same caveat as |
| - a bare "wider than about 270px overflows", which is the second |
| - half of this sequence and not the part that bites first.) */ |
| - [docs-search-form] button { |
| - white-space: nowrap; |
| - flex: 0 0 auto; |
| + /* The results region dims while a live query is in flight rather |
| + than emptying or showing a spinner. Emptying it would make every |
| + keystroke flash the list out of existence; a spinner is a second |
| + thing to look at while the answer is arriving in well under the |
| + time it takes to notice one. Dimming says "this is the previous |
| + answer" without moving anything, and the previous answer is |
| + usually a superset of the next one anyway, since typing narrows. |
| + No transition on the way IN, a short one on the way out: arriving |
| + at the dimmed state instantly is honest about the request having |
| + started, and fading back is what stops a fast response reading as |
| + a flicker. */ |
| + [docs-results][aria-busy="true"] { |
| + opacity: 0.55; |
| + } |
| + [docs-results] { |
| + transition: opacity 120ms ease-out; |
| + } |
| + @media (prefers-reduced-motion: reduce) { |
| + [docs-results] { |
| + transition: none; |
| + } |
| } |
| /* The way back out of a result page, under the results rather than |
| in the header: it is about the list being shown, not about the |
| @@ -275,13 +227,29 @@ |
| own clear control and its own keyboard on a phone. The value |
| is echoed back so a result page can be edited and |
| re-submitted rather than retyped. */}} |
| - <form docs-search-form method="get" action="/" role="search"> |
| + <form docs-search-form method="get" action="/" role="search" |
| + data-docs-results="[docs-results]"> |
| <input rst-input docs-search-input type="search" name="q" value="{{.Query}}" |
| placeholder="{{.SearchPlace}}" aria-label="{{.SearchLabel}}" autocomplete="off"> |
| - <button rst-btn="ghost" type="submit">{{.SearchLabel}}</button> |
| </form> |
| + {{/* The create form is a button and nothing else. A new document |
| + is named in the document, on its own title (show.html's |
| + [docs-title-form]), which is where a person is looking when |
| + they decide what it is called — and it is the only place the |
| + name can be changed later, so a create field would have been |
| + the one control in the app that could set a title but never |
| + correct one. |
| + The field also cost the header its shape: it and its button |
| + could not share a 288px column, which is the whole reason |
| + [docs-create-form] below carries a comment about nowrap and |
| + flex-basis. With the field gone the button is just a button |
| + and none of that applies to it. |
| + POST /documents with no `title` at all, deliberately, rather |
| + than with an empty one: handlers.go's create trims the field |
| + and falls back to docs.document.untitled either way, so the |
| + absent field and the empty field are already the same request |
| + to the server. */}} |
| <form docs-create-form method="post" action="/documents"> |
| - <input rst-input type="text" name="title" placeholder="{{.CreatePlaceholder}}" aria-label="{{.CreateLabel}}" autocomplete="off"> |
| <button rst-btn="primary" type="submit">{{.CreateLabel}}</button> |
| </form> |
| </div> |
| @@ -290,6 +258,28 @@ |
| grouping is what an unfiltered list is for; grouping ranked |
| results under "Today" and "Earlier" would order the same rows |
| twice by two different rules and show only the second. */}} |
| +{{/* [docs-results] is everything below the header that a query |
| + changes, and it exists so fixtures/live-search.mjs can replace it |
| + wholesale with the same region from GET /?q=… — the server's own |
| + rendering, fetched rather than re-implemented. |
| + |
| + That is the design decision worth stating, because the obvious |
| + alternative is worse in a specific way. GET /documents/search |
| + already returns JSON, and the palette (fixtures/jump-to.mjs) uses |
| + it; building the live list from that would mean the bundle |
| + carrying its own copy of "No documents match", of the recency |
| + group labels, and of the relative-time strings — twelve locales' |
| + worth of copy in a JavaScript file, which AGENTS.md forbids for |
| + good reason. Fetching this region instead means the live results |
| + and the submitted results are not merely consistent, they are the |
| + same bytes from the same handler. |
| + |
| + aria-live="polite" because the results change without the page |
| + navigating and without focus moving; a person typing here needs |
| + to be told the list beneath them is different now. The region is |
| + only announced when it CHANGES, so the first render on page load |
| + says nothing. */}} |
| +<div docs-results aria-live="polite"> |
| {{if .Searching}} |
| {{if .Results}} |
| <div rst-list> |
| @@ -327,5 +317,6 @@ |
| <p rst-empty-body>{{.EmptyBody}}</p> |
| </div> |
| {{end}} |
| +</div> |
| {{template "docs-jumpto" .}} |
| {{end}} |
diff --git a/internal/docs/views/show.html b/internal/docs/views/show.html| index 3f4b6c0..7ed44dd 100644 |
| --- a/internal/docs/views/show.html |
| +++ b/internal/docs/views/show.html |
| @@ -56,8 +56,33 @@ |
| The title is printed as-is, the same way list.html's row titles |
| are — it is the document's own data, not authored copy, so it |
| - never goes through rastrillo.T. */}} |
| -{{define "brand"}}<a rst-shell-brand href="/">{{.BackLabel}}</a><span docs-shell-title>{{.Title}}</span>{{end}} |
| + never goes through rastrillo.T. It is printed into an ATTRIBUTE |
| + here rather than as text, which html/template escapes for that |
| + context specifically; title.go strips control characters on the |
| + way in, which is a layout rule and not the thing that makes this |
| + safe. |
| + |
| + A FORM AND A FIELD, not a span. Paul: "'Untitled document' should |
| + be editable in place". Until this change a title was set once, by |
| + a field on the list header, and could never be corrected — the |
| + screenshot that prompted this is someone double-clicking the span |
| + and getting a text selection. |
| + |
| + It is a real <form method="post"> around a real <input>, so |
| + renaming works with no JavaScript at all: type, and the submit |
| + button that [docs-title-form]:focus-within reveals posts to |
| + handlers.go's rename, which 303s back here. fixtures/title.mjs |
| + makes it feel like a title bar rather than a form — Enter and |
| + blur save without navigating, Escape reverts — and it is an |
| + addition, never the mechanism. |
| + |
| + maxlength is the server's own cap (title.go's maxTitleRunes) |
| + restated for the field. Not a duplicated rule so much as a hint: |
| + maxlength counts UTF-16 code units and the server counts runes, so |
| + the two disagree above the astral plane and the server is the one |
| + that decides. What it buys is that the ordinary overlong paste is |
| + stopped where a person can see it happen. */}} |
| +{{define "brand"}}<a rst-shell-brand href="/">{{.BackLabel}}</a><form docs-title-form method="post" action="/d/{{.Sqid}}/title"><input rst-input docs-title-input type="text" name="title" value="{{.Title}}" aria-label="{{.TitleLabel}}" maxlength="200" autocomplete="off" spellcheck="false"><button docs-title-save rst-btn="ghost" type="submit">{{.TitleSave}}</button></form>{{end}} |
| {{/* account is the dropdown's MENU BODY, not the whole menu: the |
| shell owns the <details>/<summary> and this block supplies only |
| what sits inside [rst-dropdown-menu]. A <form> rather than a |
| @@ -99,17 +124,23 @@ |
| max-inline-size: none; |
| padding: 0; |
| } |
| - /* [docs-shell-title] is the document's own name in the shell bar, |
| - beside the brand link back to the list — the pattern both |
| - reference applications use, and the reason this page no longer |
| - carries a [rst-page-header] of its own (see render.go's |
| - shellLayout comment for the whole argument). |
| + /* [docs-title-form] and [docs-title-input] are the document's own |
| + name in the shell bar, beside the brand link back to the list — |
| + the pattern both reference applications use, and the reason this |
| + page no longer carries a [rst-page-header] of its own (see |
| + render.go's shellLayout comment for the whole argument). |
| - It is a <span>, not a heading: the document's title already IS a |
| + It was a [docs-shell-title] span until the title became editable; |
| + the layout reasoning below is that span's and survives the change |
| + intact, because the form took its exact place in the bar. |
| + |
| + Not a heading, then or now: the document's title already IS a |
| heading in the only place that counts, <title>, and a visible h1 |
| that is not the document's first line was the thing making the |
| old header read as a screen heading rather than a document's |
| - name. |
| + name. A labelled field is if anything further from a heading than |
| + the span was, which is correct — this names the document, it does |
| + not head the page. |
| flex: 1 1 0, and the zero basis is the whole point rather than a |
| tidy default. [rst-shell-bar] wraps (tokens.css), and a flex |
| @@ -125,20 +156,104 @@ |
| an ellipsis when there is none — which is the behaviour this |
| paragraph used to claim and did not have. |
| - What that costs, knowingly: a truncated title is then readable |
| - nowhere on this screen — the span is not focusable and the |
| - document's own first line is not its title. It is still in the |
| - tab title, and in full on the list. A wrapping bar would keep it |
| - legible at the price of the chrome changing height as you open |
| - documents, which is the thing this whole branch is undoing. */ |
| - [docs-shell-title] { |
| + What that cost while this was a span: a truncated title was |
| + readable nowhere on this screen, because a span is not focusable |
| + and a document's own first line is not its title. A field is |
| + focusable — click it, or Tab to it, and the text scrolls — so the |
| + ellipsis is now a summary of something reachable rather than of |
| + something lost, and that paragraph has stopped being a known |
| + shortcoming. It is still in the tab title and in full on the list |
| + as well. A wrapping bar would keep it legible at the price of the |
| + chrome changing height as you open documents, which is the thing |
| + this whole branch is undoing. */ |
| + [docs-title-form] { |
| + display: flex; |
| flex: 1 1 0; |
| + align-items: center; |
| + gap: var(--rst-sp-2); |
| min-inline-size: 0; |
| - overflow: hidden; |
| - white-space: nowrap; |
| - text-overflow: ellipsis; |
| + margin: 0; |
| + } |
| + /* The field reads as a title and behaves as a field. Google Docs and |
| + every other editor with a nameable document do the same thing, and |
| + the reason is not fashion: a bordered input in a chrome bar reads |
| + as a search box, and people do not type a document's name into |
| + what looks like a search box. |
| + |
| + So: no background, no border, the bar's own type size — and a |
| + border that APPEARS on hover and on focus rather than one that |
| + disappears. [rst-input] is on the element for its box-sizing, |
| + font inheritance and focus ring; everything visual it brings is |
| + overridden here, which is the one place in this app that does |
| + that to an rst- component, because there is no rst- vocabulary |
| + for "a field that is not shaped like a field". |
| + |
| + The transparent border is 1px and always there, never 0 changing |
| + to 1: a border that appears on hover would move every character |
| + of the title one pixel sideways as the pointer crosses it. |
| + |
| + Not a placeholder-only affordance either. An empty title cannot |
| + happen — handlers.go's rename substitutes docs.document.untitled |
| + for anything that normalises to nothing — so there is never a |
| + moment when this field is blank and needs a hint. What tells you |
| + it is editable is the hover border and the pointer's own I-beam, |
| + plus its accessible name, which says so in words. */ |
| + [docs-title-input] { |
| + min-inline-size: 0; |
| + padding: 0.1rem 0.3rem; |
| + border: 1px solid transparent; |
| + background: transparent; |
| color: var(--rst-text); |
| + font: inherit; |
| font-size: var(--rst-fs-sm); |
| + text-overflow: ellipsis; |
| + } |
| + [docs-title-input]:hover { |
| + border-color: var(--rst-line); |
| + } |
| + [docs-title-input]:focus { |
| + border-color: var(--rst-line-strong); |
| + background: var(--rst-surface); |
| + } |
| + /* The submit button exists for the person with no JavaScript, and |
| + appears when the form has focus — which is exactly when they are |
| + renaming and exactly when they need it. |
| + |
| + :focus-within, so it is revealed by the FIELD taking focus and |
| + stays revealed while the button itself has it. Tabbing from the |
| + field would otherwise move focus onto a control that vanished as |
| + it arrived, which is a keyboard trap in the shape of a tidy |
| + header. It is display: none until then rather than visually |
| + hidden: a hidden-but-present button is still a tab stop between |
| + the title and the account menu on every page load, for a control |
| + nobody is using. |
| + |
| + With fixtures/title.mjs loaded it is a third way to do what Enter |
| + and blur already do, and stays for that reason rather than being |
| + hidden by script: the same control, whether or not the script |
| + arrived. */ |
| + [docs-title-save] { |
| + display: none; |
| + flex: none; |
| + white-space: nowrap; |
| + } |
| + [docs-title-form]:focus-within [docs-title-save] { |
| + display: inline-flex; |
| + } |
| + /* A rename the server refused. The field keeps what was typed — |
| + losing it would be worse than showing a name that is not stored |
| + yet — and says so; fixtures/title.mjs falls back to submitting the |
| + form for real, so this state is what a person sees for the moment |
| + between a failed fetch and the navigation that replaces it. */ |
| + [docs-title-input][aria-invalid='true'] { |
| + /* --rst-tone-negative-fg, which is the token rastrillo actually |
| + has; .rst-danger is a class that sets colour FROM it. Written |
| + with no fallback on purpose: a var() fallback here would have |
| + silently painted a hardcoded hex if the token name were wrong, |
| + and a wrong token that looks right is the failure this app keeps |
| + finding. Without one, a typo is a border that does not change, |
| + which is visible. */ |
| + border-color: var(--rst-tone-negative-fg); |
| } |
| /* [docs-canvas] is the tray the page sits in — it is chrome around |
| paper, so it stays close to the surrounding theme's tone, but it |
| @@ -946,6 +1061,33 @@ |
| align-items: center; |
| gap: var(--rst-sp-3); |
| inline-size: 100%; |
| + /* THE FILE MENU SCROLLED SIDEWAYS WITHOUT THIS, and the cause is |
| + worth naming precisely because the symptom pointed at the wrong |
| + rule. |
| + rastrillo's tokens.css deliberately has no `* { box-sizing }` |
| + reset — it says so in its own comment, and gives the reason: the |
| + file promises exactly one collision surface with an app's own |
| + CSS, the rst- prefix. So [docs-menu-item] is content-box, and |
| + `inline-size: 100%` plus `padding: 0 var(--rst-sp-3)` is the |
| + panel's width PLUS 24px. Every item overflowed its panel by its |
| + own padding. |
| + It showed up on File first, and only on File, which is what |
| + makes the bug look like something else. A <button> gets |
| + box-sizing: border-box from the browser's own UA stylesheet; |
| + an <a> does not. File is the one menu that is mostly links — |
| + Open a document, and the two Download items — so it was the one |
| + menu wide enough to need a scrollbar. Edit, Format and Tools are |
| + all buttons and looked fine while being just as wrong. |
| + And the scrollbar is horizontal because of the OTHER rule above: |
| + [docs-menu-items] sets overflow-y: auto, and per CSS a computed |
| + overflow of anything but `visible` on one axis forces the other |
| + to compute to `auto` rather than staying visible. So the panel |
| + grew a horizontal scrollbar it never asked for, for content that |
| + was only overflowing because of a padding box. |
| + Fixing box-sizing fixes the cause. Nothing here sets |
| + overflow-x: hidden, on purpose: that would have hidden this and |
| + would hide the next one. */ |
| + box-sizing: border-box; |
| min-block-size: 28px; |
| padding: 0 var(--rst-sp-3); |
| border: 0; |
| @@ -1298,7 +1440,8 @@ |
| to the brand slot keeps that fixed and makes it the same control |
| on every screen rather than one this page draws for itself. |
| - The title is [docs-shell-title], beside that brand link. |
| + The title is [docs-title-form]'s field, beside that brand |
| + link — editable in place (title.go, fixtures/title.mjs). |
| Sign-out (POST /signout, auth.go) is in the bar's account menu, |
| which is where a 30-day session belongs and is now the one place |