Skip to content

fix(runner): give focus back to the editor when a preview rebuild steals it - #265

Open
GreenFlux wants to merge 2 commits into
masterfrom
fix/preview-focus-steal
Open

fix(runner): give focus back to the editor when a preview rebuild steals it#265
GreenFlux wants to merge 2 commits into
masterfrom
fix/preview-focus-steal

Conversation

@GreenFlux

@GreenFlux GreenFlux commented Aug 28, 2026

Copy link
Copy Markdown

Summary

Typing in the demo runner's code editor could lose focus to the preview mid-sentence. Every Tier-1 keystroke re-evaluates the demo module inside the cross-origin Sandpack iframe, and a demo that focuses its grid on boot — selectCells(), listen(), any element.focus() — pulls browser focus out of CodeMirror. The rest of the keystrokes then land in a grid cell, and the user cannot edit code. Reported against /edit/5l4di1j473 (a demo that calls hot.selectCells(...) at the end of its entry module), but any demo that focuses something on boot triggers it.

The grab cannot be prevented from the parent (the frame is cross-origin), but it is observable: a window blur while document.hasFocus() stays true means focus left the top document without leaving the page — it went into a subframe. When that happens within a keystroke of typing (2s window), EditorShell now hands focus straight back to the active editor pane. A deliberate click into the preview outside that window, and app/tab switches (hasFocus() goes false), are left alone.

Two implementation notes, both measured rather than assumed:

  • The handler does not key on document.activeElement: Chromium builds disagree about it across a cross-origin grab — sometimes it lands on the iframe element, sometimes it stays stale on the editor while every keystroke already routes into the frame.
  • The restore is a contentDOM blur+focus rather than a bare focus(): in the stale-activeElement manifestation a bare focus() is a no-op that reclaims nothing.

Test plan

  • New e2e/preview-focus.spec.ts (E2E_LIVE=1): loads the Tier-1 JavaScript starter, appends a line that focuses an element on every module evaluation, then types at a human cadence through the rebuilds and asserts the whole typed sentence lands in /index.js in one piece — including a tail typed after the final rebuild, which proves focus was restored, not merely never lost.
  • The spec fails against the deployed runner (no fix): typed characters route into the preview frame. It passes against a local build of this branch.
  • The reported mechanism verified separately with a real grid: appending new Handsontable(...) + selectCell(0, 0) and typing through the rebuilds loses the typed text on the deployed runner and keeps it intact on this branch.
  • pnpm test (890 pass), pnpm typecheck, pnpm --filter @handsontable/demo-authoring build, and the full Playwright suite with E2E_LIVE=1 (260 passed, 0 failed) are green locally.

Note

Medium Risk
Global window blur handling and programmatic refocus on every edit-adjacent subframe blur could interact oddly with intentional preview interaction, though the 2s window and hasFocus() guard limit scope to mid-edit theft.

Overview
Fixes typing in the demo runner losing focus to the Sandpack preview when Tier-1 edits trigger a rebuild and the demo module calls something like selectCells() or element.focus() on boot. The parent cannot block cross-origin focus grabs, so EditorShell now treats a window blur with document.hasFocus() still true, within 2s of the last edit, as theft and returns focus to the active CodeMirror pane via contentDOM blur + focus({ preventScroll: true }) (not document.activeElement, which Chromium can leave stale while keystrokes route into the iframe). Deliberate preview clicks and app/tab switches are ignored via the time window and hasFocus() check.

Every CodeEditor onChange updates a last-edit timestamp (typing, paste, undo) so rebuild-triggered grabs are covered.

Adds opt-in live E2E (E2E_LIVE=1, preview-focus.spec.ts): injects a line that focuses an input on each module eval, types at human cadence through rebuilds, and asserts the full comment string lands in /index.js—including text typed after the last rebuild.

Reviewed by Cursor Bugbot for commit ca16ac9. Bugbot is set up for automated code reviews on this repo. Configure here.

…als it

Typing in the code editor could lose focus to the preview mid-sentence:
every Tier-1 keystroke re-evaluates the demo module inside the
cross-origin Sandpack iframe, and a demo that focuses its grid on boot
(selectCells(), listen(), any element.focus()) pulls browser focus out
of CodeMirror — the rest of the keystrokes land in a grid cell.

The grab cannot be prevented from the parent (the frame is
cross-origin), but it is observable: a window blur while
document.hasFocus() stays true means focus went into a subframe. When
that happens within a keystroke of typing, EditorShell now hands focus
straight back to the active editor pane. Deliberate clicks into the
preview and app/tab switches are left alone.

The restore is a contentDOM blur+focus rather than a bare focus():
Chromium builds disagree on where activeElement lands after a
cross-origin grab, and on the builds where it stays stale on the editor
a bare focus() is a no-op that reclaims nothing.

The new spec fails without the fix (verified against the deployed
runner) and passes with it.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b2b53ef. Configure here.

Comment thread runner/packages/editor-shell/src/EditorShell.tsx Outdated
The focus-theft restore called contentDOM.focus() bare. CodeMirror's
own view.focus() passes preventScroll: true because .cm-content is the
whole document — a bare focus() can scroll a long file away from the
caret the user is typing at. Pass the same option here.

Refactor-only: no behavior change the focus spec can observe; the
existing e2e/preview-focus.spec.ts covers the restore path and stays
green.
@GreenFlux
GreenFlux requested a review from demtario August 28, 2026 13:09
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.

1 participant