fix(FocusScope): don't move focus if DOM mutation occurred before any nodes had focus (#2546)

romansp · web-flow · commit 91023867924e · 2026-04-01T15:22:44.000+08:00
* fix(FocusScope): don't move focus if DOM mutation occurred before any nodes had focus Fixes #2506. DOM mutation may be triggered by something unrelated to focus, e.g. class changed on an element, so first check if we even have a previously focused element. Also ensure that captured DOM mutations even have removed nodes. Add test to ensure focus is restored on container on node removal. * test(FocusScope): add test case for Dialog with Select initial focus interaction * chore(FocusScope): remove unnecessary waitFor and use toHaveFocus assertion
diff --git a/packages/core/src/FocusScope/FocusScope.test.ts b/packages/core/src/FocusScope/FocusScope.test.ts
@@ -1,9 +1,11 @@
 import type { RenderResult } from '@testing-library/vue'
 import userEvent from '@testing-library/user-event'
-import { render, waitFor } from '@testing-library/vue'
+import { render } from '@testing-library/vue'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
-import { defineComponent } from 'vue'
+import { defineComponent, nextTick } from 'vue'
 import { FocusScope } from '.'
+import { DialogContent, DialogRoot, DialogTitle, DialogTrigger } from '../Dialog'
+import { SelectContent, SelectItem, SelectRoot, SelectTrigger, SelectValue } from '../Select'
 
 const INNER_NAME_INPUT_LABEL = 'Name'
 const INNER_EMAIL_INPUT_LABEL = 'Email'
@@ -24,6 +26,7 @@ const TestField = ({
 describe('focusScope', () => {
   describe('given a default FocusScope', () => {
     let rendered: RenderResult
+    let focusContainer: HTMLElement
     let tabbableFirst: HTMLInputElement
     let tabbableSecond: HTMLInputElement
     let tabbableLast: HTMLButtonElement
@@ -33,7 +36,7 @@ describe('focusScope', () => {
         components: { TestField, FocusScope },
         template: `<div>
         <FocusScope asChild loop trapped>
-          <form>
+          <form data-testid="focus-scope">
             <TestField label=${INNER_NAME_INPUT_LABEL} />
             <TestField label=${INNER_EMAIL_INPUT_LABEL} />
             <button>${INNER_SUBMIT_LABEL}</button>
@@ -43,6 +46,7 @@ describe('focusScope', () => {
         <button>some outer button</button>
       </div>`,
       }))
+      focusContainer = rendered.getByTestId('focus-scope') as HTMLElement
       tabbableFirst = rendered.getByLabelText(INNER_NAME_INPUT_LABEL) as HTMLInputElement
       tabbableSecond = rendered.getByLabelText(INNER_EMAIL_INPUT_LABEL) as HTMLInputElement
       tabbableLast = rendered.getByText(INNER_SUBMIT_LABEL) as HTMLButtonElement
@@ -57,13 +61,20 @@ describe('focusScope', () => {
     it('should focus the last element in the scope on shift+tab from the first element in scope', async () => {
       tabbableFirst.focus()
       await userEvent.tab({ shift: true })
-      waitFor(() => expect(tabbableLast).toBe(document.activeElement))
+      expect(tabbableLast).toHaveFocus()
     })
 
     it('should focus the first element in scope on tab from the last element in scope', async () => {
       tabbableLast.focus()
       await userEvent.tab()
-      expect(tabbableFirst).toBe(document.activeElement)
+      expect(tabbableFirst).toHaveFocus()
+    })
+
+    it('should focus container when focused element is removed from the DOM', async () => {
+      tabbableFirst.focus()
+      tabbableFirst.remove()
+      await nextTick()
+      expect(focusContainer).toHaveFocus()
     })
   })
 
@@ -94,13 +105,13 @@ describe('focusScope', () => {
     it('should skip the element with a negative tabindex on tab', async () => {
       tabbableLast.focus()
       await userEvent.tab()
-      expect(tabbableSecond).toBe(document.activeElement)
+      expect(tabbableSecond).toHaveFocus()
     })
 
     it('should skip the element with a negative tabindex on shift+tab', async () => {
       tabbableSecond.focus()
       await userEvent.tab({ shift: true })
-      waitFor(() => expect(tabbableLast).toBe(document.activeElement))
+      expect(tabbableLast).toHaveFocus()
     })
   })
 
@@ -111,7 +122,9 @@ describe('focusScope', () => {
     beforeEach(() => {
       rendered = render(defineComponent({
         components: { TestField, FocusScope },
-        props: { handleLastFocusableElementBlur },
+        setup() {
+          return { handleLastFocusableElementBlur }
+        },
         template: `<div>
         <FocusScope asChild loop trapped>
           <form>
@@ -130,7 +143,48 @@ describe('focusScope', () => {
       tabbableFirst.focus()
       await userEvent.tab({ shift: true })
       await userEvent.tab()
-      waitFor(() => expect(handleLastFocusableElementBlur).toHaveBeenCalledTimes(1))
+      expect(handleLastFocusableElementBlur).toHaveBeenCalledTimes(1)
+    })
+  })
+
+  // https://github.com/unovue/reka-ui/issues/2550
+  describe('given a FocusScope with SelectTrigger inside Dialog (#2550)', () => {
+    const DialogWithSelect = defineComponent({
+      components: { DialogRoot, DialogTrigger, DialogContent, DialogTitle, SelectRoot, SelectTrigger, SelectValue, SelectContent, SelectItem },
+      template: `
+        <DialogRoot>
+          <DialogTrigger>Open</DialogTrigger>
+          <DialogContent>
+            <DialogTitle>Test Dialog</DialogTitle>
+            <input data-testid="email-input" type="text" placeholder="you@example.com" />
+            <SelectRoot>
+              <SelectTrigger>
+                <SelectValue placeholder="Select" />
+              </SelectTrigger>
+              <SelectContent>
+                <SelectItem value="a">Option A</SelectItem>
+                <SelectItem value="b">Option B</SelectItem>
+              </SelectContent>
+            </SelectRoot>
+          </DialogContent>
+        </DialogRoot>
+      `,
+    })
+
+    it('should auto-focus the first tabbable element when SelectTrigger is present', async () => {
+      const rendered = render(DialogWithSelect)
+
+      const trigger = rendered.getByRole('button', { name: 'Open' })
+      await userEvent.click(trigger)
+
+      const input1 = rendered.getByTestId('email-input')
+      expect(input1).toHaveFocus()
+
+      // close and reopen, and ensure the input is focused again, not the SelectTrigger
+      await userEvent.keyboard('{Escape}')
+      await userEvent.click(trigger)
+      const inputReopen = rendered.getByTestId('email-input')
+      expect(inputReopen).toHaveFocus()
     })
   })
 })
diff --git a/packages/core/src/FocusScope/FocusScope.vue b/packages/core/src/FocusScope/FocusScope.vue
@@ -117,7 +117,21 @@ watchEffect((cleanupFn) => {
   // if the element still exist inside the container,
   // if not then we focus to the container
   function handleMutations(mutations: MutationRecord[]) {
-    const isLastFocusedElementExist = container.contains(lastFocusedElementRef.value)
+    const lastFocusedElement = lastFocusedElementRef.value
+
+    // Mutation may be triggered by something unrelated to focus, e.g. class is set on an element
+    // so we first check if we even have a previously focused element.
+    if (lastFocusedElement === null) {
+      return
+    }
+
+    // Ensure mutations removed nodes from the DOM at all.
+    const anyNodesRemoved = mutations.some(m => m.removedNodes.length > 0)
+    if (!anyNodesRemoved) {
+      return
+    }
+
+    const isLastFocusedElementExist = container.contains(lastFocusedElement)
     if (!isLastFocusedElementExist)
       focus(container)
   }
