Skip to content

Commit 30a6d51

Browse files
authored
Fix focus not returned to SVG Element (#3704)
This PR fixes an issue where the focus is not returned to an `SVG` element with a `tabIndex` correctly. There are a few issues going on here: 1. We assume that the element to focus (`e.target`) is an instanceof `HTMLElement`, but the `SVGElement` is not an instanceof `HTMLElement`. 2. By using `instanceof` we are checking against concrete classes, so if this happen to cross certain contexts (Shadow DOM, Iframes, ...) then the instances would be different. To solve this, we will now: 1. Relax the types and only care about the actual attributes and methods we are interested in. In most cases this means changing internal types from `HTMLElement` to `Element` for example. 2. We will check whether certain properties are available in the object to deduce the correct type from the object. Fixes: #3660 ## Test plan Added an SVG to open a Dialog component and made sure that pressing `escape` or clicking outside of the Dialog does restore the focus to the SVG itself. ```tsx <svg tabIndex={0} onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { setIsOpen((v) => !v) } }} onClick={() => setIsOpen((v) => !v)} className="h-6 w-6 text-gray-500" > <BookOpenIcon /> </svg> ``` Here is a video of that behavior: https://github.com/user-attachments/assets/1805ca67-8bc7-4315-98a7-2490cba9230c
1 parent 0558bdb commit 30a6d51

25 files changed

+177
-105
lines changed

packages/@headlessui-react/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1515

1616
- Fix clicking `Label` component should open `<Input type="file">` ([#3707](https://github.com/tailwindlabs/headlessui/pull/3707))
1717
- Ensure clicking on interactive elements inside `Label` component works ([#3709](https://github.com/tailwindlabs/headlessui/pull/3709))
18+
- Fix focus not returned to SVG Element ([#3704](https://github.com/tailwindlabs/headlessui/pull/3704))
1819

1920
## [2.2.2] - 2025-04-17
2021

packages/@headlessui-react/src/components/combobox/combobox.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ import { history } from '../../utils/active-element-history'
6363
import { isDisabledReactIssue7711 } from '../../utils/bugs'
6464
import { Focus } from '../../utils/calculate-active-index'
6565
import { disposables } from '../../utils/disposables'
66+
import * as DOM from '../../utils/dom'
6667
import { match } from '../../utils/match'
6768
import { isMobile } from '../../utils/platform'
6869
import {
@@ -1012,8 +1013,8 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
10121013
}
10131014

10141015
let option = e.target.closest('[role="option"]:not([data-disabled])')
1015-
if (option !== null) {
1016-
return QuickReleaseAction.Select(option as HTMLElement)
1016+
if (DOM.isHTMLElement(option)) {
1017+
return QuickReleaseAction.Select(option)
10171018
}
10181019

10191020
if (optionsElement?.contains(e.target)) {

packages/@headlessui-react/src/components/disclosure/disclosure.tsx

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import {
3535
} from '../../internal/open-closed'
3636
import type { Props } from '../../types'
3737
import { isDisabledReactIssue7711 } from '../../utils/bugs'
38+
import * as DOM from '../../utils/dom'
3839
import { match } from '../../utils/match'
3940
import { getOwnerDocument } from '../../utils/owner'
4041
import {
@@ -185,7 +186,7 @@ function DisclosureFn<TTag extends ElementType = typeof DEFAULT_DISCLOSURE_TAG>(
185186
ref,
186187
optionalRef(
187188
(ref) => {
188-
internalDisclosureRef.current = ref as HTMLElement | null
189+
internalDisclosureRef.current = ref
189190
},
190191
props.as === undefined ||
191192
// @ts-expect-error The `as` prop _can_ be a Fragment
@@ -202,22 +203,26 @@ function DisclosureFn<TTag extends ElementType = typeof DEFAULT_DISCLOSURE_TAG>(
202203
} as StateDefinition)
203204
let [{ disclosureState, buttonId }, dispatch] = reducerBag
204205

205-
let close = useEvent((focusableElement?: HTMLElement | MutableRefObject<HTMLElement | null>) => {
206-
dispatch({ type: ActionTypes.CloseDisclosure })
207-
let ownerDocument = getOwnerDocument(internalDisclosureRef)
208-
if (!ownerDocument) return
209-
if (!buttonId) return
206+
let close = useEvent(
207+
(focusableElement?: HTMLOrSVGElement | MutableRefObject<HTMLOrSVGElement | null>) => {
208+
dispatch({ type: ActionTypes.CloseDisclosure })
209+
let ownerDocument = getOwnerDocument(internalDisclosureRef)
210+
if (!ownerDocument) return
211+
if (!buttonId) return
210212

211-
let restoreElement = (() => {
212-
if (!focusableElement) return ownerDocument.getElementById(buttonId)
213-
if (focusableElement instanceof HTMLElement) return focusableElement
214-
if (focusableElement.current instanceof HTMLElement) return focusableElement.current
213+
let restoreElement = (() => {
214+
if (!focusableElement) return ownerDocument.getElementById(buttonId)
215+
if (DOM.isHTMLorSVGElement(focusableElement)) return focusableElement
216+
if ('current' in focusableElement && DOM.isHTMLorSVGElement(focusableElement.current)) {
217+
return focusableElement.current
218+
}
215219

216-
return ownerDocument.getElementById(buttonId)
217-
})()
220+
return ownerDocument.getElementById(buttonId)
221+
})()
218222

219-
restoreElement?.focus()
220-
})
223+
restoreElement?.focus()
224+
}
225+
)
221226

222227
let api = useMemo<ContextType<typeof DisclosureAPIContext>>(() => ({ close }), [close])
223228

packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,25 +21,26 @@ import { useWatch } from '../../hooks/use-watch'
2121
import { Hidden, HiddenFeatures } from '../../internal/hidden'
2222
import type { Props } from '../../types'
2323
import { history } from '../../utils/active-element-history'
24+
import * as DOM from '../../utils/dom'
2425
import { Focus, FocusResult, focusElement, focusIn } from '../../utils/focus-management'
2526
import { match } from '../../utils/match'
2627
import { microTask } from '../../utils/micro-task'
2728
import { forwardRefWithAs, useRender, type HasDisplayName, type RefProp } from '../../utils/render'
2829

2930
type Containers =
3031
// Lazy resolved containers
31-
| (() => Iterable<HTMLElement>)
32+
| (() => Iterable<Element>)
3233

3334
// List of containers
34-
| MutableRefObject<Set<MutableRefObject<HTMLElement | null>>>
35+
| MutableRefObject<Set<MutableRefObject<Element | null>>>
3536

36-
function resolveContainers(containers?: Containers): Set<HTMLElement> {
37+
function resolveContainers(containers?: Containers): Set<Element> {
3738
if (!containers) return new Set<HTMLElement>()
3839
if (typeof containers === 'function') return new Set(containers())
3940

40-
let all = new Set<HTMLElement>()
41+
let all = new Set<Element>()
4142
for (let container of containers.current) {
42-
if (container.current instanceof HTMLElement) {
43+
if (DOM.isElement(container.current)) {
4344
all.add(container.current)
4445
}
4546
}
@@ -121,8 +122,8 @@ function FocusTrapFn<TTag extends ElementType = typeof DEFAULT_FOCUS_TRAP_TAG>(
121122

122123
let direction = useTabDirection()
123124
let handleFocus = useEvent((e: ReactFocusEvent) => {
124-
let el = container.current as HTMLElement
125-
if (!el) return
125+
if (!DOM.isHTMLElement(container.current)) return
126+
let el = container.current
126127

127128
// TODO: Cleanup once we are using real browser tests
128129
let wrapper = process.env.NODE_ENV === 'test' ? microTask : (cb: Function) => cb()
@@ -163,10 +164,10 @@ function FocusTrapFn<TTag extends ElementType = typeof DEFAULT_FOCUS_TRAP_TAG>(
163164
if (!(features & FocusTrapFeatures.FocusLock)) return
164165

165166
let allContainers = resolveContainers(containers)
166-
if (container.current instanceof HTMLElement) allContainers.add(container.current)
167+
if (DOM.isHTMLElement(container.current)) allContainers.add(container.current)
167168

168169
let relatedTarget = e.relatedTarget
169-
if (!(relatedTarget instanceof HTMLElement)) return
170+
if (!DOM.isHTMLorSVGElement(relatedTarget)) return
170171

171172
// Known guards, leave them alone!
172173
if (relatedTarget.dataset.headlessuiFocusGuard === 'true') {
@@ -190,7 +191,7 @@ function FocusTrapFn<TTag extends ElementType = typeof DEFAULT_FOCUS_TRAP_TAG>(
190191

191192
// It was invoked via something else (e.g.: click, programmatically, ...). Redirect to the
192193
// previous active item in the FocusTrap
193-
else if (e.target instanceof HTMLElement) {
194+
else if (DOM.isHTMLorSVGElement(e.target)) {
194195
focusElement(e.target)
195196
}
196197
}
@@ -247,7 +248,7 @@ export let FocusTrap = Object.assign(FocusTrapRoot, {
247248
// ---
248249

249250
function useRestoreElement(enabled: boolean = true) {
250-
let localHistory = useRef<HTMLElement[]>(history.slice())
251+
let localHistory = useRef(history.slice())
251252

252253
useWatch(
253254
([newEnabled], [oldEnabled]) => {
@@ -418,7 +419,7 @@ function useFocusLock(
418419
ownerDocument: Document | null
419420
container: MutableRefObject<HTMLElement | null>
420421
containers?: Containers
421-
previousActiveElement: MutableRefObject<HTMLElement | null>
422+
previousActiveElement: MutableRefObject<HTMLOrSVGElement | null>
422423
}
423424
) {
424425
let mounted = useIsMounted()
@@ -433,14 +434,14 @@ function useFocusLock(
433434
if (!mounted.current) return
434435

435436
let allContainers = resolveContainers(containers)
436-
if (container.current instanceof HTMLElement) allContainers.add(container.current)
437+
if (DOM.isHTMLElement(container.current)) allContainers.add(container.current)
437438

438439
let previous = previousActiveElement.current
439440
if (!previous) return
440441

441442
let toElement = event.target as HTMLElement | null
442443

443-
if (toElement && toElement instanceof HTMLElement) {
444+
if (DOM.isHTMLElement(toElement)) {
444445
if (!contains(allContainers, toElement)) {
445446
event.preventDefault()
446447
event.stopPropagation()
@@ -457,7 +458,7 @@ function useFocusLock(
457458
)
458459
}
459460

460-
function contains(containers: Set<HTMLElement>, element: HTMLElement) {
461+
function contains(containers: Set<Element>, element: Element) {
461462
for (let container of containers) {
462463
if (container.contains(element)) return true
463464
}

packages/@headlessui-react/src/components/label/label.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ function LabelFn<TTag extends ElementType = typeof DEFAULT_LABEL_TAG>(
155155
// Labels connected to 'real' controls will already click the element. But we don't know that
156156
// ahead of time. This will prevent the default click, such that only a single click happens
157157
// instead of two. Otherwise this results in a visual no-op.
158-
if (current instanceof HTMLLabelElement) {
158+
if (DOM.isHTMLLabelElement(current)) {
159159
e.preventDefault()
160160
}
161161

@@ -168,7 +168,7 @@ function LabelFn<TTag extends ElementType = typeof DEFAULT_LABEL_TAG>(
168168
context.props.onClick(e)
169169
}
170170

171-
if (current instanceof HTMLLabelElement) {
171+
if (DOM.isHTMLLabelElement(current)) {
172172
let target = document.getElementById(current.htmlFor)
173173
if (target) {
174174
// Bail if the target element is disabled
@@ -186,7 +186,7 @@ function LabelFn<TTag extends ElementType = typeof DEFAULT_LABEL_TAG>(
186186
// immediately require state changes, e.g.: Radio & Checkbox inputs need to be checked (or
187187
// unchecked).
188188
if (
189-
(target instanceof HTMLInputElement &&
189+
(DOM.isHTMLInputElement(target) &&
190190
(target.type === 'file' || target.type === 'radio' || target.type === 'checkbox')) ||
191191
target.role === 'radio' ||
192192
target.role === 'checkbox' ||

packages/@headlessui-react/src/components/listbox/listbox.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ import type { EnsureArray, Props } from '../../types'
6060
import { isDisabledReactIssue7711 } from '../../utils/bugs'
6161
import { Focus } from '../../utils/calculate-active-index'
6262
import { disposables } from '../../utils/disposables'
63+
import * as DOM from '../../utils/dom'
6364
import {
6465
Focus as FocusManagementFocus,
6566
FocusableMode,
@@ -376,8 +377,8 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
376377
}
377378

378379
let option = e.target.closest('[role="option"]:not([data-disabled])')
379-
if (option !== null) {
380-
return QuickReleaseAction.Select(option as HTMLElement)
380+
if (DOM.isHTMLElement(option)) {
381+
return QuickReleaseAction.Select(option)
381382
}
382383

383384
if (optionsElement?.contains(e.target)) {

packages/@headlessui-react/src/components/menu/menu.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import type { Props } from '../../types'
5151
import { isDisabledReactIssue7711 } from '../../utils/bugs'
5252
import { Focus } from '../../utils/calculate-active-index'
5353
import { disposables } from '../../utils/disposables'
54+
import * as DOM from '../../utils/dom'
5455
import {
5556
Focus as FocusManagementFocus,
5657
FocusableMode,
@@ -241,8 +242,8 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
241242
}
242243

243244
let item = e.target.closest('[role="menuitem"]:not([data-disabled])')
244-
if (item !== null) {
245-
return QuickReleaseAction.Select(item as HTMLElement)
245+
if (DOM.isHTMLElement(item)) {
246+
return QuickReleaseAction.Select(item)
246247
}
247248

248249
if (itemsElement?.contains(e.target)) {

packages/@headlessui-react/src/components/popover/popover.tsx

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ import {
5959
} from '../../internal/open-closed'
6060
import type { Props } from '../../types'
6161
import { isDisabledReactIssue7711 } from '../../utils/bugs'
62+
import * as DOM from '../../utils/dom'
6263
import {
6364
Focus,
6465
FocusResult,
@@ -358,7 +359,7 @@ function PopoverFn<TTag extends ElementType = typeof DEFAULT_POPOVER_TAG>(
358359
'focus',
359360
(event) => {
360361
if (event.target === window) return
361-
if (!(event.target instanceof HTMLElement)) return
362+
if (!DOM.isHTMLorSVGElement(event.target)) return
362363
if (popoverState !== PopoverStates.Open) return
363364
if (isFocusWithinPopoverGroup()) return
364365
if (!button) return
@@ -395,8 +396,8 @@ function PopoverFn<TTag extends ElementType = typeof DEFAULT_POPOVER_TAG>(
395396

396397
let restoreElement = (() => {
397398
if (!focusableElement) return button
398-
if (focusableElement instanceof HTMLElement) return focusableElement
399-
if ('current' in focusableElement && focusableElement.current instanceof HTMLElement)
399+
if (DOM.isHTMLElement(focusableElement)) return focusableElement
400+
if ('current' in focusableElement && DOM.isHTMLElement(focusableElement.current))
400401
return focusableElement.current
401402

402403
return button
@@ -679,8 +680,8 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
679680

680681
let direction = useTabDirection()
681682
let handleFocus = useEvent(() => {
682-
let el = state.panel as HTMLElement
683-
if (!el) return
683+
if (!DOM.isHTMLElement(state.panel)) return
684+
let el = state.panel
684685

685686
function run() {
686687
let result = match(direction.current, {

packages/@headlessui-react/src/components/portal/portal.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { useServerHandoffComplete } from '../../hooks/use-server-handoff-complet
2222
import { optionalRef, useSyncRefs } from '../../hooks/use-sync-refs'
2323
import { usePortalRoot } from '../../internal/portal-force-root'
2424
import type { Props } from '../../types'
25+
import * as DOM from '../../utils/dom'
2526
import { env } from '../../utils/env'
2627
import { forwardRefWithAs, useRender, type HasDisplayName, type RefProp } from '../../utils/render'
2728

@@ -120,7 +121,7 @@ let InternalPortalFn = forwardRefWithAs(function InternalPortalFn<
120121
useOnUnmount(() => {
121122
if (!target || !element) return
122123

123-
if (element instanceof Node && target.contains(element)) {
124+
if (DOM.isNode(element) && target.contains(element)) {
124125
target.removeChild(element)
125126
}
126127

packages/@headlessui-react/src/components/switch/switch.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import { FormFields } from '../../internal/form-fields'
2828
import { useProvidedId } from '../../internal/id'
2929
import type { Props } from '../../types'
3030
import { isDisabledReactIssue7711 } from '../../utils/bugs'
31+
import * as DOM from '../../utils/dom'
3132
import { attemptSubmit } from '../../utils/form'
3233
import {
3334
forwardRefWithAs,
@@ -85,7 +86,7 @@ function GroupFn<TTag extends ElementType = typeof DEFAULT_GROUP_TAG>(
8586
htmlFor: context.switch?.id,
8687
onClick(event: React.MouseEvent<HTMLLabelElement>) {
8788
if (!switchElement) return
88-
if (event.currentTarget instanceof HTMLLabelElement) {
89+
if (DOM.isHTMLLabelElement(event.currentTarget)) {
8990
event.preventDefault()
9091
}
9192
switchElement.click()

0 commit comments

Comments
 (0)