Internal refactor: use flushSync() instead of d.nextFrame() (#3263)

* use `flushSync` instead of `d.nextFrame`

This guarantees that after the `flushSync` call the DOM is updated. This
means that we don't have to guess and delay by a double
`requestAnimationFrame` (`nextFrame`) and _hope_ that the DOM was
updated already.

* inline disposables call

Each function in the `disposables()` object returns a cleanup function
which means we can return this directly.

* inline if-statements

Small one, but consistent with `<Menu />` and `<Listbox />` components.

* inline `flushSync()` callbacks
This commit is contained in:
Robin Malfait
2024-06-03 16:17:16 +02:00
committed by GitHub
parent 479853d5ed
commit 2d3ec80314
3 changed files with 49 additions and 73 deletions
@@ -21,6 +21,7 @@ import React, {
type MouseEvent as ReactMouseEvent,
type Ref,
} from 'react'
import { flushSync } from 'react-dom'
import { useActivePress } from '../../hooks/use-active-press'
import { useByComparator, type ByComparator } from '../../hooks/use-by-comparator'
import { useControllable } from '../../hooks/use-controllable'
@@ -1189,12 +1190,8 @@ function InputFn<
return match(data.comboboxState, {
[ComboboxState.Open]: () => actions.goToOption(Focus.Previous),
[ComboboxState.Closed]: () => {
actions.openCombobox()
d.nextFrame(() => {
if (!data.value) {
actions.goToOption(Focus.Last)
}
})
flushSync(() => actions.openCombobox())
if (!data.value) actions.goToOption(Focus.Last)
},
})
@@ -1320,14 +1317,12 @@ function InputFn<
if (!data.immediate) return
if (data.comboboxState === ComboboxState.Open) return
actions.openCombobox()
flushSync(() => actions.openCombobox())
// We need to make sure that tabbing through a form doesn't result in incorrectly setting the
// value of the combobox. We will set the activation trigger to `Focus`, and we will ignore
// selecting the active option when the user tabs away.
d.nextFrame(() => {
actions.setActivationTrigger(ActivationTrigger.Focus)
})
actions.setActivationTrigger(ActivationTrigger.Focus)
})
let labelledBy = useLabelledBy()
@@ -1439,7 +1434,6 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
autoFocus = false,
...theirProps
} = props
let d = useDisposables()
let refocusInput = useRefocusableInput(data.inputRef)
@@ -1452,37 +1446,30 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
event.preventDefault()
event.stopPropagation()
if (data.comboboxState === ComboboxState.Closed) {
actions.openCombobox()
flushSync(() => actions.openCombobox())
}
return d.nextFrame(() => refocusInput())
refocusInput()
return
case Keys.ArrowDown:
event.preventDefault()
event.stopPropagation()
if (data.comboboxState === ComboboxState.Closed) {
actions.openCombobox()
d.nextFrame(() => {
if (!data.value) {
actions.goToOption(Focus.First)
}
})
flushSync(() => actions.openCombobox())
if (!data.value) actions.goToOption(Focus.First)
}
return d.nextFrame(() => refocusInput())
refocusInput()
return
case Keys.ArrowUp:
event.preventDefault()
event.stopPropagation()
if (data.comboboxState === ComboboxState.Closed) {
actions.openCombobox()
d.nextFrame(() => {
if (!data.value) {
actions.goToOption(Focus.Last)
}
})
flushSync(() => actions.openCombobox())
if (!data.value) actions.goToOption(Focus.Last)
}
return d.nextFrame(() => refocusInput())
refocusInput()
return
case Keys.Escape:
if (data.comboboxState !== ComboboxState.Open) return
@@ -1490,8 +1477,9 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
if (data.optionsRef.current && !data.optionsPropsRef.current.static) {
event.stopPropagation()
}
actions.closeCombobox()
return d.nextFrame(() => refocusInput())
flushSync(() => actions.closeCombobox())
refocusInput()
return
default:
return
@@ -20,6 +20,7 @@ import React, {
type MouseEvent as ReactMouseEvent,
type Ref,
} from 'react'
import { flushSync } from 'react-dom'
import { useActivePress } from '../../hooks/use-active-press'
import { useByComparator, type ByComparator } from '../../hooks/use-by-comparator'
import { useComputed } from '../../hooks/use-computed'
@@ -755,8 +756,6 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
let buttonRef = useSyncRefs(data.buttonRef, ref, useFloatingReference())
let getFloatingReferenceProps = useFloatingReferenceProps()
let d = useDisposables()
let handleKeyDown = useEvent((event: ReactKeyboardEvent<HTMLButtonElement>) => {
switch (event.key) {
// Ref: https://www.w3.org/WAI/ARIA/apg/patterns/menubutton/#keyboard-interaction-13
@@ -768,18 +767,14 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
case Keys.Space:
case Keys.ArrowDown:
event.preventDefault()
actions.openListbox()
d.nextFrame(() => {
if (!data.value) actions.goToOption(Focus.First)
})
flushSync(() => actions.openListbox())
if (!data.value) actions.goToOption(Focus.First)
break
case Keys.ArrowUp:
event.preventDefault()
actions.openListbox()
d.nextFrame(() => {
if (!data.value) actions.goToOption(Focus.Last)
})
flushSync(() => actions.openListbox())
if (!data.value) actions.goToOption(Focus.Last)
break
}
})
@@ -798,8 +793,8 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
let handleClick = useEvent((event: ReactMouseEvent) => {
if (isDisabledReactIssue7711(event.currentTarget)) return event.preventDefault()
if (data.listboxState === ListboxStates.Open) {
actions.closeListbox()
d.nextFrame(() => data.buttonRef.current?.focus({ preventScroll: true }))
flushSync(() => actions.closeListbox())
data.buttonRef.current?.focus({ preventScroll: true })
} else {
event.preventDefault()
actions.openListbox()
@@ -995,7 +990,6 @@ function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(
let getFloatingPanelProps = useFloatingPanelProps()
let optionsRef = useSyncRefs(data.optionsRef, ref, anchor ? floatingRef : null)
let d = useDisposables()
let searchDisposables = useDisposables()
useEffect(() => {
@@ -1030,8 +1024,8 @@ function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(
actions.onChange(dataRef.current.value)
}
if (data.mode === ValueMode.Single) {
actions.closeListbox()
disposables().nextFrame(() => data.buttonRef.current?.focus({ preventScroll: true }))
flushSync(() => actions.closeListbox())
data.buttonRef.current?.focus({ preventScroll: true })
}
break
@@ -1060,8 +1054,9 @@ function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(
case Keys.Escape:
event.preventDefault()
event.stopPropagation()
actions.closeListbox()
return d.nextFrame(() => data.buttonRef.current?.focus({ preventScroll: true }))
flushSync(() => actions.closeListbox())
data.buttonRef.current?.focus({ preventScroll: true })
return
case Keys.Tab:
event.preventDefault()
@@ -1205,11 +1200,9 @@ function OptionFn<
if (data.listboxState !== ListboxStates.Open) return
if (!active) return
if (data.activationTrigger === ActivationTrigger.Pointer) return
let d = disposables()
d.requestAnimationFrame(() => {
return disposables().requestAnimationFrame(() => {
internalOptionRef.current?.scrollIntoView?.({ block: 'nearest' })
})
return d.dispose
}, [
internalOptionRef,
active,
@@ -1228,8 +1221,8 @@ function OptionFn<
if (disabled) return event.preventDefault()
actions.onChange(value)
if (data.mode === ValueMode.Single) {
actions.closeListbox()
disposables().nextFrame(() => data.buttonRef.current?.focus({ preventScroll: true }))
flushSync(() => actions.closeListbox())
data.buttonRef.current?.focus({ preventScroll: true })
}
})
@@ -20,6 +20,7 @@ import React, {
type MouseEvent as ReactMouseEvent,
type Ref,
} from 'react'
import { flushSync } from 'react-dom'
import { useActivePress } from '../../hooks/use-active-press'
import { useDidElementMove } from '../../hooks/use-did-element-move'
import { useDisposables } from '../../hooks/use-disposables'
@@ -469,8 +470,6 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
let getFloatingReferenceProps = useFloatingReferenceProps()
let buttonRef = useSyncRefs(state.buttonRef, ref, useFloatingReference())
let d = useDisposables()
let handleKeyDown = useEvent((event: ReactKeyboardEvent<HTMLButtonElement>) => {
switch (event.key) {
// Ref: https://www.w3.org/WAI/ARIA/apg/patterns/menubutton/#keyboard-interaction-13
@@ -480,15 +479,15 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
case Keys.ArrowDown:
event.preventDefault()
event.stopPropagation()
dispatch({ type: ActionTypes.OpenMenu })
d.nextFrame(() => dispatch({ type: ActionTypes.GoToItem, focus: Focus.First }))
flushSync(() => dispatch({ type: ActionTypes.OpenMenu }))
dispatch({ type: ActionTypes.GoToItem, focus: Focus.First })
break
case Keys.ArrowUp:
event.preventDefault()
event.stopPropagation()
dispatch({ type: ActionTypes.OpenMenu })
d.nextFrame(() => dispatch({ type: ActionTypes.GoToItem, focus: Focus.Last }))
flushSync(() => dispatch({ type: ActionTypes.OpenMenu }))
dispatch({ type: ActionTypes.GoToItem, focus: Focus.Last })
break
}
})
@@ -508,8 +507,8 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
if (isDisabledReactIssue7711(event.currentTarget)) return event.preventDefault()
if (disabled) return
if (state.menuState === MenuStates.Open) {
dispatch({ type: ActionTypes.CloseMenu })
d.nextFrame(() => state.buttonRef.current?.focus({ preventScroll: true }))
flushSync(() => dispatch({ type: ActionTypes.CloseMenu }))
state.buttonRef.current?.focus({ preventScroll: true })
} else {
event.preventDefault()
dispatch({ type: ActionTypes.OpenMenu })
@@ -722,20 +721,18 @@ function ItemsFn<TTag extends ElementType = typeof DEFAULT_ITEMS_TAG>(
case Keys.Escape:
event.preventDefault()
event.stopPropagation()
dispatch({ type: ActionTypes.CloseMenu })
disposables().nextFrame(() => state.buttonRef.current?.focus({ preventScroll: true }))
flushSync(() => dispatch({ type: ActionTypes.CloseMenu }))
state.buttonRef.current?.focus({ preventScroll: true })
break
case Keys.Tab:
event.preventDefault()
event.stopPropagation()
dispatch({ type: ActionTypes.CloseMenu })
disposables().microTask(() => {
focusFrom(
state.buttonRef.current!,
event.shiftKey ? FocusManagementFocus.Previous : FocusManagementFocus.Next
)
})
flushSync(() => dispatch({ type: ActionTypes.CloseMenu }))
focusFrom(
state.buttonRef.current!,
event.shiftKey ? FocusManagementFocus.Previous : FocusManagementFocus.Next
)
break
default:
@@ -837,11 +834,9 @@ function ItemFn<TTag extends ElementType = typeof DEFAULT_ITEM_TAG>(
if (state.menuState !== MenuStates.Open) return
if (!active) return
if (state.activationTrigger === ActivationTrigger.Pointer) return
let d = disposables()
d.requestAnimationFrame(() => {
return disposables().requestAnimationFrame(() => {
internalItemRef.current?.scrollIntoView?.({ block: 'nearest' })
})
return d.dispose
}, [
state.__demoMode,
internalItemRef,