Skip to content

feat(notes): save a note from its editor - #180

Merged
Bakhomious merged 3 commits into
mainfrom
step/note-save
Oct 30, 2026
Merged

Bakhomious merged 3 commits into
mainfrom
step/note-save

Conversation

@Bakhomious

@Bakhomious Bakhomious commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Brief

Goal. The note editor saves. Save, or ⌘S, sends the note with PUT /api/notes/files/<path> and If-Match naming the version the page loaded, which is the note's ETag (decisions 400 and 402). A saved note shows at once. A note that has changed since it loaded gets a 412: the editor keeps the user's text and says so.

Scope.

  • api.ts:
    • fetchNote returns the note's Markdown and its version, from the ETag.
    • saveNote sends the PUT with If-Match and text/markdown; charset=utf-8. A 204 gives the new version, a 412 gives undefined, and any other answer throws.
  • ShownNote:
    • It takes the editor's text after each change, and keeps the save's status: idle, saving, saved, changed or failed.
    • save().
    • An attachment for the Edit button, so a save can give it the focus.
  • editor.ts:
    • ⌘S, or Ctrl+S, saves.
    • The editor takes no change while a save runs.
    • Each change goes to the page.
  • NotePage:
    • While the editor is open, Save follows Cancel. Both are aria-disabled while a save runs.
    • A status line sits in the button row, like the sync's.
  • labels.ts: saveLabel.
  • @codemirror/state 6.7.6 becomes a direct dependency, for EditorState.changeFilter. The lockfile already had it, through the other CodeMirror packages.
  • Plan:
    • Decision 402 adds where Save sits, the status line, and why the note renders before the PUT.
    • The §0 row and the §8.2 line for note-save.

Out of scope.

  • Notes' PUT handler, its store and editable. They come with the store's step, which waits on §12's question.
  • Asking before Cancel, or leaving the page, drops the edits. That's note-unsaved, maybe later.
  • What the user can do about a 412 beyond keeping the text. That also comes with the store's step, under §3's conflict rules.

Acceptance criteria.

  1. Save sends the editor's text with PUT /api/notes/files/<path>, Content-Type: text/markdown; charset=utf-8, and If-Match naming the version the page loaded.
  2. On a 204, the page:
    • shows the saved note and says "Saved";
    • gives the focus to Edit;
    • sends the 204's ETag with the next save, which starts from the saved text.
  3. Save with the text unchanged, or with its changes undone, closes the editor and sends nothing.
  4. While a save runs:
    • the page says "Saving…";
    • Save and Cancel are aria-disabled and do nothing;
    • the editor takes no typing.
  5. A 412 keeps the editor and the text, and says "Not saved: the note has changed since it loaded". Any other failure, or an unreachable gateway, keeps them too and says "Save failed".
  6. ⌘S, or Ctrl+S, in the editor saves, and the browser doesn't save the page.
  7. Every gate passes with no limit raised.

QA

  • Criteria 1 to 3:
    • shown.test.ts, new, runs ShownNote against a stubbed fetch:
      • "saves the text if the note is still at its version, then shows it, says so and gives Edit the focus";
      • "Edit starts again from the saved text, and the next save sends the version the last one gave";
      • "only closes the editor with no change", and the same "with its changes undone".
    • api.test.ts checks the PUT's URL, method, headers and body, the 204's ETag, and a blank version when the 204 has none.
  • Criterion 4:
    • "Cancel and Save wait while a save runs" (shown.test.ts).
    • In NotePage.save.test.ts, new, a test types into the real CodeMirror during the save and finds the text unchanged. editor.test.ts checks that the editor takes no change while busy.
  • Criterion 5:
    • shown.test.ts covers the 412, a 500, and an unreachable gateway.
    • In api.test.ts:
      • A 412 gives no version.
      • 400, 428, 500, 502 and an unreachable gateway throw.
  • Criterion 6:
    • In editor.test.ts, Ctrl+S saves and its keydown's default is prevented, so the browser doesn't save the page. jsdom gives CodeMirror Ctrl, not ⌘.
    • The end-to-end "⌘S, or Ctrl+S, in the editor saves" runs on desktop. The phone project skips it, since a phone has no ⌘ or Ctrl without a keyboard.
  • End to end, on desktop (Chromium) and phone (iPhone 17, WebKit):
    • "Save sends the note with the version it loaded, then shows the note and says it saved" checks If-Match, the content type and the body.
    • "a note changed since it loaded keeps the editor and the text, and says so".
  • Mutation checks:
    • Ten mutations of the new logic, made by hand and reverted, each failed at least one test.
    • QA added a test for a 204 without an ETag, the one branch of the new code that no test took. A mutation of it now fails that test.
  • Live probes, through the real gateway (bin/e2e/gateway) and the real notes service (bin/notes) on a scratch vault. Only /api/notes was stubbed, to say editable. In both Chromium and WebKit:
    • The PUT passes the gateway's cross-origin protection. Notes, which has no PUT yet, answers 405 (a refused request would get 403), and the page says "Save failed".
    • If-Match is the ETag the GET gave, the note's SHA-256.
  • Keyboard: in Chromium, Shift-Tab from the editor reaches Save, then Cancel. WebKit, like Safari without full keyboard access, doesn't stop on buttons on Tab.
  • @codemirror/state 6.7.6 is the version the lockfile already had, and the newest on npm (2026-09-22).
  • Coverage, per lcov:
    • api.ts, editor.ts and labels.ts have every line, branch and function covered.
    • shown.svelte.ts has all 45 lines and 13 functions covered, and 18 of 19 branches. The one left is files ?? [] in save(), which only satisfies the type, since a loaded note always has the vault's list.

Review

No blocking findings. Non-blocking:

  • A 204 without an ETag leaves the version blank, so the next save sends an empty If-Match, which matches no version. The store's step should answer it as it answers a missing If-Match, with a 428. Notes always sends a note's ETag (decision 400).
  • Cancel still drops the edits without asking. note-unsaved may take that up.
  • After a 412, the user can only copy the text and Cancel. The store's step decides what else.

Gates

mise run check passed on aa7784c:

  • Go and repo:
    • fmt:check, gen:check, golangci-lint with 0 issues, jactionlint, the compose lint, and govulncheck with no vulnerabilities.
    • The Go tests, and go-test-coverage at 96.4% total.
  • Web:
    • Biome, plus svelte-check with 0 errors and 0 warnings.
    • tsc and knip.
    • Vitest: 1223 tests, 99.93% of lines.
    • pnpm audit.
    • Playwright: 386 passed, 8 skipped. The new skip is ⌘S on the phone.
  • Size: 139 added lines of code outside the tests, not counting blank lines and comments.

Screenshots

Demo data, a made-up note, Tokyo time. The phone shots come from WebKit.

Saving… The note changed (412) Saved
Desktop, light Riff in the editor on a desktop in light mode, saving, with Cancel and Save muted Riff in the editor on a desktop in light mode, saying the note has changed since it loaded The saved note Riff on a desktop in light mode, with Saved under its Edit button
Desktop, dark Riff in the editor on a desktop in dark mode, saving, with Cancel and Save muted Riff in the editor on a desktop in dark mode, saying the note has changed since it loaded The saved note Riff on a desktop in dark mode, with Saved under its Edit button
Phone, light Riff in the editor on a phone in light mode, saving, with Cancel and Save muted Riff in the editor on a phone in light mode, saying the note has changed since it loaded The saved note Riff on a phone in light mode, with Saved under its Edit button
Phone, dark Riff in the editor on a phone in dark mode, saving, with Cancel and Save muted Riff in the editor on a phone in dark mode, saying the note has changed since it loaded The saved note Riff on a phone in dark mode, with Saved under its Edit button

To be merged under the user's standing OK once CI's check is green on a head with origin/main merged in.

Save, or Cmd/Ctrl+S, sends the editor's text with PUT and If-Match
naming the version the page loaded, the note's ETag. A 204 shows the
saved note at once and says so; a 412, from a note changed since it
loaded, keeps the editor and the text and says so; any other failure
keeps them and says the save failed. While the save runs, the editor
takes no typing and Save and Cancel wait.

Decision 402 now also records where Save sits, the status line, and
why the note renders before the PUT.
@Bakhomious
Bakhomious merged commit 9914be8 into main Oct 30, 2026
2 checks passed
@Bakhomious
Bakhomious deleted the step/note-save branch October 30, 2026 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant