Skip to content

fix: use capture-phase keydown listener with escClosingRef to prevent duplicate ESC handling - #500

Closed
yoyo837 with Copilot wants to merge 1 commit into
fix/preview-esc-cifrom
copilot/sub-pr-499
Closed

fix: use capture-phase keydown listener with escClosingRef to prevent duplicate ESC handling#500
yoyo837 with Copilot wants to merge 1 commit into
fix/preview-esc-cifrom
copilot/sub-pr-499

Conversation

CopilotAI commented Mar 17, 2026

Copy link
Copy Markdown
Contributor

event.stopPropagation() in a bubbling-phase window.addEventListener('keydown', ...) handler has no effect on document-level listeners (which fire earlier in the bubble chain) or other window bubble listeners. The original fix in #499 used it incorrectly.

Changes

  • Capture phase: switched to window.addEventListener('keydown', onKeyDown, true) so the Preview handler fires before @rc-component/portal's onGlobalKeyDown (window bubble-phase) and any document-level listeners
  • escClosingRef guard: replaces the ineffective stopPropagation(). When our capture handler calls onClose(), it sets escClosingRef.current = true (reset via microtask) so the Portal's onEsc callback skips the duplicate close:
// capture-phase handlerif(keyCode===KeyCode.ESC||key==='Escape'){escClosingRef.current=true;event.preventDefault();onClose?.();Promise.resolve().then(()=>{escClosingRef.current=false;});return;}// Portal onEsc fallback — skips if capture handler already closedconstonEsc: PortalProps['onEsc']=({ top })=>{if(top&&!escClosingRef.current){onClose?.();}};
  • onEsc fallback restored: Portal's onEsc is kept as a safety net for edge cases where focus movement triggers ESC detection outside our window listener
  • Tests: preview.portal.test.tsx covers capture-phase registration, onEsc fallback, and the deduplication guard

stopImmediatePropagation() was considered but rejected: @rc-component/util's useId returns 'test-id' for all portals in test mode, causing portal clear() to remove sibling portal entries from the shared stack — making the second Escape unable to close the parent dialog.


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

@vercel

vercelBot commented Mar 17, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
imageErrorErrorMar 17, 2026 8:17am

CopilotAI changed the title [WIP] [WIP] Address feedback on handling Escape in preview keydownfix: use capture-phase keydown listener with escClosingRef to prevent duplicate ESC handlingMar 17, 2026
CopilotAI requested a review from yoyo837March 17, 2026 09:00
@yoyo837yoyo837 closed this Mar 17, 2026
Sign up for freeto 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.

2 participants

@yoyo837