From 71e7833dbd83a501df7efd24affaf37e0e70cf9d Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Thu, 2 Feb 2023 18:08:36 -0800 Subject: [PATCH 1/9] Fix React StrictMode NumberField, ColorField, ActionGroup --- .../actiongroup/src/useActionGroupItem.ts | 5 ++- .../spinbutton/src/useSpinButton.ts | 42 +++++++++---------- .../actiongroup/test/ActionGroup.test.js | 35 ++++++++++++++++ .../color/src/useColorFieldState.ts | 32 +++++++------- .../numberfield/src/useNumberFieldState.ts | 20 +++++---- 5 files changed, 89 insertions(+), 45 deletions(-) diff --git a/packages/@react-aria/actiongroup/src/useActionGroupItem.ts b/packages/@react-aria/actiongroup/src/useActionGroupItem.ts index 89b73c6259e..78efac68a24 100644 --- a/packages/@react-aria/actiongroup/src/useActionGroupItem.ts +++ b/packages/@react-aria/actiongroup/src/useActionGroupItem.ts @@ -44,10 +44,11 @@ export function useActionGroupItem(props: AriaActionGroupItemProps, state: Li let isFocused = props.key === state.selectionManager.focusedKey; let lastRender = useRef({isFocused, state}); - lastRender.current = {isFocused, state}; + useEffect(() => { + lastRender.current = {isFocused, state}; + }, [isFocused, state]); // If the focused item is removed from the DOM, reset the focused key to null. - // eslint-disable-next-line arrow-body-style useEffect(() => { return () => { if (lastRender.current.isFocused) { diff --git a/packages/@react-aria/spinbutton/src/useSpinButton.ts b/packages/@react-aria/spinbutton/src/useSpinButton.ts index 8239d8200e2..02e97e243fa 100644 --- a/packages/@react-aria/spinbutton/src/useSpinButton.ts +++ b/packages/@react-aria/spinbutton/src/useSpinButton.ts @@ -15,7 +15,7 @@ import {AriaButtonProps} from '@react-types/button'; import {DOMAttributes, InputBase, RangeInputBase, Validation, ValueBase} from '@react-types/shared'; // @ts-ignore import intlMessages from '../intl/*.json'; -import {useCallback, useEffect, useRef} from 'react'; +import {useCallback, useEffect, useMemo, useRef} from 'react'; import {useGlobalListeners} from '@react-aria/utils'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; @@ -56,17 +56,15 @@ export function useSpinButton( onIncrementToMax } = props; const stringFormatter = useLocalizedStringFormatter(intlMessages); - const propsRef = useRef(props); - propsRef.current = props; - const clearAsync = () => clearTimeout(_async.current); + const clearAsync = useCallback(() => clearTimeout(_async.current), [_async]); // eslint-disable-next-line arrow-body-style useEffect(() => { return () => clearAsync(); }, []); - let onKeyDown = (e) => { + let onKeyDown = useCallback((e) => { if (e.ctrlKey || e.metaKey || e.shiftKey || e.altKey || isReadOnly) { return; } @@ -113,22 +111,24 @@ export function useSpinButton( } break; } - }; + }, [isReadOnly, onIncrementPage, onIncrement, onDecrementPage, onDecrement, onDecrementToMin, onIncrementToMax]); let isFocused = useRef(false); - let onFocus = () => { + let onFocus = useCallback(() => { isFocused.current = true; - }; + }, [isFocused]); - let onBlur = () => { + let onBlur = useCallback(() => { isFocused.current = false; - }; + }, [isFocused]); // Replace Unicode hyphen-minus (U+002D) with minus sign (U+2212). // This ensures that macOS VoiceOver announces it as "minus" even with other characters between the minus sign // and the number (e.g. currency symbol). Otherwise it announces nothing because it assumes the character is a hyphen. // In addition, replace the empty string with the word "Empty" so that iOS VoiceOver does not read "50%" for an empty field. - textValue = textValue === '' ? stringFormatter.format('Empty') : (textValue || `${value}`).replace('-', '\u2212'); + textValue = useMemo( + () => textValue === '' ? stringFormatter.format('Empty') : (textValue || `${value}`).replace('-', '\u2212') + , [textValue, stringFormatter, value]); useEffect(() => { if (isFocused.current) { @@ -139,7 +139,7 @@ export function useSpinButton( const onIncrementPressStart = useCallback( (initialStepDelay: number) => { clearAsync(); - propsRef.current.onIncrement(); + onIncrement(); // Start spinning after initial delay _async.current = window.setTimeout( () => { @@ -157,7 +157,7 @@ export function useSpinButton( const onDecrementPressStart = useCallback( (initialStepDelay: number) => { clearAsync(); - propsRef.current.onDecrement(); + onDecrement(); // Start spinning after initial delay _async.current = window.setTimeout( () => { @@ -193,26 +193,26 @@ export function useSpinButton( onBlur }, incrementButtonProps: { - onPressStart: () => { + onPressStart: useCallback(() => { onIncrementPressStart(400); addGlobalListener(window, 'contextmenu', cancelContextMenu); - }, - onPressEnd: () => { + }, [onIncrementPressStart, addGlobalListener, cancelContextMenu]), + onPressEnd: useCallback(() => { clearAsync(); removeAllGlobalListeners(); - }, + }, [clearAsync, removeAllGlobalListeners]), onFocus, onBlur }, decrementButtonProps: { - onPressStart: () => { + onPressStart: useCallback(() => { onDecrementPressStart(400); addGlobalListener(window, 'contextmenu', cancelContextMenu); - }, - onPressEnd: () => { + }, [onIncrementPressStart, addGlobalListener, cancelContextMenu]), + onPressEnd: useCallback(() => { clearAsync(); removeAllGlobalListeners(); - }, + }, [clearAsync, removeAllGlobalListeners]), onFocus, onBlur } diff --git a/packages/@react-spectrum/actiongroup/test/ActionGroup.test.js b/packages/@react-spectrum/actiongroup/test/ActionGroup.test.js index 86e7714d8ab..ed23202766d 100644 --- a/packages/@react-spectrum/actiongroup/test/ActionGroup.test.js +++ b/packages/@react-spectrum/actiongroup/test/ActionGroup.test.js @@ -772,6 +772,41 @@ describe('ActionGroup', function () { expect(buttons[1]).toHaveAttribute('tabIndex', '0'); }); + it('moves focus if the focused button was removed', function () { + let onAction = jest.fn(); + let tree = render( + + + One + Two + Three + Four + + + ); + + let actiongroup = tree.getByRole('toolbar'); + let buttons = within(actiongroup).getAllByRole('button'); + expect(buttons[0]).toHaveAttribute('tabIndex', '0'); + expect(buttons[1]).toHaveAttribute('tabIndex', '0'); + + act(() => buttons[2].focus()); + tree.rerender( + + + One + Two + Four + + + ); + actiongroup = tree.getByRole('toolbar'); + buttons = within(actiongroup).getAllByRole('button'); + expect(buttons[0]).toHaveAttribute('tabIndex', '0'); + expect(buttons[1]).toHaveAttribute('tabIndex', '0'); + expect(buttons[2]).toHaveAttribute('tabIndex', '0'); + }); + it('passes aria labeling props through to menu button if it is the only child', function () { jest.spyOn(HTMLElement.prototype, 'offsetWidth', 'get').mockImplementation(function () { if (this instanceof HTMLButtonElement) { diff --git a/packages/@react-stately/color/src/useColorFieldState.ts b/packages/@react-stately/color/src/useColorFieldState.ts index e02ce7f1ec7..a68c0e26dc2 100644 --- a/packages/@react-stately/color/src/useColorFieldState.ts +++ b/packages/@react-stately/color/src/useColorFieldState.ts @@ -12,9 +12,9 @@ import {Color, ColorFieldProps} from '@react-types/color'; import {parseColor} from './Color'; +import {useCallback, useEffect, useMemo, useRef, useState} from 'react'; import {useColor} from './useColor'; import {useControlledState} from '@react-stately/utils'; -import {useMemo, useRef, useState} from 'react'; export interface ColorFieldState { /** @@ -86,10 +86,12 @@ export function useColorFieldState( }; let prevValue = useRef(colorValue); - if (prevValue.current !== colorValue) { - setInputValue(colorValue ? colorValue.toString('hex') : ''); - prevValue.current = colorValue; - } + useEffect(() => { + if (prevValue.current !== colorValue) { + setInputValue(colorValue ? colorValue.toString('hex') : ''); + prevValue.current = colorValue; + } + }); let parsedValue = useMemo(() => { @@ -102,9 +104,11 @@ export function useColorFieldState( return color; }, [inputValue]); let parsed = useRef(null); - parsed.current = parsedValue; + useEffect(() => { + parsed.current = parsedValue; + }); - let commit = () => { + let commit = useCallback(() => { // Set to empty state if input value is empty if (!inputValue.length) { safelySetColorValue(null); @@ -125,9 +129,9 @@ export function useColorFieldState( newColorValue = colorValue.toString('hex'); } setInputValue(newColorValue); - }; + }, [inputValue, safelySetColorValue, setInputValue, colorValue, parsed]); - let increment = () => { + let increment = useCallback(() => { let newValue = addColorValue(parsed.current, step); // if we've arrived at the same value that was previously in the state, the // input value should be updated to match @@ -137,8 +141,8 @@ export function useColorFieldState( setInputValue(newValue.toString('hex')); } safelySetColorValue(newValue); - }; - let decrement = () => { + }, [safelySetColorValue, parsed, colorValue, setInputValue, step]); + let decrement = useCallback(() => { let newValue = addColorValue(parsed.current, -step); // if we've arrived at the same value that was previously in the state, the // input value should be updated to match @@ -148,9 +152,9 @@ export function useColorFieldState( setInputValue(newValue.toString('hex')); } safelySetColorValue(newValue); - }; - let incrementToMax = () => safelySetColorValue(MAX_COLOR); - let decrementToMin = () => safelySetColorValue(MIN_COLOR); + }, [safelySetColorValue, parsed, colorValue, setInputValue, step]); + let incrementToMax = useCallback(() => safelySetColorValue(MAX_COLOR), [safelySetColorValue]); + let decrementToMin = useCallback(() => safelySetColorValue(MIN_COLOR), [safelySetColorValue]); let validate = (value: string) => value === '' || !!value.match(/^#?[0-9a-f]{0,6}$/i)?.[0]; diff --git a/packages/@react-stately/numberfield/src/useNumberFieldState.ts b/packages/@react-stately/numberfield/src/useNumberFieldState.ts index c75ba28610d..e9ca6450f14 100644 --- a/packages/@react-stately/numberfield/src/useNumberFieldState.ts +++ b/packages/@react-stately/numberfield/src/useNumberFieldState.ts @@ -13,7 +13,7 @@ import {clamp, snapValueToStep, useControlledState} from '@react-stately/utils'; import {NumberFieldProps} from '@react-types/numberfield'; import {NumberFormatter, NumberParser} from '@internationalized/number'; -import {useCallback, useMemo, useRef, useState} from 'react'; +import {useCallback, useEffect, useMemo, useRef, useState} from 'react'; export interface NumberFieldState { /** @@ -107,17 +107,21 @@ export function useNumberFieldState( let prevValue = useRef(numberValue); let prevLocale = useRef(locale); let prevFormatOptions = useRef(formatOptions); - if (!Object.is(numberValue, prevValue.current) || locale !== prevLocale.current || formatOptions !== prevFormatOptions.current) { - setInputValue(format(numberValue)); - prevValue.current = numberValue; - prevLocale.current = locale; - prevFormatOptions.current = formatOptions; - } + useEffect(() => { + if (!Object.is(numberValue, prevValue.current) || locale !== prevLocale.current || formatOptions !== prevFormatOptions.current) { + setInputValue(format(numberValue)); + prevValue.current = numberValue; + prevLocale.current = locale; + prevFormatOptions.current = formatOptions; + } + }); // Store last parsed value in a ref so it can be used by increment/decrement below let parsedValue = useMemo(() => numberParser.parse(inputValue), [numberParser, inputValue]); let parsed = useRef(0); - parsed.current = parsedValue; + useEffect(() => { + parsed.current = parsedValue; + }); let commit = () => { // Set to empty state if input value is empty From 6c43ef84989cc1bf2de944c91afe6cacc7ca5202 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Mon, 6 Feb 2023 16:21:12 -0800 Subject: [PATCH 2/9] Simplify colorfield logic --- .../color/src/useColorFieldState.ts | 59 +++++++++---------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/packages/@react-stately/color/src/useColorFieldState.ts b/packages/@react-stately/color/src/useColorFieldState.ts index a68c0e26dc2..cd748ffd765 100644 --- a/packages/@react-stately/color/src/useColorFieldState.ts +++ b/packages/@react-stately/color/src/useColorFieldState.ts @@ -71,19 +71,9 @@ export function useColorFieldState( let initialValue = useColor(value); let initialDefaultValue = useColor(defaultValue); - let [colorValue, setColorValue] = useControlledState(initialValue, initialDefaultValue, onChange); - let [inputValue, setInputValue] = useState(() => (value || defaultValue) && colorValue ? colorValue.toString('hex') : ''); - - let safelySetColorValue = (newColor: Color) => { - if (!colorValue || !newColor) { - setColorValue(newColor); - return; - } - if (newColor.toHexInt() !== colorValue.toHexInt()) { - setColorValue(newColor); - return; - } - }; + let [colorValue, _setColorValue] = useControlledState(initialValue, initialDefaultValue, onChange); + let [inputValue, _setInputValue] = useState(() => (value || defaultValue) && colorValue ? colorValue.toString('hex') : ''); + let [parsedColor, setParsedColor] = useState(colorValue); let prevValue = useRef(colorValue); useEffect(() => { @@ -92,21 +82,30 @@ export function useColorFieldState( prevValue.current = colorValue; } }); - - - let parsedValue = useMemo(() => { + let setColorValue = (val) => { + _setColorValue(val); + }; + let setInputValue = (val) => { + _setInputValue(val); let color; try { - color = parseColor(inputValue.startsWith('#') ? inputValue : `#${inputValue}`); + color = parseColor(val.startsWith('#') ? val : `#${val}`); } catch (err) { color = null; } - return color; - }, [inputValue]); - let parsed = useRef(null); - useEffect(() => { - parsed.current = parsedValue; - }); + setParsedColor(color); + }; + + let safelySetColorValue = (newColor: Color) => { + if (!colorValue || !newColor) { + setColorValue(newColor); + return; + } + if (newColor.toHexInt() !== colorValue.toHexInt()) { + setColorValue(newColor); + return; + } + }; let commit = useCallback(() => { // Set to empty state if input value is empty @@ -117,22 +116,22 @@ export function useColorFieldState( } // if it failed to parse, then reset input to formatted version of current number - if (parsed.current == null) { + if (parsedColor == null) { setInputValue(colorValue ? colorValue.toString('hex') : ''); return; } - safelySetColorValue(parsed.current); + safelySetColorValue(parsedColor); // in a controlled state, the numberValue won't change, so we won't go back to our old input without help let newColorValue = ''; if (colorValue) { newColorValue = colorValue.toString('hex'); } setInputValue(newColorValue); - }, [inputValue, safelySetColorValue, setInputValue, colorValue, parsed]); + }, [inputValue, safelySetColorValue, setInputValue, colorValue, parsedColor]); let increment = useCallback(() => { - let newValue = addColorValue(parsed.current, step); + let newValue = addColorValue(parsedColor, step); // if we've arrived at the same value that was previously in the state, the // input value should be updated to match // ex type 4, press increment, highlight the number in the input, type 4 again, press increment @@ -141,9 +140,9 @@ export function useColorFieldState( setInputValue(newValue.toString('hex')); } safelySetColorValue(newValue); - }, [safelySetColorValue, parsed, colorValue, setInputValue, step]); + }, [safelySetColorValue, parsedColor, colorValue, setInputValue, step]); let decrement = useCallback(() => { - let newValue = addColorValue(parsed.current, -step); + let newValue = addColorValue(parsedColor, -step); // if we've arrived at the same value that was previously in the state, the // input value should be updated to match // ex type 4, press increment, highlight the number in the input, type 4 again, press increment @@ -152,7 +151,7 @@ export function useColorFieldState( setInputValue(newValue.toString('hex')); } safelySetColorValue(newValue); - }, [safelySetColorValue, parsed, colorValue, setInputValue, step]); + }, [safelySetColorValue, parsedColor, colorValue, setInputValue, step]); let incrementToMax = useCallback(() => safelySetColorValue(MAX_COLOR), [safelySetColorValue]); let decrementToMin = useCallback(() => safelySetColorValue(MIN_COLOR), [safelySetColorValue]); From afbe8ff18ac6103173ee17cc1306e7b1357a2387 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Mon, 6 Feb 2023 19:23:36 -0800 Subject: [PATCH 3/9] Fix controlled fields flicker regression --- .../spinbutton/src/useSpinButton.ts | 15 ++-- .../numberfield/test/NumberField.test.js | 1 + .../color/src/useColorFieldState.ts | 69 +++++++++++-------- .../numberfield/src/useNumberFieldState.ts | 65 +++++++++-------- 4 files changed, 84 insertions(+), 66 deletions(-) diff --git a/packages/@react-aria/spinbutton/src/useSpinButton.ts b/packages/@react-aria/spinbutton/src/useSpinButton.ts index 02e97e243fa..0dff6cb1944 100644 --- a/packages/@react-aria/spinbutton/src/useSpinButton.ts +++ b/packages/@react-aria/spinbutton/src/useSpinButton.ts @@ -59,9 +59,10 @@ export function useSpinButton( const clearAsync = useCallback(() => clearTimeout(_async.current), [_async]); - // eslint-disable-next-line arrow-body-style + // only run on unmount useEffect(() => { return () => clearAsync(); + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); let onKeyDown = useCallback((e) => { @@ -150,8 +151,7 @@ export function useSpinButton( initialStepDelay ); }, - // eslint-disable-next-line react-hooks/exhaustive-deps - [onIncrement, maxValue, value] + [onIncrement, maxValue, value, clearAsync, _async] ); const onDecrementPressStart = useCallback( @@ -168,13 +168,12 @@ export function useSpinButton( initialStepDelay ); }, - // eslint-disable-next-line react-hooks/exhaustive-deps - [onDecrement, minValue, value] + [onDecrement, minValue, value, clearAsync, _async] ); - let cancelContextMenu = (e) => { + let cancelContextMenu = useCallback((e) => { e.preventDefault(); - }; + }, []); let {addGlobalListener, removeAllGlobalListeners} = useGlobalListeners(); @@ -208,7 +207,7 @@ export function useSpinButton( onPressStart: useCallback(() => { onDecrementPressStart(400); addGlobalListener(window, 'contextmenu', cancelContextMenu); - }, [onIncrementPressStart, addGlobalListener, cancelContextMenu]), + }, [onDecrementPressStart, addGlobalListener, cancelContextMenu]), onPressEnd: useCallback(() => { clearAsync(); removeAllGlobalListeners(); diff --git a/packages/@react-spectrum/numberfield/test/NumberField.test.js b/packages/@react-spectrum/numberfield/test/NumberField.test.js index d6f7d58c4be..915c3b89dd3 100644 --- a/packages/@react-spectrum/numberfield/test/NumberField.test.js +++ b/packages/@react-spectrum/numberfield/test/NumberField.test.js @@ -278,6 +278,7 @@ describe('NumberField', function () { act(() => {textField.blur();}); expect(onChangeSpy).toHaveBeenLastCalledWith(5); expect(onChangeSpy).toHaveBeenCalledTimes(2); + expect(textField).toHaveAttribute('value', '5'); triggerPress(incrementButton); expect(onChangeSpy).toHaveBeenLastCalledWith(10); expect(onChangeSpy).toHaveBeenCalledTimes(3); diff --git a/packages/@react-stately/color/src/useColorFieldState.ts b/packages/@react-stately/color/src/useColorFieldState.ts index cd748ffd765..9aff86e23bc 100644 --- a/packages/@react-stately/color/src/useColorFieldState.ts +++ b/packages/@react-stately/color/src/useColorFieldState.ts @@ -12,7 +12,7 @@ import {Color, ColorFieldProps} from '@react-types/color'; import {parseColor} from './Color'; -import {useCallback, useEffect, useMemo, useRef, useState} from 'react'; +import {useCallback, useEffect, useRef, useState} from 'react'; import {useColor} from './useColor'; import {useControlledState} from '@react-stately/utils'; @@ -71,21 +71,12 @@ export function useColorFieldState( let initialValue = useColor(value); let initialDefaultValue = useColor(defaultValue); - let [colorValue, _setColorValue] = useControlledState(initialValue, initialDefaultValue, onChange); + let [colorValue, setColorValue] = useControlledState(initialValue, initialDefaultValue, onChange); let [inputValue, _setInputValue] = useState(() => (value || defaultValue) && colorValue ? colorValue.toString('hex') : ''); let [parsedColor, setParsedColor] = useState(colorValue); + let [forceSync, setForceSync] = useState(false); - let prevValue = useRef(colorValue); - useEffect(() => { - if (prevValue.current !== colorValue) { - setInputValue(colorValue ? colorValue.toString('hex') : ''); - prevValue.current = colorValue; - } - }); - let setColorValue = (val) => { - _setColorValue(val); - }; - let setInputValue = (val) => { + let setInputValue = useCallback((val) => { _setInputValue(val); let color; try { @@ -94,18 +85,25 @@ export function useColorFieldState( color = null; } setParsedColor(color); - }; + }, [_setInputValue, setParsedColor]); - let safelySetColorValue = (newColor: Color) => { + let prevValue = useRef(colorValue); + useEffect(() => { + if (forceSync || prevValue.current !== colorValue) { + setInputValue(colorValue ? colorValue.toString('hex') : ''); + setForceSync(false); + prevValue.current = colorValue; + } + }, [forceSync, colorValue, setInputValue]); + + let safelySetColorValue = useCallback((newColor: Color) => { if (!colorValue || !newColor) { setColorValue(newColor); - return; - } - if (newColor.toHexInt() !== colorValue.toHexInt()) { + } else if (newColor.toHexInt() !== colorValue.toHexInt()) { setColorValue(newColor); - return; } - }; + return newColor; + }, [colorValue, setColorValue]); let commit = useCallback(() => { // Set to empty state if input value is empty @@ -121,17 +119,19 @@ export function useColorFieldState( return; } - safelySetColorValue(parsedColor); + let newColor = safelySetColorValue(parsedColor); // in a controlled state, the numberValue won't change, so we won't go back to our old input without help - let newColorValue = ''; - if (colorValue) { - newColorValue = colorValue.toString('hex'); + if (value !== undefined || colorsAreEqual(newColor, colorValue)) { + setForceSync(true); } - setInputValue(newColorValue); - }, [inputValue, safelySetColorValue, setInputValue, colorValue, parsedColor]); + }, [inputValue, safelySetColorValue, setInputValue, value, colorValue, parsedColor]); + let spinRef = useRef(parsedColor); + useEffect(() => { + spinRef.current = parsedColor; + }); let increment = useCallback(() => { - let newValue = addColorValue(parsedColor, step); + let newValue = addColorValue(spinRef.current, step); // if we've arrived at the same value that was previously in the state, the // input value should be updated to match // ex type 4, press increment, highlight the number in the input, type 4 again, press increment @@ -140,9 +140,9 @@ export function useColorFieldState( setInputValue(newValue.toString('hex')); } safelySetColorValue(newValue); - }, [safelySetColorValue, parsedColor, colorValue, setInputValue, step]); + }, [safelySetColorValue, colorValue, setInputValue, step]); let decrement = useCallback(() => { - let newValue = addColorValue(parsedColor, -step); + let newValue = addColorValue(spinRef.current, -step); // if we've arrived at the same value that was previously in the state, the // input value should be updated to match // ex type 4, press increment, highlight the number in the input, type 4 again, press increment @@ -151,7 +151,7 @@ export function useColorFieldState( setInputValue(newValue.toString('hex')); } safelySetColorValue(newValue); - }, [safelySetColorValue, parsedColor, colorValue, setInputValue, step]); + }, [safelySetColorValue, colorValue, setInputValue, step]); let incrementToMax = useCallback(() => safelySetColorValue(MAX_COLOR), [safelySetColorValue]); let decrementToMin = useCallback(() => safelySetColorValue(MIN_COLOR), [safelySetColorValue]); @@ -181,3 +181,12 @@ function addColorValue(color: Color, step: number) { } return newColor; } + +function colorsAreEqual(color1: Color, color2: Color) { + if (!color1 && !color2) { + return true; + } else if (!color1 || !color2) { + return false; + } + return color1.toHexInt() === color2.toHexInt(); +} diff --git a/packages/@react-stately/numberfield/src/useNumberFieldState.ts b/packages/@react-stately/numberfield/src/useNumberFieldState.ts index e9ca6450f14..d28837e4a26 100644 --- a/packages/@react-stately/numberfield/src/useNumberFieldState.ts +++ b/packages/@react-stately/numberfield/src/useNumberFieldState.ts @@ -88,19 +88,27 @@ export function useNumberFieldState( } = props; let [numberValue, setNumberValue] = useControlledState(value, isNaN(defaultValue) ? NaN : defaultValue, onChange); - let [inputValue, setInputValue] = useState(() => isNaN(numberValue) ? '' : new NumberFormatter(locale, formatOptions).format(numberValue)); + let [inputValue, _setInputValue] = useState(() => isNaN(numberValue) ? '' : new NumberFormatter(locale, formatOptions).format(numberValue)); let numberParser = useMemo(() => new NumberParser(locale, formatOptions), [locale, formatOptions]); let numberingSystem = useMemo(() => numberParser.getNumberingSystem(inputValue), [numberParser, inputValue]); let formatter = useMemo(() => new NumberFormatter(locale, {...formatOptions, numberingSystem}), [locale, formatOptions, numberingSystem]); let intlOptions = useMemo(() => formatter.resolvedOptions(), [formatter]); let format = useCallback((value: number) => (isNaN(value) || value === null) ? '' : formatter.format(value), [formatter]); + let [parsedValue, setParsedValue] = useState(numberValue); + let [forceSync, setForceSync] = useState(false); let clampStep = !isNaN(step) ? step : 1; if (intlOptions.style === 'percent' && isNaN(step)) { clampStep = 0.01; } + let setInputValue = useCallback((val: string) => { + _setInputValue(val); + let num = numberParser.parse(val); + setParsedValue(num); + }, [_setInputValue, numberParser, setParsedValue]); + // Update the input value when the number value or format options change. This is done // in a useEffect so that the controlled behavior is correct and we only update the // textfield after prop changes. @@ -108,22 +116,16 @@ export function useNumberFieldState( let prevLocale = useRef(locale); let prevFormatOptions = useRef(formatOptions); useEffect(() => { - if (!Object.is(numberValue, prevValue.current) || locale !== prevLocale.current || formatOptions !== prevFormatOptions.current) { + if (forceSync || !Object.is(numberValue, prevValue.current) || locale !== prevLocale.current || formatOptions !== prevFormatOptions.current) { setInputValue(format(numberValue)); + setForceSync(false); prevValue.current = numberValue; prevLocale.current = locale; prevFormatOptions.current = formatOptions; } - }); - - // Store last parsed value in a ref so it can be used by increment/decrement below - let parsedValue = useMemo(() => numberParser.parse(inputValue), [numberParser, inputValue]); - let parsed = useRef(0); - useEffect(() => { - parsed.current = parsedValue; - }); + }, [forceSync, numberValue, locale, formatOptions, setInputValue, format]); - let commit = () => { + let commit = useCallback(() => { // Set to empty state if input value is empty if (!inputValue.length) { setNumberValue(NaN); @@ -132,7 +134,7 @@ export function useNumberFieldState( } // if it failed to parse, then reset input to formatted version of current number - if (isNaN(parsed.current)) { + if (isNaN(parsedValue)) { setInputValue(format(numberValue)); return; } @@ -140,20 +142,27 @@ export function useNumberFieldState( // Clamp to min and max, round to the nearest step, and round to specified number of digits let clampedValue: number; if (isNaN(step)) { - clampedValue = clamp(parsed.current, minValue, maxValue); + clampedValue = clamp(parsedValue, minValue, maxValue); } else { - clampedValue = snapValueToStep(parsed.current, minValue, maxValue, step); + clampedValue = snapValueToStep(parsedValue, minValue, maxValue, step); } clampedValue = numberParser.parse(format(clampedValue)); + setNumberValue(clampedValue); // in a controlled state, the numberValue won't change, so we won't go back to our old input without help - setInputValue(format(value === undefined ? clampedValue : numberValue)); - }; + if (value !== undefined || clampedValue === numberValue) { + setForceSync(true); + } + }, [inputValue, setNumberValue, setInputValue, value, format, numberValue, parsedValue, step, minValue, maxValue, numberParser]); - let safeNextStep = (operation: '+' | '-', minMax: number) => { - let prev = parsed.current; + let spinRef = useRef(parsedValue); + useEffect(() => { + spinRef.current = parsedValue; + }); + let safeNextStep = useCallback((operation: '+' | '-', minMax: number) => { + let prev = spinRef.current; if (isNaN(prev)) { // if the input is empty, start from the min/max value when incrementing/decrementing, @@ -175,9 +184,9 @@ export function useNumberFieldState( clampStep ); } - }; + }, [minValue, maxValue, clampStep]); - let increment = () => { + let increment = useCallback(() => { let newValue = safeNextStep('+', minValue); // if we've arrived at the same value that was previously in the state, the @@ -189,9 +198,9 @@ export function useNumberFieldState( } setNumberValue(newValue); - }; + }, [safeNextStep, setInputValue, minValue, format, numberValue, setNumberValue]); - let decrement = () => { + let decrement = useCallback(() => { let newValue = safeNextStep('-', maxValue); if (newValue === numberValue) { @@ -199,19 +208,19 @@ export function useNumberFieldState( } setNumberValue(newValue); - }; + }, [safeNextStep, setInputValue, maxValue, format, numberValue, setNumberValue]); - let incrementToMax = () => { + let incrementToMax = useCallback(() => { if (maxValue != null) { setNumberValue(snapValueToStep(maxValue, minValue, maxValue, clampStep)); } - }; + }, [maxValue, setNumberValue, minValue, clampStep]); - let decrementToMin = () => { + let decrementToMin = useCallback(() => { if (minValue != null) { setNumberValue(minValue); } - }; + }, [minValue, setNumberValue]); let canIncrement = useMemo(() => ( !isDisabled && @@ -247,7 +256,7 @@ export function useNumberFieldState( canDecrement, minValue, maxValue, - numberValue: parsedValue, + numberValue, setInputValue, inputValue, commit From 2856060db2e5ec402f7bce57da5cddd8304f78a8 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Tue, 7 Feb 2023 09:50:54 -0800 Subject: [PATCH 4/9] Don't implicitly type coerce --- packages/@react-stately/color/src/useColorFieldState.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/@react-stately/color/src/useColorFieldState.ts b/packages/@react-stately/color/src/useColorFieldState.ts index 9aff86e23bc..be2c0af63df 100644 --- a/packages/@react-stately/color/src/useColorFieldState.ts +++ b/packages/@react-stately/color/src/useColorFieldState.ts @@ -152,8 +152,12 @@ export function useColorFieldState( } safelySetColorValue(newValue); }, [safelySetColorValue, colorValue, setInputValue, step]); - let incrementToMax = useCallback(() => safelySetColorValue(MAX_COLOR), [safelySetColorValue]); - let decrementToMin = useCallback(() => safelySetColorValue(MIN_COLOR), [safelySetColorValue]); + let incrementToMax = useCallback(() => { + safelySetColorValue(MAX_COLOR); + }, [safelySetColorValue]); + let decrementToMin = useCallback(() => { + safelySetColorValue(MIN_COLOR); + }, [safelySetColorValue]); let validate = (value: string) => value === '' || !!value.match(/^#?[0-9a-f]{0,6}$/i)?.[0]; From 83d52da3f49b6483a9f9323df2b538e833484001 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Tue, 7 Mar 2023 22:53:35 -0800 Subject: [PATCH 5/9] make spin button easier to read --- packages/@react-aria/spinbutton/src/useSpinButton.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/@react-aria/spinbutton/src/useSpinButton.ts b/packages/@react-aria/spinbutton/src/useSpinButton.ts index 0dff6cb1944..e601bdd4089 100644 --- a/packages/@react-aria/spinbutton/src/useSpinButton.ts +++ b/packages/@react-aria/spinbutton/src/useSpinButton.ts @@ -39,7 +39,6 @@ export interface SpinbuttonAria { export function useSpinButton( props: SpinButtonProps ): SpinbuttonAria { - const _async = useRef(); let { value, textValue, @@ -57,13 +56,12 @@ export function useSpinButton( } = props; const stringFormatter = useLocalizedStringFormatter(intlMessages); + const _async = useRef(); const clearAsync = useCallback(() => clearTimeout(_async.current), [_async]); - // only run on unmount useEffect(() => { return () => clearAsync(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [clearAsync]); let onKeyDown = useCallback((e) => { if (e.ctrlKey || e.metaKey || e.shiftKey || e.altKey || isReadOnly) { From 87270e2405fe939f4307cd4199c941896b27bd38 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Wed, 29 Mar 2023 17:26:53 -0700 Subject: [PATCH 6/9] correct our tests --- .../numberfield/test/NumberField.test.js | 23 ++----------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/packages/@react-spectrum/numberfield/test/NumberField.test.js b/packages/@react-spectrum/numberfield/test/NumberField.test.js index eac2fb7d702..42aa4d5dcf8 100644 --- a/packages/@react-spectrum/numberfield/test/NumberField.test.js +++ b/packages/@react-spectrum/numberfield/test/NumberField.test.js @@ -882,12 +882,8 @@ describe('NumberField', function () { act(() => {textField.focus();}); textField.setSelectionRange(2, 3); userEvent.type(textField, '{backspace}'); - expect(announce).toHaveBeenCalledTimes(2); - expect(announce).toHaveBeenLastCalledWith('−$0.00', 'assertive'); textField.setSelectionRange(2, 2); typeText(textField, '1'); - expect(announce).toHaveBeenCalledTimes(3); - expect(announce).toHaveBeenLastCalledWith('−$1.00', 'assertive'); textField.setSelectionRange(3, 3); typeText(textField, '8'); expect(textField).toHaveAttribute('value', '($18.00)'); @@ -895,15 +891,11 @@ describe('NumberField', function () { expect(textField).toHaveAttribute('value', '($18.00)'); expect(onChangeSpy).toHaveBeenCalledTimes(2); expect(onChangeSpy).toHaveBeenLastCalledWith(-18); - expect(announce).toHaveBeenCalledTimes(4); - expect(announce).toHaveBeenLastCalledWith('−$18.00', 'assertive'); act(() => {textField.focus();}); textField.setSelectionRange(7, 8); userEvent.type(textField, '{backspace}'); expect(textField).toHaveAttribute('value', '($18.00'); - expect(announce).toHaveBeenCalledTimes(5); - expect(announce).toHaveBeenLastCalledWith('$18.00', 'assertive'); act(() => {textField.blur();}); expect(textField).toHaveAttribute('value', '$18.00'); expect(onChangeSpy).toHaveBeenCalledTimes(3); @@ -911,12 +903,8 @@ describe('NumberField', function () { act(() => {textField.focus();}); userEvent.clear(textField); - expect(announce).toHaveBeenCalledTimes(6); - expect(announce).toHaveBeenLastCalledWith('Empty', 'assertive'); typeText(textField, '($32)'); expect(textField).toHaveAttribute('value', '($32)'); - expect(announce).toHaveBeenCalledTimes(9); - expect(announce).toHaveBeenLastCalledWith('−$32.00', 'assertive'); act(() => {textField.blur();}); expect(textField).toHaveAttribute('value', '($32.00)'); expect(onChangeSpy).toHaveBeenCalledTimes(4); @@ -930,10 +918,9 @@ describe('NumberField', function () { let {textField} = renderNumberField({onChange: onChangeSpy, formatOptions: {style: 'currency', currency: 'USD', currencySign: 'accounting'}}, {locale: 'ar-AE'}); act(() => {textField.focus();}); - userEvent.type(textField, '(10)'); + typeText(textField, '(10)'); expect(textField).toHaveAttribute('value', '(10)'); - expect(announce).toHaveBeenCalledTimes(3); - expect(announce).toHaveBeenLastCalledWith('؜−١٠٫٠٠ US$', 'assertive'); + expect(announce).toHaveBeenCalledTimes(0); act(() => {textField.blur();}); expect(textField).toHaveAttribute('value', '(US$10.00)'); expect(onChangeSpy).toHaveBeenCalledTimes(1); @@ -963,17 +950,11 @@ describe('NumberField', function () { }); textField.setSelectionRange(1, 2); userEvent.type(textField, '{backspace}'); - expect(announce).toHaveBeenCalledTimes(2); - expect(announce).toHaveBeenLastCalledWith('−0,00 $', 'assertive'); textField.setSelectionRange(1, 1); typeText(textField, '1'); - expect(announce).toHaveBeenCalledTimes(3); - expect(announce).toHaveBeenLastCalledWith('−1,00 $', 'assertive'); textField.setSelectionRange(2, 2); typeText(textField, '8'); expect(textField).toHaveAttribute('value', '-18,00 $'); - expect(announce).toHaveBeenCalledTimes(4); - expect(announce).toHaveBeenLastCalledWith('−18,00 $', 'assertive'); act(() => { textField.blur(); }); From 087f8c1bcd94b27a38d1b0add86dae274e70e4b2 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Thu, 6 Apr 2023 11:12:24 -0700 Subject: [PATCH 7/9] partial check in --- .../@react-aria/spinbutton/src/useSpinButton.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/@react-aria/spinbutton/src/useSpinButton.ts b/packages/@react-aria/spinbutton/src/useSpinButton.ts index 3f6527f6489..a8a19374539 100644 --- a/packages/@react-aria/spinbutton/src/useSpinButton.ts +++ b/packages/@react-aria/spinbutton/src/useSpinButton.ts @@ -54,6 +54,10 @@ export function useSpinButton( onDecrementToMin, onIncrementToMax } = props; + let propsRef = useRef(props); + useEffect(() => { + propsRef.current = props; + }); const stringFormatter = useLocalizedStringFormatter(intlMessages); const _async = useRef(); @@ -139,35 +143,35 @@ export function useSpinButton( const onIncrementPressStart = useCallback( (initialStepDelay: number) => { clearAsync(); - onIncrement(); + propsRef.current.onIncrement(); // Start spinning after initial delay _async.current = window.setTimeout( () => { - if (isNaN(maxValue) || isNaN(value) || value < maxValue) { + if (isNaN(propsRef.current.maxValue) || isNaN(propsRef.current.value) || propsRef.current.value < propsRef.current.maxValue) { onIncrementPressStart(60); } }, initialStepDelay ); }, - [onIncrement, maxValue, value, clearAsync, _async] + [clearAsync, _async] ); const onDecrementPressStart = useCallback( (initialStepDelay: number) => { clearAsync(); - onDecrement(); + propsRef.current.onDecrement(); // Start spinning after initial delay _async.current = window.setTimeout( () => { - if (isNaN(minValue) || isNaN(value) || value > minValue) { + if (isNaN(propsRef.current.minValue) || isNaN(propsRef.current.value) || propsRef.current.value > propsRef.current.minValue) { onDecrementPressStart(60); } }, initialStepDelay ); }, - [onDecrement, minValue, value, clearAsync, _async] + [clearAsync, _async] ); let cancelContextMenu = useCallback((e) => { From ab3b7370785d50b93569401bcbee60bb1981b4c9 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Thu, 13 Apr 2023 15:06:21 -0700 Subject: [PATCH 8/9] Translate to useEffectEvent --- .../actiongroup/src/useActionGroupItem.ts | 15 ++--- .../spinbutton/src/useSpinButton.ts | 67 ++++++++----------- .../@react-aria/utils/src/useEffectEvent.ts | 6 +- .../color/src/useColorFieldState.ts | 12 ++-- .../numberfield/src/useNumberFieldState.ts | 8 +-- 5 files changed, 45 insertions(+), 63 deletions(-) diff --git a/packages/@react-aria/actiongroup/src/useActionGroupItem.ts b/packages/@react-aria/actiongroup/src/useActionGroupItem.ts index 78efac68a24..85ea9dd5040 100644 --- a/packages/@react-aria/actiongroup/src/useActionGroupItem.ts +++ b/packages/@react-aria/actiongroup/src/useActionGroupItem.ts @@ -13,7 +13,7 @@ import {DOMAttributes, FocusableElement} from '@react-types/shared'; import {Key, RefObject, useEffect, useRef} from 'react'; import {ListState} from '@react-stately/list'; -import {mergeProps} from '@react-aria/utils'; +import {mergeProps, useEffectEvent} from '@react-aria/utils'; import {PressProps} from '@react-aria/interactions'; export interface AriaActionGroupItemProps { @@ -43,17 +43,16 @@ export function useActionGroupItem(props: AriaActionGroupItemProps, state: Li } let isFocused = props.key === state.selectionManager.focusedKey; - let lastRender = useRef({isFocused, state}); - useEffect(() => { - lastRender.current = {isFocused, state}; - }, [isFocused, state]); + let onRemovedWithFocus = useEffectEvent(() => { + if (isFocused) { + state.selectionManager.setFocusedKey(null); + } + }); // If the focused item is removed from the DOM, reset the focused key to null. useEffect(() => { return () => { - if (lastRender.current.isFocused) { - lastRender.current.state.selectionManager.setFocusedKey(null); - } + onRemovedWithFocus(); }; }, []); diff --git a/packages/@react-aria/spinbutton/src/useSpinButton.ts b/packages/@react-aria/spinbutton/src/useSpinButton.ts index a8a19374539..590d669ee38 100644 --- a/packages/@react-aria/spinbutton/src/useSpinButton.ts +++ b/packages/@react-aria/spinbutton/src/useSpinButton.ts @@ -16,7 +16,7 @@ import {DOMAttributes, InputBase, RangeInputBase, Validation, ValueBase} from '@ // @ts-ignore import intlMessages from '../intl/*.json'; import {useCallback, useEffect, useMemo, useRef} from 'react'; -import {useGlobalListeners} from '@react-aria/utils'; +import {useEffectEvent, useGlobalListeners} from '@react-aria/utils'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; @@ -54,10 +54,7 @@ export function useSpinButton( onDecrementToMin, onIncrementToMax } = props; - let propsRef = useRef(props); - useEffect(() => { - propsRef.current = props; - }); + const stringFormatter = useLocalizedStringFormatter(intlMessages); const _async = useRef(); @@ -140,39 +137,33 @@ export function useSpinButton( } }, [textValue]); - const onIncrementPressStart = useCallback( - (initialStepDelay: number) => { - clearAsync(); - propsRef.current.onIncrement(); - // Start spinning after initial delay - _async.current = window.setTimeout( - () => { - if (isNaN(propsRef.current.maxValue) || isNaN(propsRef.current.value) || propsRef.current.value < propsRef.current.maxValue) { - onIncrementPressStart(60); - } - }, - initialStepDelay - ); - }, - [clearAsync, _async] - ); - - const onDecrementPressStart = useCallback( - (initialStepDelay: number) => { - clearAsync(); - propsRef.current.onDecrement(); - // Start spinning after initial delay - _async.current = window.setTimeout( - () => { - if (isNaN(propsRef.current.minValue) || isNaN(propsRef.current.value) || propsRef.current.value > propsRef.current.minValue) { - onDecrementPressStart(60); - } - }, - initialStepDelay - ); - }, - [clearAsync, _async] - ); + let onIncrementPressStart = useEffectEvent((initialStepDelay: number) => { + clearAsync(); + onIncrement(); + // Start spinning after initial delay + _async.current = window.setTimeout( + () => { + if (isNaN(maxValue) || isNaN(value) || value < maxValue) { + onIncrementPressStart(60); + } + }, + initialStepDelay + ); + }); + + let onDecrementPressStart = useEffectEvent((initialStepDelay: number) => { + clearAsync(); + onDecrement(); + // Start spinning after initial delay + _async.current = window.setTimeout( + () => { + if (isNaN(minValue) || isNaN(value) || value > minValue) { + onDecrementPressStart(60); + } + }, + initialStepDelay + ); + }); let cancelContextMenu = useCallback((e) => { e.preventDefault(); diff --git a/packages/@react-aria/utils/src/useEffectEvent.ts b/packages/@react-aria/utils/src/useEffectEvent.ts index 66ebdd2396f..f47ae391e1b 100644 --- a/packages/@react-aria/utils/src/useEffectEvent.ts +++ b/packages/@react-aria/utils/src/useEffectEvent.ts @@ -13,13 +13,13 @@ import {useCallback, useRef} from 'react'; import {useLayoutEffect} from './useLayoutEffect'; -export function useEffectEvent(fn) { +export function useEffectEvent(fn: T): T { const ref = useRef(null); useLayoutEffect(() => { ref.current = fn; }, [fn]); - return useCallback((...args) => { + return useCallback((...args) => { const f = ref.current; return f(...args); - }, []); + }, []) as T; } diff --git a/packages/@react-stately/color/src/useColorFieldState.ts b/packages/@react-stately/color/src/useColorFieldState.ts index be2c0af63df..a2da83df1c3 100644 --- a/packages/@react-stately/color/src/useColorFieldState.ts +++ b/packages/@react-stately/color/src/useColorFieldState.ts @@ -126,12 +126,8 @@ export function useColorFieldState( } }, [inputValue, safelySetColorValue, setInputValue, value, colorValue, parsedColor]); - let spinRef = useRef(parsedColor); - useEffect(() => { - spinRef.current = parsedColor; - }); let increment = useCallback(() => { - let newValue = addColorValue(spinRef.current, step); + let newValue = addColorValue(parsedColor, step); // if we've arrived at the same value that was previously in the state, the // input value should be updated to match // ex type 4, press increment, highlight the number in the input, type 4 again, press increment @@ -140,9 +136,9 @@ export function useColorFieldState( setInputValue(newValue.toString('hex')); } safelySetColorValue(newValue); - }, [safelySetColorValue, colorValue, setInputValue, step]); + }, [parsedColor, safelySetColorValue, colorValue, setInputValue, step]); let decrement = useCallback(() => { - let newValue = addColorValue(spinRef.current, -step); + let newValue = addColorValue(parsedColor, -step); // if we've arrived at the same value that was previously in the state, the // input value should be updated to match // ex type 4, press increment, highlight the number in the input, type 4 again, press increment @@ -151,7 +147,7 @@ export function useColorFieldState( setInputValue(newValue.toString('hex')); } safelySetColorValue(newValue); - }, [safelySetColorValue, colorValue, setInputValue, step]); + }, [parsedColor, safelySetColorValue, colorValue, setInputValue, step]); let incrementToMax = useCallback(() => { safelySetColorValue(MAX_COLOR); }, [safelySetColorValue]); diff --git a/packages/@react-stately/numberfield/src/useNumberFieldState.ts b/packages/@react-stately/numberfield/src/useNumberFieldState.ts index d28837e4a26..63151e84bed 100644 --- a/packages/@react-stately/numberfield/src/useNumberFieldState.ts +++ b/packages/@react-stately/numberfield/src/useNumberFieldState.ts @@ -157,12 +157,8 @@ export function useNumberFieldState( } }, [inputValue, setNumberValue, setInputValue, value, format, numberValue, parsedValue, step, minValue, maxValue, numberParser]); - let spinRef = useRef(parsedValue); - useEffect(() => { - spinRef.current = parsedValue; - }); let safeNextStep = useCallback((operation: '+' | '-', minMax: number) => { - let prev = spinRef.current; + let prev = parsedValue; if (isNaN(prev)) { // if the input is empty, start from the min/max value when incrementing/decrementing, @@ -184,7 +180,7 @@ export function useNumberFieldState( clampStep ); } - }, [minValue, maxValue, clampStep]); + }, [parsedValue, minValue, maxValue, clampStep]); let increment = useCallback(() => { let newValue = safeNextStep('+', minValue); From 4e4c6544b780560192aa28f0a6e97882a9d58fc9 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 13 Apr 2023 16:23:54 -0700 Subject: [PATCH 9/9] Fix lint --- packages/@react-aria/actiongroup/src/useActionGroupItem.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@react-aria/actiongroup/src/useActionGroupItem.ts b/packages/@react-aria/actiongroup/src/useActionGroupItem.ts index 85ea9dd5040..4c999cc8803 100644 --- a/packages/@react-aria/actiongroup/src/useActionGroupItem.ts +++ b/packages/@react-aria/actiongroup/src/useActionGroupItem.ts @@ -11,7 +11,7 @@ */ import {DOMAttributes, FocusableElement} from '@react-types/shared'; -import {Key, RefObject, useEffect, useRef} from 'react'; +import {Key, RefObject, useEffect} from 'react'; import {ListState} from '@react-stately/list'; import {mergeProps, useEffectEvent} from '@react-aria/utils'; import {PressProps} from '@react-aria/interactions'; @@ -54,7 +54,7 @@ export function useActionGroupItem(props: AriaActionGroupItemProps, state: Li return () => { onRemovedWithFocus(); }; - }, []); + }, [onRemovedWithFocus]); return { buttonProps: mergeProps(buttonProps, {