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

packages/base/codemirror-editor.gts

@@ -7,7 +7,7 @@ import { on } from '@ember/modifier';
 import { scheduleOnce } from '@ember/runloop';
 import { htmlSafe } from '@ember/template';
 import { Tooltip } from '@cardstack/boxel-ui/components';
-import { eq, not } from '@cardstack/boxel-ui/helpers';
+import { eq } from '@cardstack/boxel-ui/helpers';
 
 import {
   baseRRI,
@@ -199,6 +199,13 @@ interface ToolbarItem {
   // binding in the CodeMirror keymap (bold/italic/code); absent items render a
   // label-only tooltip.
   shortcut?: string;
+  // Inline-format toggles (bold/italic/etc.) wrap the current selection, so they
+  // only make sense when text is highlighted. Set for those buttons so they
+  // disable when the selection is collapsed. Line-based buttons omit it.
+  requiresSelection?: boolean;
+  // Computed enablement for this button, folding in focus and (for
+  // selection-requiring buttons) whether text is highlighted.
+  disabled?: boolean;
 }
 
 const EMPTY_FORMATS: SelectionFormats = Object.freeze({
@@ -212,6 +219,9 @@ const EMPTY_FORMATS: SelectionFormats = Object.freeze({
 function sameToolbarState(a: SelectionInfo, b: SelectionInfo): boolean {
   return (
     a.hasFocus === b.hasFocus &&
+    // Selection presence gates the inline-format buttons' enablement, so a
+    // collapse/expand must refresh the toolbar even when nothing else changed.
+    a.hasSelection === b.hasSelection &&
     a.formats.bold === b.formats.bold &&
     a.formats.italic === b.formats.italic &&
     a.formats.code === b.formats.code &&
@@ -467,7 +477,13 @@ export default class CodeMirrorEditor extends GlimmerComponent<CodeMirrorEditorS
   get toolbarButtons(): ToolbarItem[] {
     let f = this.toolbarFormats;
     let pressed = (active: boolean) => (active ? 'true' : 'false');
-    return [
+    let enabled = this.toolbarEnabled;
+    let hasSelection = this._selectionInfo?.hasSelection ?? false;
+    // Inline-format toggles additionally require a highlighted selection;
+    // line-based buttons only require focus.
+    let disabledFor = (requiresSelection?: boolean) =>
+      !enabled || (!!requiresSelection && !hasSelection);
+    let items: ToolbarItem[] = [
       {
         testId: 'bold',
         label: 'Bold',
@@ -476,6 +492,7 @@ export default class CodeMirrorEditor extends GlimmerComponent<CodeMirrorEditorS
         active: f.bold,
         ariaPressed: pressed(f.bold),
         shortcut: `${modKey}B`,
+        requiresSelection: true,
       },
       {
         testId: 'italic',
@@ -485,6 +502,7 @@ export default class CodeMirrorEditor extends GlimmerComponent<CodeMirrorEditorS
         active: f.italic,
         ariaPressed: pressed(f.italic),
         shortcut: `${modKey}I`,
+        requiresSelection: true,
       },
       {
         testId: 'strikethrough',
@@ -493,6 +511,7 @@ export default class CodeMirrorEditor extends GlimmerComponent<CodeMirrorEditorS
         action: this._wrapStrikethrough,
         active: f.strikethrough,
         ariaPressed: pressed(f.strikethrough),
+        requiresSelection: true,
       },
       {
         testId: 'code',
@@ -502,6 +521,7 @@ export default class CodeMirrorEditor extends GlimmerComponent<CodeMirrorEditorS
         active: f.code,
         ariaPressed: pressed(f.code),
         shortcut: `${modKey}\``,
+        requiresSelection: true,
       },
       {
         testId: 'link',
@@ -510,6 +530,7 @@ export default class CodeMirrorEditor extends GlimmerComponent<CodeMirrorEditorS
         action: this._toggleLink,
         active: f.link,
         ariaPressed: pressed(f.link),
+        requiresSelection: true,
       },
       { divider: true },
       {
@@ -550,6 +571,12 @@ export default class CodeMirrorEditor extends GlimmerComponent<CodeMirrorEditorS
         action: this._toggleBlockquote,
       },
     ];
+    for (let item of items) {
+      if (!item.divider) {
+        item.disabled = disabledFor(item.requiresSelection);
+      }
+    }
+    return items;
   }
 
   /** Prevent mousedown on toolbar/popup buttons from stealing editor focus/selection */
@@ -1212,7 +1239,7 @@ export default class CodeMirrorEditor extends GlimmerComponent<CodeMirrorEditorS
                   is suppressed while the control is disabled. }}
               <Tooltip
                 @placement='top'
-                @disabled={{not this.toolbarEnabled}}
+                @disabled={{btn.disabled}}
                 data-test-toolbar-tooltip={{btn.testId}}
               >
                 <:trigger>
@@ -1222,7 +1249,7 @@ export default class CodeMirrorEditor extends GlimmerComponent<CodeMirrorEditorS
                     type='button'
                     aria-label={{btn.label}}
                     aria-pressed={{btn.ariaPressed}}
-                    disabled={{not this.toolbarEnabled}}
+                    disabled={{btn.disabled}}
                     {{on 'mousedown' this._preventFocusLoss}}
                     {{on 'click' btn.action}}
                   >{{#let btn.icon as |Icon|}}<Icon

packages/host/app/lib/codemirror-context.ts

@@ -1011,13 +1011,11 @@ function wrapWith(marker: string) {
     let { from, to } = view.state.selection.main;
     let len = marker.length;
 
-    // No selection: insert an empty pair of markers and drop the cursor
-    // between them so the user can type the content (e.g. **|**).
+    // No selection: do nothing. Inserting an empty marker pair here leaves
+    // stray markers (e.g. **) visible in source and preview. Inline formatting
+    // applies to selected text only — the toolbar disables these buttons and
+    // the keyboard shortcut is a no-op until the user highlights something.
     if (from === to) {
-      view.dispatch({
-        changes: { from, insert: marker + marker },
-        selection: { anchor: from + len },
-      });
       return true;
     }
 
@@ -1099,12 +1097,8 @@ function toggleLink(view: EditorView): boolean {
   }
 
   if (from === to) {
-    // No selection: insert empty link syntax with the cursor inside the
-    // brackets so the user can type the link text — [|](url).
-    view.dispatch({
-      changes: { from, insert: '[](url)' },
-      selection: { anchor: from + 1 },
-    });
+    // No selection: do nothing. Inserting empty link syntax leaves a stray
+    // [](url) in the document. Linking applies to selected text only.
     return true;
   }
 

packages/host/tests/integration/components/codemirror-editor-test.gts

@@ -1327,7 +1327,7 @@ module('Integration | codemirror-context', function (hooks) {
     }
   });
 
-  test('wrapWith inserts empty markers with cursor centered when no selection', async function (assert) {
+  test('wrapWith does nothing when there is no selection', async function (assert) {
     let element = document.createElement('div');
     document.body.appendChild(element);
 
@@ -1348,19 +1348,15 @@ module('Integration | codemirror-context', function (hooks) {
       view.dispatch({ selection: { anchor: 6, head: 6 } });
       let result = cmContext.wrapWith('**')(view);
 
-      assert.true(result, 'returns true after inserting markers');
+      assert.true(result, 'returns true (shortcut consumed) without editing');
       assert.strictEqual(
         view.state.doc.toString(),
-        'Hello ****World',
-        'an empty pair of bold markers is inserted at the cursor',
+        'Hello World',
+        'no stray markers are inserted when nothing is selected',
       );
       let sel = view.state.selection.main;
       assert.true(sel.empty, 'cursor is collapsed (no selection)');
-      assert.strictEqual(
-        sel.from,
-        8,
-        'cursor sits between the two pairs of markers',
-      );
+      assert.strictEqual(sel.from, 6, 'cursor stays where it was');
 
       view.destroy();
     } finally {
@@ -1957,6 +1953,43 @@ module('Integration | codemirror-context', function (hooks) {
     }
   });
 
+  test('toggleLink does nothing when there is no selection', async function (assert) {
+    let element = document.createElement('div');
+    document.body.appendChild(element);
+
+    try {
+      let state = cmContext.createEditorState({
+        content: 'Hello World',
+        onDocChange: () => {},
+        onCardTargetsChange: () => {},
+        onOpenCardSearch: () => {},
+      });
+
+      let view = new cmContext.EditorView({
+        state,
+        parent: element,
+      });
+
+      // Cursor at position 6, no selection
+      view.dispatch({ selection: { anchor: 6, head: 6 } });
+      let result = cmContext.toggleLink(view);
+
+      assert.true(result, 'returns true without editing');
+      assert.strictEqual(
+        view.state.doc.toString(),
+        'Hello World',
+        'no stray [](url) is inserted when nothing is selected',
+      );
+      let sel = view.state.selection.main;
+      assert.true(sel.empty, 'cursor is collapsed (no selection)');
+      assert.strictEqual(sel.from, 6, 'cursor stays where it was');
+
+      view.destroy();
+    } finally {
+      element.remove();
+    }
+  });
+
   // ── Lazy loading ──
 
   test('globalThis.__loadCodeMirror returns context with expected exports', async function (assert) {

packages/host/tests/integration/components/rich-markdown-field-test.gts

@@ -1607,6 +1607,13 @@ module('Integration | RichMarkdownField', function (hooks) {
     assert
       .dom('[data-test-toolbar="bold"]')
       .isDisabled('Bold is disabled before the editor gains focus');
+    // The inline-format toggles wrap a selection, so they stay disabled with no
+    // focus (and, once focused, until text is highlighted — see the note above).
+    for (let testId of ['italic', 'strikethrough', 'code', 'link']) {
+      assert
+        .dom(`[data-test-toolbar="${testId}"]`)
+        .isDisabled(`${testId} is disabled before the editor gains focus`);
+    }
     assert
       .dom('[data-test-toolbar="blockquote"]')
       .isDisabled('all formatting controls start disabled');