fix(sidebar): scope the Escape dismiss to the panel

Co-Authored-By: Claude <noreply@anthropic.com>

Author: frankieyan

src/sidebar/sidebar.mdx

@@ -34,8 +34,8 @@ styles the real elements while the provider holds state and behavior.
 <Markdown>{`
 | Part | Renders | Owns |
 | --- | --- | --- |
-| \`Sidebar\` | the modal backdrop only (a sibling of the panel) | All controlled state and behavior: open, overlay, width and bounds, dismiss; derives the overlay state; runs the Escape dismiss effect; provides context |
-| \`SidebarContent\` | the panel element (a neutral \`<div>\` with \`role="dialog"\` when needed) | Positioning, the slide / collapse transition, dialog semantics, the committed width, and the focus trap and assistive-tech hiding while modal |
+| \`Sidebar\` | the modal backdrop only (a sibling of the panel) | All controlled state and behavior: open, overlay, width and bounds, dismiss; derives the overlay state; provides context |
+| \`SidebarContent\` | the panel element (a neutral \`<div>\` with \`role="dialog"\` when needed) | Positioning, the slide / collapse transition, dialog semantics, the committed width, the focus trap and assistive-tech hiding while modal, and panel-scoped Escape-to-dismiss |
 | \`SidebarResizeHandle\` | a \`role="separator"\` | The pointer and keyboard resize affordance and its ARIA; self-positions on the inner edge from \`align\` |
 `}</Markdown>
 
@@ -245,7 +245,9 @@ CSS custom properties, set on `SidebarContent` (or any ancestor).
   automatically (it marks sibling content `aria-hidden`). Still apply `inert` to
   the main element: it also blocks pointer interaction, which the automatic
   hiding does not.
-- Escape dismisses an open overlay when `dismissOverlayOnEscape` is set, and respects
-  `event.defaultPrevented` so app-level key handling can opt out.
+- Escape dismisses an open overlay when `dismissOverlayOnEscape` is set, but only
+  while focus is within the panel (it is handled on the panel, not app-wide). It
+  respects `event.defaultPrevented`, so a child that consumes Escape (an open menu
+  or tooltip) keeps it.
 - The resize handle is a focusable `separator` while open, and leaves the tab
   order and the accessibility tree while closed.

src/sidebar/sidebar.test.tsx

@@ -174,41 +174,74 @@ describe('modal background', () => {
 })
 
 describe('Escape dismissal', () => {
-    it('dismisses an open overlay on Escape when enabled', () => {
+    it('dismisses an open overlay on Escape from within the panel when enabled', () => {
         const onDismiss = jest.fn()
         renderSidebar({
             isOverlay: true,
             overlayMode: 'modal',
             dismissOverlayOnEscape: true,
             onDismiss,
         })
-        fireEvent.keyDown(document.body, { key: 'Escape' })
+        fireEvent.keyDown(screen.getByTestId('sidebar-panel'), { key: 'Escape' })
         expect(onDismiss).toHaveBeenCalledTimes(1)
     })
 
-    it('respects defaultPrevented so app handlers can opt out', () => {
+    it('dismisses a non-modal overlay on Escape from within the panel', () => {
         const onDismiss = jest.fn()
         renderSidebar({
             isOverlay: true,
-            overlayMode: 'modal',
+            overlayMode: 'dialog',
             dismissOverlayOnEscape: true,
             onDismiss,
         })
+        fireEvent.keyDown(screen.getByTestId('sidebar-panel'), { key: 'Escape' })
+        expect(onDismiss).toHaveBeenCalledTimes(1)
+    })
 
-        function preventEscape(event: KeyboardEvent) {
-            if (event.key === 'Escape') event.preventDefault()
-        }
-        document.body.addEventListener('keydown', preventEscape, { capture: true })
+    it('ignores Escape fired outside the panel', () => {
+        const onDismiss = jest.fn()
+        renderSidebar({
+            isOverlay: true,
+            overlayMode: 'dialog',
+            dismissOverlayOnEscape: true,
+            onDismiss,
+        })
         fireEvent.keyDown(document.body, { key: 'Escape' })
-        document.body.removeEventListener('keydown', preventEscape, { capture: true })
+        expect(onDismiss).not.toHaveBeenCalled()
+    })
 
+    it('respects defaultPrevented so a descendant can consume Escape', () => {
+        const onDismiss = jest.fn()
+        renderSidebar(
+            {
+                isOverlay: true,
+                overlayMode: 'modal',
+                dismissOverlayOnEscape: true,
+                onDismiss,
+            },
+            {
+                children: (
+                    <button
+                        type="button"
+                        onKeyDown={(event) => {
+                            if (event.key === 'Escape') event.preventDefault()
+                        }}
+                    >
+                        Consumes Escape
+                    </button>
+                ),
+            },
+        )
+        fireEvent.keyDown(screen.getByRole('button', { name: 'Consumes Escape' }), {
+            key: 'Escape',
+        })
         expect(onDismiss).not.toHaveBeenCalled()
     })
 
     it('does not dismiss while docked', () => {
         const onDismiss = jest.fn()
         renderSidebar({ isOverlay: false, dismissOverlayOnEscape: true, onDismiss })
-        fireEvent.keyDown(document.body, { key: 'Escape' })
+        fireEvent.keyDown(screen.getByTestId('sidebar-panel'), { key: 'Escape' })
         expect(onDismiss).not.toHaveBeenCalled()
     })
 
@@ -220,7 +253,7 @@ describe('Escape dismissal', () => {
             dismissOverlayOnEscape: false,
             onDismiss,
         })
-        fireEvent.keyDown(document.body, { key: 'Escape' })
+        fireEvent.keyDown(screen.getByTestId('sidebar-panel'), { key: 'Escape' })
         expect(onDismiss).not.toHaveBeenCalled()
     })
 })

src/sidebar/sidebar.tsx

@@ -31,6 +31,7 @@ type SidebarContextValue = {
     unmountOnHide: boolean
     panelId: string
     panelRef: React.RefObject<HTMLDivElement | null>
+    dismissOverlayOnEscape: boolean
     onDismiss?: () => void
     width?: number
     minWidth?: number
@@ -191,22 +192,6 @@ function Sidebar({
     const overlayOpen = isOverlay && isOpen
     const shouldTrap = overlayOpen && overlayMode === 'modal'
 
-    React.useEffect(
-        function dismissOverlayOnEscapeEffect() {
-            if (!overlayOpen || !dismissOverlayOnEscape) return
-
-            function handleEscapeKeyDown(event: KeyboardEvent) {
-                if (event.key === 'Escape' && !event.defaultPrevented) {
-                    onDismiss?.()
-                }
-            }
-
-            window.addEventListener('keydown', handleEscapeKeyDown)
-            return () => window.removeEventListener('keydown', handleEscapeKeyDown)
-        },
-        [overlayOpen, dismissOverlayOnEscape, onDismiss],
-    )
-
     const contextValue: SidebarContextValue = {
         align,
         overlayMode,
@@ -217,6 +202,7 @@ function Sidebar({
         unmountOnHide,
         panelId,
         panelRef,
+        dismissOverlayOnEscape,
         onDismiss,
         width,
         minWidth,
@@ -282,6 +268,7 @@ const SidebarContent = React.forwardRef<HTMLDivElement, SidebarContentProps>(
             exceptionallySetClassName,
             children,
             style,
+            onKeyDown: consumerOnKeyDown,
             'aria-label': ariaLabel,
             'aria-labelledby': ariaLabelledby,
             ...rest
@@ -299,6 +286,8 @@ const SidebarContent = React.forwardRef<HTMLDivElement, SidebarContentProps>(
             panelId,
             panelRef,
             width,
+            dismissOverlayOnEscape,
+            onDismiss,
         } = useSidebarContext('SidebarContent')
 
         const mergedRef = useMergeRefs([panelRef, ref])
@@ -328,11 +317,24 @@ const SidebarContent = React.forwardRef<HTMLDivElement, SidebarContentProps>(
             ? children
             : null
 
+        function handlePanelKeyDown(event: React.KeyboardEvent<HTMLDivElement>) {
+            if (
+                overlayOpen &&
+                dismissOverlayOnEscape &&
+                event.key === 'Escape' &&
+                !event.defaultPrevented
+            ) {
+                onDismiss?.()
+            }
+            consumerOnKeyDown?.(event)
+        }
+
         return (
             <Box
                 {...rest}
                 as="div"
                 ref={mergedRef}
+                onKeyDown={handlePanelKeyDown}
                 display="flex"
                 flexDirection="column"
                 flexShrink={0}