Skip to content

Fix focus not returned to SVG Element #3704

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Apr 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

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

## [2.2.2] - 2025-04-17

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import { history } from '../../utils/active-element-history'
import { isDisabledReactIssue7711 } from '../../utils/bugs'
import { Focus } from '../../utils/calculate-active-index'
import { disposables } from '../../utils/disposables'
import * as DOM from '../../utils/dom'
import { match } from '../../utils/match'
import { isMobile } from '../../utils/platform'
import {
Expand Down Expand Up @@ -1012,8 +1013,8 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
}

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

if (optionsElement?.contains(e.target)) {
Expand Down
33 changes: 19 additions & 14 deletions packages/@headlessui-react/src/components/disclosure/disclosure.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
} from '../../internal/open-closed'
import type { Props } from '../../types'
import { isDisabledReactIssue7711 } from '../../utils/bugs'
import * as DOM from '../../utils/dom'
import { match } from '../../utils/match'
import { getOwnerDocument } from '../../utils/owner'
import {
Expand Down Expand Up @@ -185,7 +186,7 @@ function DisclosureFn<TTag extends ElementType = typeof DEFAULT_DISCLOSURE_TAG>(
ref,
optionalRef(
(ref) => {
internalDisclosureRef.current = ref as HTMLElement | null
internalDisclosureRef.current = ref
},
props.as === undefined ||
// @ts-expect-error The `as` prop _can_ be a Fragment
Expand All @@ -202,22 +203,26 @@ function DisclosureFn<TTag extends ElementType = typeof DEFAULT_DISCLOSURE_TAG>(
} as StateDefinition)
let [{ disclosureState, buttonId }, dispatch] = reducerBag

let close = useEvent((focusableElement?: HTMLElement | MutableRefObject<HTMLElement | null>) => {
dispatch({ type: ActionTypes.CloseDisclosure })
let ownerDocument = getOwnerDocument(internalDisclosureRef)
if (!ownerDocument) return
if (!buttonId) return
let close = useEvent(
(focusableElement?: HTMLOrSVGElement | MutableRefObject<HTMLOrSVGElement | null>) => {
dispatch({ type: ActionTypes.CloseDisclosure })
let ownerDocument = getOwnerDocument(internalDisclosureRef)
if (!ownerDocument) return
if (!buttonId) return

let restoreElement = (() => {
if (!focusableElement) return ownerDocument.getElementById(buttonId)
if (focusableElement instanceof HTMLElement) return focusableElement
if (focusableElement.current instanceof HTMLElement) return focusableElement.current
let restoreElement = (() => {
if (!focusableElement) return ownerDocument.getElementById(buttonId)
if (DOM.isHTMLorSVGElement(focusableElement)) return focusableElement
if ('current' in focusableElement && DOM.isHTMLorSVGElement(focusableElement.current)) {
return focusableElement.current
}

return ownerDocument.getElementById(buttonId)
})()
return ownerDocument.getElementById(buttonId)
})()

restoreElement?.focus()
})
restoreElement?.focus()
}
)

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

Expand Down
31 changes: 16 additions & 15 deletions packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,26 @@ import { useWatch } from '../../hooks/use-watch'
import { Hidden, HiddenFeatures } from '../../internal/hidden'
import type { Props } from '../../types'
import { history } from '../../utils/active-element-history'
import * as DOM from '../../utils/dom'
import { Focus, FocusResult, focusElement, focusIn } from '../../utils/focus-management'
import { match } from '../../utils/match'
import { microTask } from '../../utils/micro-task'
import { forwardRefWithAs, useRender, type HasDisplayName, type RefProp } from '../../utils/render'

type Containers =
// Lazy resolved containers
| (() => Iterable<HTMLElement>)
| (() => Iterable<Element>)

// List of containers
| MutableRefObject<Set<MutableRefObject<HTMLElement | null>>>
| MutableRefObject<Set<MutableRefObject<Element | null>>>

function resolveContainers(containers?: Containers): Set<HTMLElement> {
function resolveContainers(containers?: Containers): Set<Element> {
if (!containers) return new Set<HTMLElement>()
if (typeof containers === 'function') return new Set(containers())

let all = new Set<HTMLElement>()
let all = new Set<Element>()
for (let container of containers.current) {
if (container.current instanceof HTMLElement) {
if (DOM.isElement(container.current)) {
all.add(container.current)
}
}
Expand Down Expand Up @@ -121,8 +122,8 @@ function FocusTrapFn<TTag extends ElementType = typeof DEFAULT_FOCUS_TRAP_TAG>(

let direction = useTabDirection()
let handleFocus = useEvent((e: ReactFocusEvent) => {
let el = container.current as HTMLElement
if (!el) return
if (!DOM.isHTMLElement(container.current)) return
let el = container.current

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

let allContainers = resolveContainers(containers)
if (container.current instanceof HTMLElement) allContainers.add(container.current)
if (DOM.isHTMLElement(container.current)) allContainers.add(container.current)

let relatedTarget = e.relatedTarget
if (!(relatedTarget instanceof HTMLElement)) return
if (!DOM.isHTMLorSVGElement(relatedTarget)) return

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

// It was invoked via something else (e.g.: click, programmatically, ...). Redirect to the
// previous active item in the FocusTrap
else if (e.target instanceof HTMLElement) {
else if (DOM.isHTMLorSVGElement(e.target)) {
focusElement(e.target)
}
}
Expand Down Expand Up @@ -247,7 +248,7 @@ export let FocusTrap = Object.assign(FocusTrapRoot, {
// ---

function useRestoreElement(enabled: boolean = true) {
let localHistory = useRef<HTMLElement[]>(history.slice())
let localHistory = useRef(history.slice())

useWatch(
([newEnabled], [oldEnabled]) => {
Expand Down Expand Up @@ -418,7 +419,7 @@ function useFocusLock(
ownerDocument: Document | null
container: MutableRefObject<HTMLElement | null>
containers?: Containers
previousActiveElement: MutableRefObject<HTMLElement | null>
previousActiveElement: MutableRefObject<HTMLOrSVGElement | null>
}
) {
let mounted = useIsMounted()
Expand All @@ -433,14 +434,14 @@ function useFocusLock(
if (!mounted.current) return

let allContainers = resolveContainers(containers)
if (container.current instanceof HTMLElement) allContainers.add(container.current)
if (DOM.isHTMLElement(container.current)) allContainers.add(container.current)

let previous = previousActiveElement.current
if (!previous) return

let toElement = event.target as HTMLElement | null

if (toElement && toElement instanceof HTMLElement) {
if (DOM.isHTMLElement(toElement)) {
if (!contains(allContainers, toElement)) {
event.preventDefault()
event.stopPropagation()
Expand All @@ -457,7 +458,7 @@ function useFocusLock(
)
}

function contains(containers: Set<HTMLElement>, element: HTMLElement) {
function contains(containers: Set<Element>, element: Element) {
for (let container of containers) {
if (container.contains(element)) return true
}
Expand Down
6 changes: 3 additions & 3 deletions packages/@headlessui-react/src/components/label/label.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ function LabelFn<TTag extends ElementType = typeof DEFAULT_LABEL_TAG>(
// Labels connected to 'real' controls will already click the element. But we don't know that
// ahead of time. This will prevent the default click, such that only a single click happens
// instead of two. Otherwise this results in a visual no-op.
if (current instanceof HTMLLabelElement) {
if (DOM.isHTMLLabelElement(current)) {
e.preventDefault()
}

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

if (current instanceof HTMLLabelElement) {
if (DOM.isHTMLLabelElement(current)) {
let target = document.getElementById(current.htmlFor)
if (target) {
// Bail if the target element is disabled
Expand All @@ -186,7 +186,7 @@ function LabelFn<TTag extends ElementType = typeof DEFAULT_LABEL_TAG>(
// immediately require state changes, e.g.: Radio & Checkbox inputs need to be checked (or
// unchecked).
if (
(target instanceof HTMLInputElement &&
(DOM.isHTMLInputElement(target) &&
(target.type === 'file' || target.type === 'radio' || target.type === 'checkbox')) ||
target.role === 'radio' ||
target.role === 'checkbox' ||
Expand Down
5 changes: 3 additions & 2 deletions packages/@headlessui-react/src/components/listbox/listbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import type { EnsureArray, Props } from '../../types'
import { isDisabledReactIssue7711 } from '../../utils/bugs'
import { Focus } from '../../utils/calculate-active-index'
import { disposables } from '../../utils/disposables'
import * as DOM from '../../utils/dom'
import {
Focus as FocusManagementFocus,
FocusableMode,
Expand Down Expand Up @@ -376,8 +377,8 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
}

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

if (optionsElement?.contains(e.target)) {
Expand Down
5 changes: 3 additions & 2 deletions packages/@headlessui-react/src/components/menu/menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import type { Props } from '../../types'
import { isDisabledReactIssue7711 } from '../../utils/bugs'
import { Focus } from '../../utils/calculate-active-index'
import { disposables } from '../../utils/disposables'
import * as DOM from '../../utils/dom'
import {
Focus as FocusManagementFocus,
FocusableMode,
Expand Down Expand Up @@ -241,8 +242,8 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
}

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

if (itemsElement?.contains(e.target)) {
Expand Down
11 changes: 6 additions & 5 deletions packages/@headlessui-react/src/components/popover/popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import {
} from '../../internal/open-closed'
import type { Props } from '../../types'
import { isDisabledReactIssue7711 } from '../../utils/bugs'
import * as DOM from '../../utils/dom'
import {
Focus,
FocusResult,
Expand Down Expand Up @@ -358,7 +359,7 @@ function PopoverFn<TTag extends ElementType = typeof DEFAULT_POPOVER_TAG>(
'focus',
(event) => {
if (event.target === window) return
if (!(event.target instanceof HTMLElement)) return
if (!DOM.isHTMLorSVGElement(event.target)) return
if (popoverState !== PopoverStates.Open) return
if (isFocusWithinPopoverGroup()) return
if (!button) return
Expand Down Expand Up @@ -395,8 +396,8 @@ function PopoverFn<TTag extends ElementType = typeof DEFAULT_POPOVER_TAG>(

let restoreElement = (() => {
if (!focusableElement) return button
if (focusableElement instanceof HTMLElement) return focusableElement
if ('current' in focusableElement && focusableElement.current instanceof HTMLElement)
if (DOM.isHTMLElement(focusableElement)) return focusableElement
if ('current' in focusableElement && DOM.isHTMLElement(focusableElement.current))
return focusableElement.current

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

let direction = useTabDirection()
let handleFocus = useEvent(() => {
let el = state.panel as HTMLElement
if (!el) return
if (!DOM.isHTMLElement(state.panel)) return
let el = state.panel

function run() {
let result = match(direction.current, {
Expand Down
3 changes: 2 additions & 1 deletion packages/@headlessui-react/src/components/portal/portal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { useServerHandoffComplete } from '../../hooks/use-server-handoff-complet
import { optionalRef, useSyncRefs } from '../../hooks/use-sync-refs'
import { usePortalRoot } from '../../internal/portal-force-root'
import type { Props } from '../../types'
import * as DOM from '../../utils/dom'
import { env } from '../../utils/env'
import { forwardRefWithAs, useRender, type HasDisplayName, type RefProp } from '../../utils/render'

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

if (element instanceof Node && target.contains(element)) {
if (DOM.isNode(element) && target.contains(element)) {
target.removeChild(element)
}

Expand Down
3 changes: 2 additions & 1 deletion packages/@headlessui-react/src/components/switch/switch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { FormFields } from '../../internal/form-fields'
import { useProvidedId } from '../../internal/id'
import type { Props } from '../../types'
import { isDisabledReactIssue7711 } from '../../utils/bugs'
import * as DOM from '../../utils/dom'
import { attemptSubmit } from '../../utils/form'
import {
forwardRefWithAs,
Expand Down Expand Up @@ -85,7 +86,7 @@ function GroupFn<TTag extends ElementType = typeof DEFAULT_GROUP_TAG>(
htmlFor: context.switch?.id,
onClick(event: React.MouseEvent<HTMLLabelElement>) {
if (!switchElement) return
if (event.currentTarget instanceof HTMLLabelElement) {
if (DOM.isHTMLLabelElement(event.currentTarget)) {
event.preventDefault()
}
switchElement.click()
Expand Down
Loading