fix(ui-drawer-layout,ui-a11y-utils): fix Tray closing immediately after opening and calling onDismiss

INSTUI-4828

Author: matyasf

CLAUDE.md

@@ -5,6 +5,7 @@
 InstUI is a React component library and design system. **Lerna 8 monorepo** with 100+ packages.
 
 **Key Resources:**
+
 - Website: https://instructure.design
 - AI docs: https://instructure.design/llms.txt
 - Component docs: https://instructure.design/markdowns/[ComponentName].md
@@ -27,6 +28,7 @@ InstUI is a React component library and design system. **Lerna 8 monorepo** with
 ```
 
 **Important file locations:**
+
 - Component source: `/packages/ui-*/src/`
 - Component tests: Co-located with components
 - Theme types: `/packages/shared-types/src/ComponentThemeVariables.ts`
@@ -76,11 +78,13 @@ npm run clean-node && npm install        # Nuclear option (removes all node_modu
 ## Finding Component Information
 
 **To find available components and their packages:**
+
 - Check `/packages/__docs__/src/components.ts` - lists all components with their package locations
 
 **Each component has two READMEs:**
 
 1. **Component README**: `/packages/[package]/src/[Component]/README.md` ⭐
+
    - What the component does and usage examples
    - **Check this first** - it has the detailed information
    - These READMEs are also used to render the component documentation pages on instructure.design
@@ -89,6 +93,7 @@ npm run clean-node && npm install        # Nuclear option (removes all node_modu
    - Package overview, installation, exports
 
 **For complete API details of a component:**
+
 - Props: Check `props.ts` files in component source
 - Theme variables: Check `theme.ts` files in component source
 
@@ -108,6 +113,7 @@ npm run clean-node && npm install        # Nuclear option (removes all node_modu
 A change is **breaking** if it requires consumers to modify their code:
 
 **Breaking changes (avoid):**
+
 - ❌ Removing or renaming a prop
 - ❌ Changing a prop's type or behavior
 - ❌ Removing or renaming a component
@@ -116,6 +122,7 @@ A change is **breaking** if it requires consumers to modify their code:
 - ❌ Removing exported utilities or functions
 
 **Not breaking changes (preferred):**
+
 - ✅ Adding new optional props
 - ✅ Adding new components
 - ✅ Bug fixes that restore intended behavior
@@ -129,11 +136,17 @@ When a breaking change is explicitly requested, document it clearly in the commi
 ## Testing Requirements
 
 All components **MUST**:
+
 1. Have unit tests (Vitest + React Testing Library in `*.test.tsx`)
 2. Have visual regression tests in `/regression-test`
 3. Pass accessibility audits (WCAG 2.1 AA)
 4. Support RTL languages
 
+### Writing tests
+
+- Try to use `vitest`'s fake timers in unit tests
+- When simulating mouse events (e.g., `fireEvent.click`), use `{ button: 0, detail: 1 }` if the click events don't seem to work. This is needed because `FocusRegion.ts/handleDocumentClick` uses `event.detail` and `event.button` to determine if the event was a mouse/touch click.
+
 ### Running Tests
 
 ```bash
@@ -158,11 +171,13 @@ See `/regression-test/README.md` for detailed instructions.
 InstUI uses custom markdown with special code blocks (using [gray-matter](https://github.com/jonschlinkert/gray-matter) YAML syntax) for interactive examples.
 
 **Code block types:**
+
 - `type: code` - Syntax highlighting only
 - `type: embed` - Rendered component only
 - `type: example` - Interactive (rendered + editable) ⭐ Most common
 
 **IMPORTANT:**
+
 - Always write functional component examples with hooks
 - All InstUI components are available without imports in examples
 

packages/ui-a11y-utils/src/FocusRegion.ts

@@ -85,10 +85,12 @@ class FocusRegion {
     this._contextElement = element
     if (options) {
       this._options = options
-      this.addOrRemoveListeners(
-        options.shouldCloseOnDocumentClick,
-        options.shouldCloseOnEscape
-      )
+      if (this._active) {
+        this.addOrRemoveListeners(
+          options.shouldCloseOnDocumentClick,
+          options.shouldCloseOnEscape
+        )
+      }
     }
     if (this._keyboardFocusRegion) {
       this._keyboardFocusRegion.updateElement(element)
@@ -114,6 +116,9 @@ class FocusRegion {
   }
 
   handleDocumentClick = (event: React.PointerEvent) => {
+    // we used event.pointerType === 'mouse' here, but it is not supported in Safari
+    // https://developer.mozilla.org/en-US/docs/Web/API/PointerEvent/pointerType
+    // TODO: Check this in the future, because the linked Webkit bug is marked as fixed in 2025-09
     if (
       this._options.shouldCloseOnDocumentClick &&
       event.button === 0 &&

packages/ui-a11y-utils/src/__tests__/FocusRegion.test.tsx

@@ -68,7 +68,6 @@ describe('FocusRegion', () => {
       const outsideElement = document.createElement('div')
       document.body.appendChild(outsideElement)
 
-      fireEvent.mouseDown(outsideElement, { button: 0 })
       fireEvent.click(outsideElement, { button: 0, detail: 1 })
 
       await waitFor(() => {
@@ -91,7 +90,6 @@ describe('FocusRegion', () => {
       const outsideElement = document.createElement('div')
       document.body.appendChild(outsideElement)
 
-      fireEvent.mouseDown(outsideElement, { button: 0 })
       fireEvent.click(outsideElement, { button: 0, detail: 1 })
 
       // Wait a bit to ensure no dismiss is called
@@ -252,6 +250,42 @@ describe('FocusRegion', () => {
       // The internal state should be updated
       expect(focusRegion).toBeDefined()
     })
+
+    it('should not add event listeners when updateElement is called on inactive region', async () => {
+      const onDismiss = vi.fn()
+      focusRegion = new FocusRegion(container, {
+        shouldCloseOnDocumentClick: false,
+        shouldCloseOnEscape: false,
+        onDismiss
+      })
+
+      // Do NOT activate the region - it should remain inactive
+      expect(focusRegion.focused).toBe(false)
+
+      // Update element with options that would add listeners if region was active
+      const newContainer = document.createElement('div')
+      const newOptions = {
+        shouldCloseOnDocumentClick: true,
+        shouldCloseOnEscape: true,
+        onDismiss
+      }
+
+      focusRegion.updateElement(newContainer, newOptions)
+
+      // Region should still be inactive
+      expect(focusRegion.focused).toBe(false)
+
+      // Click outside the container - should NOT dismiss since no listeners were added
+      const outsideElement = document.createElement('div')
+      document.body.appendChild(outsideElement)
+
+      fireEvent.click(outsideElement, { button: 0, detail: 1 })
+
+      fireEvent.keyUp(document, { keyCode: 27 })
+      expect(onDismiss).not.toHaveBeenCalled()
+
+      document.body.removeChild(outsideElement)
+    })
   })
 
   describe('keyboardFocusable', () => {

packages/ui-drawer-layout/src/DrawerLayout/DrawerTray/props.ts

@@ -102,6 +102,15 @@ type PropsPassedToDialog = {
    */
   liveRegion?: LiveRegion
 
+  /**
+   * Event fired when the underlying FocusRegion is dismissed in overlay mode.
+   * This can happen if:
+   * - `shouldCloseOnDocumentClick` is `true` and the user
+   * clicks outside the `<DrawerLayout.Tray />`
+   * - If `shouldCloseOnEscape` is `true` and the user presses the ESC key.
+   *
+   * This should be used to close the `<DrawerLayout.Tray />` in these cases
+   */
   onDismiss?: (
     event: React.UIEvent | React.FocusEvent,
     documentClick?: boolean

packages/ui-drawer-layout/src/DrawerLayout/__tests__/DrawerLayout.test.tsx

@@ -22,10 +22,14 @@
  * SOFTWARE.
  */
 
-import { render, screen } from '@testing-library/react'
+import { render, screen, waitFor, fireEvent, act } from '@testing-library/react'
+import { vi } from 'vitest'
 import '@testing-library/jest-dom'
 
+import { DrawerLayout } from '../index'
 import DrawerLayoutFixture from '../__fixtures__/DrawerLayout.fixture'
+import { Button } from '@instructure/ui-buttons'
+import { View } from '@instructure/ui-view'
 
 describe('<DrawerLayout />', () => {
   it('should render', () => {
@@ -50,4 +54,48 @@ describe('<DrawerLayout />', () => {
     expect(contentWrapper).toHaveTextContent('Hello from content')
     expect(contentWrapper).toHaveAttribute('aria-label', 'Test DrawerContent')
   })
+
+  it('should close the tray when ESC is pressed in overlay mode', async () => {
+    let open = true
+    const msg = 'This is in the Tray'
+    const onDismiss = vi.fn(() => {
+      open = false
+    })
+    // Small layout width to trigger overlay mode
+    const TestComponent = () => (
+      <View height="25rem" as="div" position="relative">
+        <DrawerLayout minWidth="4444px">
+          <DrawerLayout.Tray
+            id="DrawerLayoutTrayExample1"
+            open={open}
+            label="Drawer Tray Start Example"
+            onDismiss={onDismiss}
+          >
+            <Button onClick={() => (open = false)}>Close Tray</Button>
+            {msg}
+          </DrawerLayout.Tray>
+          <DrawerLayout.Content label="Drawer content example">
+            <Button
+              onClick={() => {
+                open = true
+              }}
+            >
+              Expand tray
+            </Button>
+          </DrawerLayout.Content>
+        </DrawerLayout>
+      </View>
+    )
+    const { rerender } = render(<TestComponent />)
+    expect(screen.getByText(msg)).toBeInTheDocument()
+    await act(() => new Promise((resolve) => requestAnimationFrame(resolve)))
+    fireEvent.keyUp(document, { keyCode: 27 }) // ESC key
+    await waitFor(() => {
+      expect(onDismiss).toHaveBeenCalled()
+    })
+    rerender(<TestComponent />)
+    await waitFor(() => {
+      expect(screen.queryByText(msg)).not.toBeInTheDocument()
+    })
+  })
 })