Don't insert stray markers when formatting with no selection (CS-11697)

[FadhlanR]

Background and Goal

In the markdown editor, triggering an inline format with no text selected (cursor merely placed in the field) inserted a stray empty marker pair — **/*/~~/` via wrapWith, or [](url) via toggleLink — that stayed visible in both source and preview. Fixes CS-11697.

The defect had two layers, and both are fixed:

1. Command layer (packages/host/app/lib/codemirror-context.ts) — wrapWith and toggleLink inserted the empty marker pair whenever the selection was collapsed. This fired the same for the toolbar button and the Mod-B/Mod-I/Mod-\`` keyboard shortcut, so it was wrong regardless of trigger. Both now do nothing (no dispatch) when nothing is selected. The keymap is untouched — the shortcut simply no-ops on a bare cursor instead of producing ****. One exception is preserved: toggleLinkstill **unlinks** from a bare caret inside an existingtext` (its syntax-tree unlink branch runs before the collapsed-cursor check).
2. Toolbar layer (packages/base/codemirror-editor.gts) — the inline-format buttons (Bold, Italic, Strikethrough, Code, Link) now carry requiresSelection: true and disable until text is highlighted, so a click can't reach the command and the UI signals inapplicability. An active toggle stays clickable without a selection, since untoggling works at a bare caret — concretely, a caret inside a link marks the Link button pressed and keeps it enabled so it can unlink. The link-at-caret signal comes from findEnclosingLink, the same syntax-tree check toggleLink's unlink branch uses, so button state can't drift from command behavior. Line-based buttons (headings, lists, blockquote) act on a bare cursor and stay enabled. hasSelection was added to sameToolbarState so the toolbar re-renders when the selection collapses/expands.

Tests

• codemirror-editor-test.gts — rewrote the wrapWith empty-selection test to assert no change; added a toggleLink no-op test for a caret outside any link and an unlink test for a bare caret inside a link; the selection-based unlink test now calls the real toggleLink (it previously simulated the command with a string scan); added a selection-info test pinning formats.link at a caret inside vs. outside a link.
• rich-markdown-field-test.gts — extended the "controls start disabled" test to cover all five inline-format toggles, and added a test driving the live editor's caret into and out of a link to assert the Link button's aria-pressed wiring.

Note: the focused-with-selection → enabled transition can't be asserted in headless CI because view.hasFocus ANDs document.hasFocus() (pre-existing, documented limitation); the disabled side, the active-state wiring, and the command-level behavior are covered.

Screen Recording

Screen.Recording.2026-08-03.at.21.38.48.mov

[github-actions]

Preview deployments

• Host staging preview

• Host production preview

Host Test Results

    1 files      1 suites   2h 56m 0s ⏱️
3 813 tests 3 799 ✅ 14 💤 0 ❌
3 832 runs  3 818 ✅ 14 💤 0 ❌

Results for commit 7d74454.

Realm Server Test Results

    1 files  ±0      1 suites  ±0   13m 47s ⏱️ -43s
2 032 tests ±0  2 032 ✅ ±0  0 💤 ±0  0 ❌ ±0 
2 111 runs  ±0  2 111 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 7d74454. ± Comparison against earlier commit 35602ae.

[FadhlanR]

[Claude Code 🤖] This review focused on whether the two layers of the fix — the command-level no-op and the toolbar enablement — stay in agreement, and whether any existing collapsed-cursor behavior was load-bearing before being gated off. The diff is small; the interesting questions are all one call site away from it.

Bottom line: no blocking issues. One minor behavior regression worth an explicit decision (collapsed-cursor unlink, inline thread on the Link button), everything else verified sound.

What lands right

• Fixing at both layers is the correct shape. The keymap can't be gated by toolbar state, so the command-layer no-op in wrapWith/toggleLink is the real fix; the disabled buttons are honest UI on top rather than the only line of defense.
• The hasSelection addition to sameToolbarState is necessary, not defensive. Verified by tracing the memo: expanding a selection over unformatted text changes neither hasFocus, nor any formats flag (all remain false), nor currentRef — so without the new comparison the update is swallowed, _selectionInfo never refreshes, and the inline buttons stay disabled with text visibly highlighted. The new comparison catches exactly that transition.
• Line-based buttons staying enabled is right — headings/lists/blockquote act on the cursor's line, so a collapsed cursor is a valid target for them.
• Test posture is sound. Both command-level no-ops are pinned (document unchanged and cursor position unchanged), and the headless focus limitation is documented rather than papered over with an assertion that would flake.

Recommendation

1. Decide the collapsed-cursor unlink question — see the inline thread on the Link button's requiresSelection in packages/base/codemirror-editor.gts. Non-blocking either way, but the code and comment should match whichever way it goes.

Adjacent, out of scope

• With the caret inside already-bold text, Mod-B now no-ops. Not a regression — the previous behavior inserted a stray empty marker pair there, which is worse — but conventional editors toggle formatting at the caret (unwrap the enclosing marks). If users ask for editor parity, that's the natural follow-up, and it lives entirely in wrapWith.
• The Add embed / Edit embed buttons at the left of the toolbar don't participate in the disabled model at all — they render enabled even before the editor has focus, unlike every other control. Pre-existing, noticed while reading the toolbar template; flagging for whoever touches that toolbar next.

[FadhlanR]

[Claude Code 🤖] Regression (minor, non-blocking): unlinking with a collapsed cursor is no longer reachable from the UI.

The mechanism. toggleLink in packages/host/app/lib/codemirror-context.ts resolves the syntax tree and checks for an enclosing Link node before its collapsed-selection check, and a collapsed cursor inside [text](url) satisfies from >= link.from && to <= link.to — so previously, placing the caret in a link and clicking the Link button unlinked it. This requiresSelection: true disables the button whenever the selection is collapsed, and the toolbar button is toggleLink's only caller (the markdown keymap binds only Mod-B / Mod-I / Mod-`), so the collapsed-cursor entry into the unlink branch is now dead in practice.

Verified. Traced the base-branch toggleLink: a collapsed caret inside a link takes the unlink branch (it precedes if (from === to)) and dispatches the text replacement. Grepped all non-test callers: toggleLink is invoked only via _toggleLink in this file.

What still works. Selecting any range within the link enables the button, and that selection still satisfies the unlink guard — unlink remains reachable, it just now demands a selection. That's why this is minor.

The way out — two options, either fine:

1. Accept it (select-then-click still unlinks, and it's consistent with the disabled-button story). If so, consider tightening the no-op comment inside toggleLink — "Linking applies to selected text only" isn't quite true of the function itself, whose earlier branch still unlinks at a collapsed caret; it's only true of what the UI now lets through.
2. Restore it: enable the Link button when the collapsed caret sits inside a link. That needs a link-at-caret signal in SelectionInfo — note the update listener in createEditorState hardcodes formats to all-false for collapsed selections, so formats.link can't serve as-is.

[FadhlanR]

[Claude Code 🤖] Resolved by restoring the affordance (option 2). What changed:

• findEnclosingLink is extracted from toggleLink's unlink branch and shared with the selection-info emitter in createEditorState, so the toolbar's Link-button state is computed by the exact same syntax-tree check the command runs — the two can't drift.
• At a collapsed caret the emitter now reports formats.link from that helper (the other formats stay false, matching wrapWith's no-op at a caret), and the toolbar's enablement rule gains an escape: an active toggle stays clickable without a selection, since untoggling works at a bare caret.
• The stale wording in toggleLink's no-op comment is fixed — that branch is now correctly described as "caret outside any link".

Coverage added: a toggleLink unlink-from-bare-caret command test, a selection-info test pinning formats.link at a caret inside vs. outside a link, and a component test asserting the Link button's aria-pressed flips as the caret enters and leaves a link (drivable headless — only enablement needs OS focus). The pre-existing selection-based unlink test also now calls the real toggleLink instead of simulating it with a string scan. Both touched modules pass: codemirror-context 55/55, RichMarkdownField 30/30.

[FadhlanR]

[Claude Code 🤖] Confirmation: returning true on a collapsed selection is the right call.

Returning false would let the key event fall through CodeMirror's keymap to the browser default, and these chords have browser-level bindings in some contexts (e.g. Ctrl-B toggling a bookmarks sidebar). Consuming the shortcut while editing nothing is the correct no-op, and the rewritten test pins both halves — returns true, document and cursor unchanged.

When this would stop being safe: if a lower-priority keymap ever wants to handle Mod-B/Mod-I/Mod-on a collapsed cursor, thistrue` will shadow it silently. Nothing binds them today.

[lukemelia]

In most writing environments I could type something, then press Ctrl+B and what I type after that is in bold and then press Ctrl+B again and what I type after that is not bold. Does that pattern work before/after this change?