Skip to content

Remove CodeMirror dependency from live editor playground#25387

Open
Copilot wants to merge 5 commits intomainfrom
copilot/remove-codemirror-dependency
Open

Remove CodeMirror dependency from live editor playground#25387
Copilot wants to merge 5 commits intomainfrom
copilot/remove-codemirror-dependency

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 8, 2026

The docs/public/editor/ playground loaded 7 CodeMirror packages (~200+ KB) from esm.sh at runtime. This replaces all of it with vanilla JS — zero external JS imports.

Changes

  • Input editor (editor.js): EditorView<textarea> with keydown (Tab inserts 2 spaces, Mod-Enter compiles) and input (localStorage save + auto-compile) listeners
  • Output display (editor.js): Read-only EditorView<pre><code> with textContent updates
  • Hover tooltips (hover-tooltips.js): CodeMirror hoverTooltip extension → mousemove handler using monospace char-width math to resolve line/column from mouse position. All pure logic functions (extractKeyFromLine, resolveKeyPath, lookupSchema, buildTooltipDOM) preserved unchanged.
  • Theme switching: Deleted Compartment-based oneDark toggle entirely — Primer CSS already handles dark/light via data-color-mode="auto"
  • CSS (index.html): Replaced .cm-* rules with #editorTextarea, #outputPre, .hover-tooltip, .tooltip-docs-*

Tradeoffs

Lost syntax highlighting, line numbers, bracket matching, and pixel-perfect hover detection. Gained instant load (no CDN fetch), zero JS overhead, no esm.sh availability risk, and ~60% less JS code.

Screenshots

Light mode:
editor-light-mode

Dark mode:
editor-dark-mode

Replace 7 CodeMirror CDN imports with vanilla JS:
- Input editor: CodeMirror EditorView → <textarea> with Tab/Mod-Enter keydown handlers
- Output display: read-only EditorView → <pre><code> with textContent updates
- Hover tooltips: CodeMirror hoverTooltip → mousemove-based positioning using monospace font math
- Theme switching: Compartment-based oneDark toggle deleted; Primer CSS handles dark/light via data-color-mode="auto"
- CSS: .cm-* rules replaced with textarea/pre/#outputPre and .tooltip-docs rules

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/37a82432-a892-459f-bc5d-44e443649570

Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove CodeMirror dependency from live editor playground Remove CodeMirror dependency from live editor playground Apr 8, 2026
Copilot AI requested a review from Mossaka April 8, 2026 21:28
@Mossaka Mossaka marked this pull request as ready for review April 8, 2026 21:43
@Mossaka Mossaka requested review from Copilot April 8, 2026 21:43
@Mossaka
Copy link
Copy Markdown
Collaborator

Mossaka commented Apr 8, 2026

@copilot upload screenshots of the editor website running locally using playwright.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes the runtime CodeMirror (esm.sh) dependency from the docs live editor playground and replaces it with a vanilla JS <textarea> input editor plus a <pre><code> output display.

Changes:

  • Replaced CodeMirror-based input/output editors with <textarea> and <pre><code> rendering, including keyboard shortcuts and localStorage persistence.
  • Reimplemented hover-tooltips without CodeMirror using mousemove + monospace character measurement to map mouse position to line/column.
  • Updated HTML/CSS to remove .cm-* styling and add new styles for the textarea/output/tooltip components.
Show a summary per file
File Description
docs/public/editor/index.html Swaps editor/output mounts for <textarea> and <pre><code>; updates styling rules and tooltip class names.
docs/public/editor/editor.js Removes CodeMirror setup; wires textarea input, keyboard shortcuts, output rendering, and tooltip attachment.
docs/public/editor/hover-tooltips.js Drops CodeMirror hover extension; adds dependency-free tooltip wiring and character measurement for <textarea>.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 5

Comment on lines +239 to +241
span.style.cssText =
'position:absolute;visibility:hidden;white-space:pre;' +
window.getComputedStyle(textarea).font;
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

measureCharWidth() builds span.style.cssText by concatenating getComputedStyle(textarea).font without the font: property name, so the font declaration is invalid and the measured width may not match the textarea’s font. Set span.style.font = ... (or include font: in the cssText) to ensure accurate character width measurement.

Suggested change
span.style.cssText =
'position:absolute;visibility:hidden;white-space:pre;' +
window.getComputedStyle(textarea).font;
span.style.cssText = 'position:absolute;visibility:hidden;white-space:pre;';
span.style.font = window.getComputedStyle(textarea).font;

Copilot uses AI. Check for mistakes.
Comment on lines +294 to +297
function hideTooltip() {
hideTimer = setTimeout(() => {
tooltip.style.display = 'none';
tooltip.innerHTML = '';
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hideTooltip() starts a new setTimeout every time it’s called but never clears an existing pending timer. During mousemove, this can enqueue many timeouts and cause unnecessary work/flicker; clear any existing hideTimer before scheduling a new one (and null it after it fires).

Suggested change
function hideTooltip() {
hideTimer = setTimeout(() => {
tooltip.style.display = 'none';
tooltip.innerHTML = '';
function hideTooltip() {
if (hideTimer) clearTimeout(hideTimer);
hideTimer = setTimeout(() => {
tooltip.style.display = 'none';
tooltip.innerHTML = '';
hideTimer = null;

Copilot uses AI. Check for mistakes.
Comment on lines +288 to +289
const maxLeft = window.innerWidth - tooltip.offsetWidth - pad;
if (left > maxLeft) left = maxLeft;
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tooltip horizontal clamping can place the tooltip off-screen on narrow viewports: if tooltip.offsetWidth exceeds window.innerWidth, maxLeft becomes negative and left can be set to a negative value. Consider clamping left to a minimum padding value as well as the maximum.

Suggested change
const maxLeft = window.innerWidth - tooltip.offsetWidth - pad;
if (left > maxLeft) left = maxLeft;
const minLeft = pad;
const maxLeft = window.innerWidth - tooltip.offsetWidth - pad;
left = Math.max(minLeft, Math.min(left, Math.max(minLeft, maxLeft)));

Copilot uses AI. Check for mistakes.
}
if ((e.metaKey || e.ctrlKey) && e.key === 'Enter') {
e.preventDefault();
doCompile();
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the Mod-Enter handler, calling doCompile() while the compiler is still loading is a no-op because doCompile() returns early when !isReady. This makes the shortcut appear broken during load; consider setting pendingCompile = true (similar to the input handler) when the user triggers compile before readiness.

Suggested change
doCompile();
if (isReady) {
doCompile();
} else {
pendingCompile = true;
}

Copilot uses AI. Check for mistakes.
</span>
</div>
<div class="flex-1" style="min-height: 0; overflow: hidden;" id="editorMount"></div>
<textarea class="flex-1" style="min-height: 0;" id="editorTextarea" spellcheck="false" autocomplete="off" autocorrect="off" autocapitalize="off"></textarea>
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new <textarea> has no associated <label> or ARIA labeling. Add an accessible name (e.g., aria-label or aria-labelledby pointing at the “Workflow (.md)” header) so screen readers can identify the editor control.

See below for a potential fix:

        <span class="d-flex flex-items-center gap-1" id="editorTextareaLabel">
          <svg width="14" height="14" viewBox="0 0 16 16" fill="currentColor" style="opacity: 0.6;">
            <path d="M1.75 1h12.5c.966 0 1.75.784 1.75 1.75v10.5A1.75 1.75 0 0114.25 15H1.75A1.75 1.75 0 010 13.25V2.75C0 1.784.784 1 1.75 1zm12.5 1.5H1.75a.25.25 0 00-.25.25v10.5c0 .138.112.25.25.25h12.5a.25.25 0 00.25-.25V2.75a.25.25 0 00-.25-.25zM3 5.5a.75.75 0 01.75-.75h8.5a.75.75 0 010 1.5h-8.5A.75.75 0 013 5.5zm0 4a.75.75 0 01.75-.75h5.5a.75.75 0 010 1.5h-5.5A.75.75 0 013 9.5z"/>
          </svg>
          Workflow (.md)
        </span>
      </div>
      <textarea class="flex-1" style="min-height: 0;" id="editorTextarea" aria-labelledby="editorTextareaLabel" spellcheck="false" autocomplete="off" autocorrect="off" autocapitalize="off"></textarea>

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

@Mossaka Mossaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Good work on this — the approach is clean and the pure-logic functions are nicely preserved. A few issues to address:

Bug (must fix)

measureCharWidth sets font incorrectly (hover-tooltips.js ~line 237):

span.style.cssText =
  'position:absolute;visibility:hidden;white-space:pre;' +
  window.getComputedStyle(textarea).font;

The font shorthand value (e.g., 13px / 1.6 "SF Mono"...) is concatenated directly into cssText without a font: property name, producing invalid CSS. The measurement span uses the browser default font, making _charWidth wrong and tooltip positioning inaccurate. Fix:

span.style.cssText = 'position:absolute;visibility:hidden;white-space:pre;';
span.style.font = window.getComputedStyle(textarea).font;

Performance

  • getComputedStyle called on every mousemove in attachHoverTooltips. lineHeight, paddingTop, paddingLeft don't change between moves — cache them alongside cachedLines and invalidate on resize if needed.
  • No debounce/throttle on mousemove — the DOM manipulation in showTooltip (clearing innerHTML, building tooltip DOM) runs on every pixel of mouse movement. Wrap in requestAnimationFrame or add a short debounce.

Minor suggestions

  • Tooltip container: The tooltip uses position: fixed but is appended to panelEditor. If that element ever gets transform/will-change, fixed positioning breaks. Consider appending to document.body instead.
  • tooltip.innerHTML = '' before appendChildtooltip.replaceChildren(dom) is cleaner.
  • Tab handling breaks undo: Directly assigning .value clears the browser undo stack. Consider document.execCommand('insertText', false, ' ') (still works in all browsers) to preserve undo, or add a comment noting the tradeoff.
  • Shift-Tab dedent is not implemented — would improve the editing experience.
  • #outputPre height: Verify the <pre> fills the output container vertically — it may need an explicit height: 100% inline style to match the old outputMount behavior.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 8, 2026

@copilot upload screenshots of the editor website running locally using playwright.

Here are Playwright screenshots of the editor running locally with Primer CSS loaded:

Light mode:
editor-light-mode

Dark mode:
editor-dark-mode

Layout shows: header with sample selector and status badge, side-by-side textarea (input) and <pre><code> (output) panels with draggable divider, and footer. Primer CSS handles all theming via data-color-mode — no CodeMirror theme machinery needed.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • unpkg.com
    • Triggering command: /usr/bin/curl curl -sL -o primer.css REDACTED (dns block)
    • Triggering command: /usr/bin/wget wget -q -O primer.css REDACTED (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI and others added 2 commits April 8, 2026 21:58
- Fix measureCharWidth font setting (was invalid cssText concat)
- Cache getComputedStyle values, invalidate on resize
- Throttle mousemove with requestAnimationFrame
- Fix hideTooltip to clear existing timer before scheduling
- Clamp tooltip left to min padding for narrow viewports
- Append tooltip to document.body (avoids transform issues)
- Use replaceChildren() instead of innerHTML=''
- Use execCommand('insertText') for Tab to preserve undo
- Add Shift-Tab dedent support
- Fix Mod-Enter to set pendingCompile when compiler not ready
- Add aria-labelledby to textarea for accessibility

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/96fa5b92-7f2f-4beb-8e80-e75a9885cb69

Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 8, 2026

@copilot Good work on this — the approach is clean and the pure-logic functions are nicely preserved. A few issues to address:

Bug (must fix)...

All items addressed in d8962cd and eebdf18:

Bug fix: measureCharWidth now sets span.style.font separately instead of concatenating into cssText.

Performance:

  • getComputedStyle values (lineHeight, paddingTop, paddingLeft) cached in cachedStyle, invalidated on resize
  • mousemove handler wrapped in requestAnimationFrame throttle

All minor items:

  • Tooltip appended to document.body instead of panelEditor
  • replaceChildren() replaces innerHTML = '' + appendChild
  • Tab uses execCommand('insertText') to preserve undo
  • Shift-Tab dedent implemented (preserves selection range)
  • hideTooltip() clears existing timer before scheduling
  • Tooltip left clamped to minLeft for narrow viewports
  • Mod-Enter sets pendingCompile = true when compiler not ready
  • aria-labelledby="editorTextareaLabel" added to textarea
  • #outputPre already had height: 100% — verified correct

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.

Remove CodeMirror dependency from live editor playground

3 participants