chore: nicer cm widgets for aria (#33524)

packages/recorder/src/recorder.tsx

@@ -220,7 +220,7 @@ function parseAriaSnapshot(ariaSnapshot: string): { fragment?: ParsedYaml, error
   for (const error of yamlDoc.errors) {
     errors.push({
       line: lineCounter.linePos(error.pos[0]).line,
-      type: 'error',
+      type: 'subtle-error',
       message: error.message,
     });
   }
@@ -233,10 +233,12 @@ function parseAriaSnapshot(ariaSnapshot: string): { fragment?: ParsedYaml, error
       parseAriaKey(key.value);
     } catch (e) {
       const keyError = e as AriaKeyError;
+      const linePos = lineCounter.linePos(key.srcToken!.offset + keyError.pos);
       errors.push({
         message: keyError.shortMessage,
-        line: lineCounter.linePos(key.srcToken!.offset + keyError.pos).line,
+        line: linePos.line,
-        type: 'error',
+        column: linePos.col,
+        type: 'subtle-error',
       });
     }
   };

packages/web/src/components/codeMirrorWrapper.css

@@ -163,12 +163,6 @@ body.dark-mode .CodeMirror span.cm-type {
   /* Intentionally empty. */
 }
 
-.CodeMirror .source-line-error-underline {
-  text-decoration: underline wavy var(--vscode-errorForeground);
-  position: relative;
-  top: -12px;
-}
-
 .CodeMirror .source-line-error-widget {
   background-color: var(--vscode-inputValidation-errorBackground);
   white-space: pre-wrap;
@@ -181,3 +175,9 @@ body.dark-mode .CodeMirror span.cm-type {
   text-decoration: underline;
   cursor: pointer;
 }
+
+.CodeMirror .source-line-error-underline {
+  text-decoration: underline;
+  text-decoration-color: var(--vscode-errorForeground);
+  text-decoration-style: wavy;
+}

packages/web/src/components/codeMirrorWrapper.tsx

@@ -22,7 +22,8 @@ import { useMeasure, kWebLinkRe } from '../uiUtils';
 
 export type SourceHighlight = {
   line: number;
-  type: 'running' | 'paused' | 'error';
+  column?: number;
+  type: 'running' | 'paused' | 'error' | 'subtle-error';
   message?: string;
 };
 
@@ -64,7 +65,12 @@ export const CodeMirrorWrapper: React.FC<SourceProps> = ({
 }) => {
   const [measure, codemirrorElement] = useMeasure<HTMLDivElement>();
   const [modulePromise] = React.useState<Promise<CodeMirror>>(import('./codeMirrorModule').then(m => m.default));
-  const codemirrorRef = React.useRef<{ cm: CodeMirror.Editor, highlight?: SourceHighlight[], widgets?: CodeMirror.LineWidget[] } | null>(null);
+  const codemirrorRef = React.useRef<{
+    cm: CodeMirror.Editor,
+    highlight?: SourceHighlight[],
+    widgets?: CodeMirror.LineWidget[],
+    markers?: CodeMirror.TextMarker[],
+  } | null>(null);
   const [codemirror, setCodemirror] = React.useState<CodeMirror.Editor>();
 
   React.useEffect(() => {
@@ -115,13 +121,8 @@ export const CodeMirrorWrapper: React.FC<SourceProps> = ({
       return;
 
     let valueChanged = false;
-    // CodeMirror has a bug that renders cursor poorly on a last line.
+    if (codemirror.getValue() !== text) {
-    let normalizedText = text;
+      codemirror.setValue(text);
-    if (!readOnly && !wrapLines && !normalizedText.endsWith('\n'))
-      normalizedText = normalizedText + '\n';
-
-    if (codemirror.getValue() !== normalizedText) {
-      codemirror.setValue(normalizedText);
       valueChanged = true;
       if (focusOnChange) {
         codemirror.execCommand('selectAll');
@@ -139,26 +140,36 @@ export const CodeMirrorWrapper: React.FC<SourceProps> = ({
       // Error widgets.
       for (const w of codemirrorRef.current!.widgets || [])
         codemirror.removeLineWidget(w);
+      for (const m of codemirrorRef.current!.markers || [])
+        m.clear();
       const widgets: CodeMirror.LineWidget[] = [];
+      const markers: CodeMirror.TextMarker[] = [];
       for (const h of highlight || []) {
-        if (h.type !== 'error')
+        if (h.type !== 'subtle-error' && h.type !== 'error')
           continue;
 
         const line = codemirrorRef.current?.cm.getLine(h.line - 1);
         if (line) {
-          const underlineWidgetElement = document.createElement('div');
+          const attributes: Record<string, string> = {};
-          underlineWidgetElement.className = 'source-line-error-underline';
+          attributes['title'] = h.message || '';
-          underlineWidgetElement.innerHTML = '&nbsp;'.repeat(line.length || 1);
+          markers.push(codemirror.markText(
-          widgets.push(codemirror.addLineWidget(h.line, underlineWidgetElement, { above: true, coverGutter: false }));
+              { line: h.line - 1, ch: 0 },
+              { line: h.line - 1, ch: h.column || line.length },
+              { className: 'source-line-error-underline', attributes }));
         }
 
+        if (h.type === 'error') {
           const errorWidgetElement = document.createElement('div');
           errorWidgetElement.innerHTML = ansi2html(h.message || '');
           errorWidgetElement.className = 'source-line-error-widget';
           widgets.push(codemirror.addLineWidget(h.line, errorWidgetElement, { above: true, coverGutter: false }));
         }
+      }
+
+      // Error markers.
       codemirrorRef.current!.highlight = highlight;
       codemirrorRef.current!.widgets = widgets;
+      codemirrorRef.current!.markers = markers;
     }
 
     // Line-less locations have line = 0, but they mean to reveal the file.
@@ -175,7 +186,7 @@ export const CodeMirrorWrapper: React.FC<SourceProps> = ({
       if (changeListener)
         codemirror.off('change', changeListener);
     };
-  }, [codemirror, text, highlight, revealLine, focusOnChange, onChange, readOnly]);
+  }, [codemirror, text, highlight, revealLine, focusOnChange, onChange]);
 
   return <div data-testid={dataTestId} className='cm-wrapper' ref={codemirrorElement} onClick={onCodeMirrorClick}></div>;
 };

tests/library/inspector/cli-codegen-aria.spec.ts

@@ -95,7 +95,6 @@ test.describe(() => {
     `);
 
     await recorder.recorderPage.locator('.tab-aria .CodeMirror').click();
-    await recorder.recorderPage.keyboard.press('ArrowLeft');
     for (let i = 0; i < '"Submit"'.length; i++)
       await recorder.recorderPage.keyboard.press('Backspace');
 
@@ -140,10 +139,8 @@ test.describe(() => {
     `);
 
     await recorder.recorderPage.locator('.tab-aria .CodeMirror').click();
-    await recorder.recorderPage.keyboard.press('ArrowLeft');
     await recorder.recorderPage.keyboard.press('Backspace');
-    await expect(recorder.recorderPage.locator('.tab-aria .CodeMirror')).toMatchAriaSnapshot(`
+    // 3 highlighted tokens.
-      - text: '- button "Submit Unterminated string'
+    await expect(recorder.recorderPage.locator('.source-line-error-underline')).toHaveCount(3);
-    `);
   });
 });

tests/playwright-test/ui-mode-test-source.spec.ts

@@ -122,7 +122,6 @@ test('should show top-level errors in file', async ({ runUITest }) => {
   await expect(
       page.locator('.CodeMirror-linewidget')
   ).toHaveText([
-    '            ',
     'TypeError: Assignment to constant variable.'
   ]);
 });
@@ -155,7 +154,6 @@ test('should show syntax errors in file', async ({ runUITest }) => {
   await expect(
       page.locator('.CodeMirror-linewidget')
   ).toHaveText([
-    '                                              ',
     /Missing semicolon./
   ]);
 });
@@ -183,7 +181,6 @@ test('should load error (dupe tests) indicator on sources', async ({ runUITest }
   await expect(
       page.locator('.CodeMirror-linewidget')
   ).toHaveText([
-    '                              ',
     /Error: duplicate test title "first", first declared in a.test.ts:3/
   ]);