feat(staged): add a "Write note" action with a WYSIWYG markdown editor - #934
Open
matt2e wants to merge 14 commits into
Open
feat(staged): add a "Write note" action with a WYSIWYG markdown editor#934matt2e wants to merge 14 commits into
matt2e wants to merge 14 commits into
Conversation
Branch card footers get a `…` dropdown at the end of the left-aligned session buttons. It always holds a new "Write note" action, and absorbs the session buttons as the card narrows. Unlike "New note" (which launches an agent that writes a note), "Write note" opens a WYSIWYG markdown editor the user types into directly — both when creating the note and when clicking it in the timeline. Notes gain a nullable `subtype` column: NULL for session-produced notes, 'written' for user-authored ones. Session-less notes (drag-dropped files, saved action output) are backfilled to 'written', since they are exactly that class and become editable too. `update_note` refuses anything else, so an agent's note can't be overwritten out from under its session. Written notes have no separate title field — the leading H1 is the title, matching how `resolve_note_title_and_body` stores session notes. The overflow tier can't be a container query like the existing label tiers: the menu's content is portaled out of the `timeline` container, so button and menu both read a ResizeObserver width through the pure `computeFooterOverflow` in `footerOverflow.ts`. Review drops out first, then Commit, leaving Note last. The editor is Milkdown's Crepe, wrapped in `MarkdownWysiwygEditor.svelte` behind a value-in/`getMarkdown()`-out contract so it stays swappable, and lazily imported so ProseMirror and CodeMirror stay out of the main bundle. Its theme is remapped onto our `var(--*)` tokens. Thresholds are the plan's starting values and still want a visual pass on a real card; the nested DropdownMenu inside `TimelineContextMenu` also wants a manual right-click check. Signed-off-by: Matt Toohey <contact@matttoohey.com>
The bottom-left Note/Commit/Review buttons drop the dashed border-subtle outline for a solid border in each button's own theme color (--note-color/--commit-color/--review-color), so the resting state already carries the color the hover state fills in. The now-redundant hover border-color override goes away with it. The `…` overflow trigger read as disabled with its dashed border and faint text; it now uses the standard outline button variant at icon-sm (buttonVariants), matching the right-hand Diff/PR buttons, with the open state handled by the variant's aria-expanded styling. Signed-off-by: Matt Toohey <contact@matttoohey.com>
…tlining The solid theme-colored border from the previous commit read too loud at rest. The Note/Commit/Review buttons now carry no visible outline at all and instead sit on the faint 8%-alpha tint of their own icon color (--note-bg/--commit-bg/--review-bg), with hover stepping up to the matching 15% --*-bg-emphasis token. No explicit border class is needed to hold the 32px box: the shared buttonVariants base already applies `border border-transparent`, so removing the color leaves the metrics unchanged. The `…` overflow trigger keeps the neutral outline variant, since it is a generic affordance matched to the right-hand Diff/PR buttons rather than to a session type. Signed-off-by: Matt Toohey <contact@matttoohey.com>
Revert the Note/Commit/Review footer buttons to their pre-branch look: dashed border-subtle outline at rest, with hover swapping in the button's own theme border color, background tint, and text color. The two restyles on this branch (solid theme-colored border, then borderless 8%-alpha tint fill) are undone. The `…` overflow trigger keeps its buttonVariants outline icon-sm styling, which matches the right-hand Diff/PR buttons. Signed-off-by: Matt Toohey <contact@matttoohey.com>
The complaint about the "Write note" editor was Crepe's editor UI, not WYSIWYG editing itself. Those are separate layers: the live formatting — typing `# ` or `**bold**` and watching it take effect — comes from Milkdown's commonmark/GFM presets, which load unconditionally, while each widget is an independently disableable feature flag. So the flags go off: BlockEdit (per-block hover `+`/drag handle and the `/` menu), Toolbar (floating format bar on selection), ImageBlock, Table (cell handles and row/column buttons), and CodeMirror, whose language picker also drags in a hardcoded one-dark theme that ignores our tokens and clashes in light mode. Bold/italic stay reachable through markdown syntax and the preset keymaps, which are core rather than part of any feature. Left on are the quiet ones: ListItem for `- [ ]` checkboxes, LinkTooltip (the only way to edit an href without retyping the markdown), Placeholder, and Cursor. Disabled features take their node views with them, so tables and code blocks now render through the plain schema and need styling that Crepe's widget CSS used to provide. That folds into a typography pass: Crepe sizes its document for a standalone page editor (16px base, 2.6em h1, 60px/120px page margins), and the overrides here resize headings, paragraphs, lists, code, quotes, rules and tables to match NoteModal's `.markdown-content`, so writing a note looks close to reading one. The ListItem marker box in particular is hardcoded to 32px, which at our base font drops the bullet below its own first line. Crepe's `style.css` is still imported whole: every disabled feature's rules are scoped to class names that are now never emitted, so they are inert, and one import stays robust across upgrades. `--crepe-color-inline-code` is remapped from `--ui-danger` to `--text-primary`, matching NoteModal, which leaves inline code in body colour. This is the cheap, reversible half of the plan. If the calm surface still reads wrong, the problem is deeper than chrome and the next step is dropping Crepe for Milkdown core. Signed-off-by: Matt Toohey <contact@matttoohey.com>
…ten-note editor Three rough edges from typing in the "Write note" editor. Typing `[ ] ` on a plain line did nothing: the GFM preset's task rule only fires inside an existing list item (text typed after `- `), so the brackets stayed literal text, which the markdown serializer then escaped to `\[ ]` in the saved and copied note. A custom input rule (wysiwygPlugins.ts) now wraps the paragraph into a fresh task list on `[ ] `, `[x] `, and the lazier `[] ` — the last of which the preset also missed inside lists, so the rule covers that in place too. It imports from @milkdown/kit, Crepe's own customization surface, added as a direct dependency; pnpm resolves it onto the exact instance Crepe already uses, so nothing new ships. The placeholder rendered on every empty line the cursor visited, because Crepe's default mode decorates the current empty block. mode: 'doc' shows it only while the whole document is empty. The first line is the note's title on save — splitNoteMarkdown drafts it even without a heading — but the editor showed it as body text, and that fallback path duplicated the line on the next edit (noteMarkdownWithTitle prepends `# title` while the body still starts with the same line). An appendTransaction plugin now promotes a non-empty first paragraph to the title H1 as it is typed, so the title line looks like the title it becomes and round-trips on the H1 path. An empty first line stays a paragraph, keeping the doc-empty placeholder visible; a heading the user demoted with `## ` keeps its level; IME composition transactions are skipped and promoted on the next ordinary edit. Task syntax on the first line becomes title text by design — checkboxes start on line two — and the modal placeholder now says "the first line becomes its title". Both plugins are verified by integration tests that drive a real Milkdown editor (the same presets Crepe layers over) through handleTextInput, including that the converted item serializes as `* [ ] …` rather than escaped brackets. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
The previous commit taught the editor to convert `[ ] ` into a checkbox on a plain line, on the theory that the `\[ ]` in a saved note was a bug. It wasn't: GFM only defines task syntax inside a list item, so brackets on a bare line are literal text and escaping them on serialize is correct. The rule was inventing a non-standard shorthand — and its `[] ` spelling, which is not GFM at all — to fix something that already worked as specified. So it goes, along with its tests. Checkboxes are back to the preset's behaviour: type `- ` first, then `[ ] ` or `[x] `. The rest of that commit stands. The placeholder still shows only on an empty document, and the first non-empty line is still promoted to the title H1 as it is typed; that plugin is now the only thing `wysiwygPlugins` carries, and still the reason `@milkdown/kit` is a direct dependency. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
No, it wasn't a normal cursor. Crepe's Cursor feature bundles two unrelated things, and the chrome-stripping commit kept both after describing the feature as "drop / gap cursors" — which was only half of it. Alongside those it installs prosemirror-virtual-cursor, which sets `caret-color: transparent` to hide the real caret and redraws it as an absolutely positioned div with a 2px border. Everything the platform decides about a caret was therefore ignored: width (2px, not the 1px the OS asks for), blink rate and phase (a fixed 1s CSS keyframe with a 0.5s delay), the accent colour, and the reduce-motion setting — the animation has no `prefers-reduced-motion` guard. Colour was the loudest tell: Crepe paints the virtual caret with `--crepe-color-outline`, which our token remap points at `--border-muted`, so the caret was drawn in a faint *border* grey rather than a text colour. `virtual: false` turns off just that part. The drop cursor (drag position) and gap cursor (placing the cursor beside a block node like a table or rule) stay, since those have no native equivalent to defer to. Crepe's `.prosemirror-virtual-cursor` rules stay in the imported stylesheet and go inert, matching how the other disabled features are handled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ca839a9a4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Keyed so the editor remounts (and re-seeds its document) when the dialog | ||
| // opens on a different note rather than reusing the previous one's content. | ||
| let editorKey = $derived(open ? (note?.id ?? 'new') : null); | ||
| let initialMarkdown = $derived(note ? noteMarkdownWithTitle(note.title, note.content) : ''); |
There was a problem hiding this comment.
When a written note contains another H1 immediately after its title (for example # Title\n\n# Section), splitNoteMarkdown stores Title separately and leaves # Section at the start of the body. Reopening it here passes that body to noteMarkdownWithTitle, whose existing-H1 branch returns only the body and drops the stored title; saving then permanently replaces the title with Section. The editor must always restore the separately stored title without treating a body H1 as that title.
Useful? React with 👍 / 👎.
Comment on lines
+276
to
+279
| conn.execute( | ||
| "UPDATE notes SET title = ?1, content = ?2, updated_at = ?3, completed_at = ?4 WHERE id = ?5", | ||
| params![title, content, now, completed_at, id], | ||
| )?; |
There was a problem hiding this comment.
When a note is edited while the same branch is open in another window, this mutation performs the SQL update but never publishes StoreChange::Notes. The explicit invalidation in BranchCard.svelte only dispatches a DOM event in the originating window, while cacheInvalidationListener.ts depends on the backend notes-changed event to invalidate every window, so other views retain stale title/content until an unrelated refresh. Publish a notes change for existing.branch_id after the update, as the other note mutators do.
Useful? React with 👍 / 👎.
Clicking Save made the footer visibly glitch: swapping the button label from "Save" to a spinner plus "Saving..." widened the button, and with the footer right-aligned that shoved Cancel leftward. On a fast save the finally block then flipped saving back off just as onClose() started the dialog's fade-out, snapping both buttons back mid-animation — two reflows a frame apart that read as the buttons overlapping. The saving state is now width-stable: the spinner is absolutely positioned over the label, which goes visibility-hidden but keeps its box, so the button never changes size and Cancel never moves. The "Saving..." text goes away; aria-busy carries the state for assistive tech instead. The snap-back is gone too: success no longer resets saving in a finally block, and the re-seed effect skips the closed state (editorKey null), so the spinner persists through the close animation. The effect still runs on every open — closed→open always changes the key — which is where the stale draft, error, and saving flags now get cleared. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Four fixes from the review of this branch. `update_written_note` was the only note mutation in the store that didn't publish `StoreChange::Notes`, so the `notes-changed` event other views invalidate their timeline caches on never fired for a written-note edit. The saving card refreshed itself, but a second window — or the browser view against the same store — kept showing the old title and content until something unrelated woke it up. The branch id is already in hand from the row we just read, so it publishes like its siblings. Cmd+Enter could silently no-op. The shortcut gated on `canSave`, which reads the `markdown` state Milkdown updates on a debounce; type a short note and save inside that window and the guard saw an empty document. It now gates on `saving` alone and checks emptiness against `editor.getMarkdown()` — the same source of truth the content already came from. `canSave` stays as the Save button's disabled state. The title round-trip duplicated a line. `splitNoteMarkdown`'s no-H1 fallback derives the title from the first line but keeps the whole document as the body, since that line is real content — a list item, a deeper heading. `noteMarkdownWithTitle` then saw no leading H1 and prepended `# title`, so reopening the note showed the title line twice, every time. It now also returns the content untouched when the content's own first line is the title that was derived from it. Two paths reach that shape, and the fix above only covers the second. For the first — a `## Sub` heading on line one — the editor plugin now re-levels it to H1 as it's typed rather than leaving it, per the note on this task that auto-converting the first line is fine. That keeps the saved markdown on the H1 path where the title and body actually separate. Documents opening with a list or a code fence are still left alone: there is no in-place promotion that wouldn't mangle them, and `noteMarkdownWithTitle` now handles that shape. Last, the footer's `…` menu rendered only when `onWriteNote` was set, while the buttons it absorbs hide unconditionally at narrow widths. No caller hits that today, but a future one passing only `onNewNote` would lose the button with nothing to recover it from. The trigger now renders whenever something overflowed, with "Write note" as the conditional part. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
… for an H1 The title is its own column, and the stored content is the body with the title line already taken out of it — that is what `extract_note_title` does on the Rust side, and both the viewer and the editor put the H1 back with `noteMarkdownWithTitle`. Detecting a leading H1 in the body and skipping the prepend, as that function did, breaks the moment a body legitimately opens with a heading of its own: the note is shown under the wrong title, and in the editor the next save reads that heading as the title and overwrites the stored one. The previous commit widened that check rather than removing it, which was the wrong direction. So the detection goes, and the two functions become exact inverses. `noteMarkdownWithTitle` always prepends `# title` (an empty title is still a session stub with nothing to prepend). `splitNoteMarkdown` always takes the first non-empty line as the title and the rest as the body, on one path instead of an H1 path and a fallback path. Two H1s at the top of a document are now just a title and a section heading, and they survive a round trip. The only thing still read off that line is the heading marker, stripped so the title is stored as text rather than as `## Overview` — which is also why the editor re-levels a deeper first-line heading as it is typed: the `#`s come off on save either way, and doing it live keeps what is on screen equal to what is stored. Title clipping goes with it. It only made sense while the body kept the title line; now that the line moves into the title column, truncating at 80 characters would delete the rest of it on every save. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
The first line becomes the note's title, and the title is stored as plain text and shown as a single line in the timeline. So a first line whose meaning is its markup has nothing to give it: an image has no text at all, a link and a bullet and a table row all read there as raw syntax. Such a line is no longer taken as the title. It stays in the body — it is content, not a heading the user typed for us — and the note is titled "Untitled note". Reopening puts a real title line above it, and the next save reads that line instead, so nothing is lost or duplicated on the way through. `canBeNoteTitle` is the rule: lists (bulleted, numbered, task), quotes, fences, table rows, rules and raw HTML, plus a link or an image anywhere in the line, including a bare URL. Headings still qualify, since `#` is the title's own syntax and comes off; emphasis and inline code do too, being decoration on text that still reads as a title. The editor applies the same rule, because otherwise it would contradict it. An image on its own line is a *paragraph* in ProseMirror, so the title plugin used to promote it and produce `# ` — a heading that the save path then declines to use, leaving the image walled inside an H1 in the body. Now such a block is left as a paragraph, and a title that stops qualifying — a link pasted into it — is demoted back to one. What reads as a title on screen is what gets stored as the title. The two halves see different things: the editor holds parsed nodes, where a link is a mark and an image is a node, while the save path sees a serialized line. They share the text-level rule, which is also why it strips backslash escapes first — Milkdown writes a typed-out URL as `https\://example.com`, and the line the reader sees is the one that matters. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
A note is stored as a title column plus a body with that title line taken out of it, and `noteMarkdownWithTitle` puts the line back unconditionally — so every writer has to hand over a body the title has left. The drag-drop writer didn't: it stored the file name as the title and the file whole as the content, so a dropped `README.md` opening with `# Project` was shown under two headings, its name and its own. Removing the leading-H1 check from `noteMarkdownWithTitle` is what surfaced that; the check had been hiding it. So the drop path goes through `splitNoteMarkdown`, the same split the editor saves through, which now takes the caller's own name for the note as a fallback. A caller holding one gives it up only to a leading `# H1` — the document naming itself, which is both the better title and the line that would otherwise be shown directly beneath it. Anything else on line one is content there: a log's first line is not its title, and promoting it would both retitle the note and redraw that line as a heading. The editor passes no fallback, since it has no title field, and keeps taking the first line whether or not it is a heading. The third writer, saved action output, hands over a fenced block. Its first line can never be a title, so there is nothing there to extract and it is left as it is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
…itle
A note's title column holds plain text — `TimelineRow` renders it as
`{title}` and `#note:` references resolve to it the same way — but on the
editor's save path it was taken verbatim from a line Milkdown had just
*serialized*, so it arrived carrying whatever backslashes the markdown
serializer added. Type `snake_case_name` as a note's first line and the
timeline row showed `snake\_case\_name`. `_` and `&` are the ones that
actually bite: identifiers and company names in note titles are ordinary.
`canBeNoteTitle` already knew about this — it undid those escapes before
deciding whether the line could be a title, so a typed-out URL reached
the rule as `https://…` rather than `https\://…` — and twenty lines later
`splitNoteMarkdown` stored the line unmodified. The two halves of one
decision disagreed about what the line said.
So the transform gets a name, `unescapeMarkdown`, and both halves use it.
Its rule changes with the move: from a lookahead that deleted backslashes
to a pair rule over exactly CommonMark's 32 escapable ASCII punctuation
characters. That fixes two things only visible once the output is stored
rather than tested — a zero-width lookahead re-examined the second
backslash of a `\\` pair and ate it too, so every backslash the user
typed disappeared, and the old negated class also covered non-ASCII
punctuation, where a backslash is literal and the serializer leaves it be.
`splitNoteMarkdown` strips the heading marker first and unescapes what
remains. Order matters: `# ` on a serialized line is real structure, so
unescaping first would turn `\# Heading` — a paragraph a reader sees as
`# Heading` — into a marker plus `Heading`, dropping a visible character.
`isDocumentTitle` keeps reading the raw line for the same reason. The drop
path gets this for free and wants it: a dropped `README.md` whose H1 is
`# snake\_case\_name` is now listed as `snake_case_name`.
With the unescape named, the rule splits at the escape boundary.
`canBeNoteTitleLine` unescapes and delegates; `canBeNoteTitleText` is what
the editor plugin calls, since a parsed node's `textContent` is plain
already and was being unescaped a second time. That double pass is where
the plugin and the save path could disagree: for a paragraph whose visible
text is `\- item`, the plugin read a bullet and refused to promote it while
the save path took `\- item` as the title. Both doc comments promise these
halves agree; now they do, with the whole question of escapes in one
function.
Left alone deliberately: `noteMarkdownWithTitle` still prepends the title
verbatim, so a stored title holding a *complete* inline construct is
re-read as markup on reopen (`a _b_ c` comes back italic) and the next
save stores the equivalent `a *b* c`, stable from there — re-escaping would
push backslashes into the markdown the viewer renders and the user copies,
to defend a case that needs a plain-text paste to reach. The Rust
`extract_note_title` is unchanged, since agent-written notes never passed
through a serializer. No migration either: an escaped title heals on its
first re-save.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a way to write a note by hand, instead of only asking an agent to write one.
Changes
"Write note" action. Branch card footers get a
…dropdown at the end of the left-aligned session buttons. It always holds a new "Write note" action, which opens a WYSIWYG markdown editor the user types into directly — both when creating the note and when clicking a written note in the timeline. This is distinct from "New note", which launches an agent that writes the note for you.Footer overflow menu. The same
…menu absorbs the Note/Commit/Review buttons as the card narrows. This tier cannot be a container query like the existing label tiers, because the menu content is portaled out of thetimelinecontainer, so button and menu both read aResizeObserverwidth through the purecomputeFooterOverflowinfooterOverflow.ts. Review drops out first, then Commit, leaving Note last. Footer button styling ends up back at the original dashed look after two restyle attempts on the branch; the…trigger uses the standard outline variant to match the right-hand Diff/PR buttons.Note
subtypecolumn (migration0027).NULLfor session-produced notes,writtenfor user-authored ones. Existing session-less notes (drag-dropped files, saved action output) are backfilled towritten, since they are exactly that class and become editable too. The newupdate_notecommand refuses anything else, so an agent's note cannot be overwritten out from under its session. Written notes have no separate title field — the leading H1 is the title, matching howresolve_note_title_and_bodystores session notes.Editor. Milkdown's Crepe, wrapped in
MarkdownWysiwygEditor.sveltebehind a value-in/getMarkdown()-out contract so it stays swappable, and lazily imported so ProseMirror and CodeMirror stay out of the main bundle. Its theme is remapped onto ourvar(--*)tokens. Crepe's chrome is stripped (BlockEdit, Toolbar, ImageBlock, Table, CodeMirror, and the virtual cursor) so the OS draws the caret and the document typography matchesNoteModal's.markdown-content.Tests
Unit tests for
splitNoteMarkdown,computeFooterOverflow, the migration, and the store's written-note update path; integration tests drive a real Milkdown editor throughhandleTextInputfor the placeholder and title-promotion plugins.Follow-ups
Overflow thresholds are starting values and still want a visual pass on a real card, and the nested DropdownMenu inside
TimelineContextMenuwants a manual right-click check.