feat(RadioGroup)!: do not have value by default (#2017)

gravity-ui · Jan 9, 2025 · c43cdf5 · c43cdf5
1 parent c812a15
commit c43cdf5
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 30 deletions.
diff --git a/src/components/RadioGroup/__stories__/RadioGroup.stories.tsx b/src/components/RadioGroup/__stories__/RadioGroup.stories.tsx
@@ -1,3 +1,4 @@
+import {action} from '@storybook/addon-actions';
 import type {Meta, StoryObj} from '@storybook/react';
 
 import {Showcase} from '../../../demo/Showcase';
@@ -20,6 +21,10 @@ export const Default: Story = {
             {value: 'Value 2', content: 'Value 2'},
             {value: 'Value 3', content: 'Value 3'},
         ],
+        defaultValue: 'Value 1',
+        onUpdate: action('onUpdate'),
+        onFocus: action('onFocus'),
+        onBlur: action('onBlur'),
     },
 };
 

diff --git a/src/components/RadioGroup/__tests__/RadioGroup.test.tsx b/src/components/RadioGroup/__tests__/RadioGroup.test.tsx
@@ -198,7 +198,7 @@ describe('RadioGroup', () => {
     );
 
     describe('form', () => {
-        test('should submit first value by default', async () => {
+        test('should submit no value by default', async () => {
             let value;
             const onSubmit = jest.fn((e) => {
                 e.preventDefault();
@@ -215,7 +215,7 @@ describe('RadioGroup', () => {
             );
             await userEvent.click(screen.getByTestId('submit'));
             expect(onSubmit).toHaveBeenCalledTimes(1);
-            expect(value).toEqual([['radio-field', 'Value 1']]);
+            expect(value).toEqual([]);
         });
 
         test('should submit default value', async () => {

diff --git a/src/components/SegmentedRadioGroup/__stories__/SegmentedRadioGroup.stories.tsx b/src/components/SegmentedRadioGroup/__stories__/SegmentedRadioGroup.stories.tsx
@@ -1,3 +1,4 @@
+import {action} from '@storybook/addon-actions';
 import type {Meta, StoryObj} from '@storybook/react';
 
 import {Showcase} from '../../../demo/Showcase';
@@ -20,6 +21,10 @@ export const Default: Story = {
             <SegmentedRadioGroup.Option key="Value 2" value="Value 2" content="Value 2" />,
             <SegmentedRadioGroup.Option key="Value 3" value="Value 3" content="Value 3" />,
         ],
+        defaultValue: 'Value 1',
+        onUpdate: action('onUpdate'),
+        onFocus: action('onFocus'),
+        onBlur: action('onBlur'),
     },
 };
 

diff --git a/src/hooks/private/useRadioGroup/useRadioGroup.ts b/src/hooks/private/useRadioGroup/useRadioGroup.ts
@@ -1,4 +1,4 @@
-import {useControlledState, useUniqId} from '../..';
+import {useControlledState, useFocusWithin, useUniqId} from '../..';
 import type {ControlGroupOption, ControlGroupProps} from '../../../components/types';
 import {filterDOMProps} from '../../../components/utils/filterDOMProps';
 import {useFormResetHandler} from '../useFormResetHandler';
@@ -41,8 +41,7 @@ export function useRadioGroup<ValueType extends string = string>(
     const controlId = useUniqId();
     const [currentValue, setValueState] = useControlledState<string | null, ValueType>(
         value,
-        // FIXME: Do not set defaultValue to first option value
-        defaultValue ?? options[0]?.value ?? null,
+        defaultValue ?? null,
         onUpdate,
     );
 
@@ -51,6 +50,8 @@ export function useRadioGroup<ValueType extends string = string>(
         onReset: setValueState,
     });
 
+    const {focusWithinProps} = useFocusWithin({onFocusWithin: onFocus, onBlurWithin: onBlur});
+
     const handleChange = (event: React.ChangeEvent<HTMLInputElement>) => {
         setValueState(event.target.value as ValueType);
 
@@ -61,6 +62,7 @@ export function useRadioGroup<ValueType extends string = string>(
 
     const containerProps = {
         ...filterDOMProps(props, {labelable: true}),
+        ...focusWithinProps,
         role: 'radiogroup',
         'aria-disabled': disabled,
     };
@@ -73,9 +75,6 @@ export function useRadioGroup<ValueType extends string = string>(
         checked: currentValue === String(option.value),
         disabled: disabled || option.disabled,
         onChange: handleChange,
-        // FIXME: onFocus and onBlur should be on the container via useFocusWithin hook
-        onFocus: onFocus,
-        onBlur: onBlur,
         ref: fieldRef,
     }));
 

diff --git a/src/hooks/useFocusWithin/index.ts b/src/hooks/useFocusWithin/index.ts
@@ -1,2 +1,2 @@
 export {useFocusWithin} from './useFocusWithin';
-export type {UseFocusWithinProps, FocusWithinProps, UseFocusWithinResult} from './useFocusWithin';
+export type {UseFocusWithinProps, UseFocusWithinResult} from './useFocusWithin';
diff --git a/src/hooks/useFocusWithin/useFocusWithin.ts b/src/hooks/useFocusWithin/useFocusWithin.ts
@@ -3,23 +3,21 @@ import * as React from 'react';
 import {SyntheticFocusEvent} from './SyntheticFocusEvent';
 import {useSyntheticBlurEvent} from './useSyntheticBlurEvent';
 
-export interface FocusWithinProps {
+export interface UseFocusWithinProps<T extends Element = Element> {
     /** Whether the focus within events should be disabled. */
     isDisabled?: boolean;
     /** Handler that is called when the target element or a descendant receives focus. */
-    onFocusWithin?: (e: React.FocusEvent) => void;
+    onFocusWithin?: (e: React.FocusEvent<T>) => void;
     /** Handler that is called when the target element and all descendants lose focus. */
-    onBlurWithin?: (e: React.FocusEvent) => void;
-    /** Handler that is called when the the focus within state changes. */
+    onBlurWithin?: (e: React.FocusEvent<T>) => void;
+    /** Handler that is called when the focus within state changes. */
     onFocusWithinChange?: (isFocusWithin: boolean) => void;
 }
 
-export interface UseFocusWithinProps extends FocusWithinProps {}
-
-export interface UseFocusWithinResult {
+export interface UseFocusWithinResult<T extends Element = Element> {
     focusWithinProps: {
-        onFocus?: (event: React.FocusEvent<Element, Element>) => void;
-        onBlur?: (event: React.FocusEvent<Element, Element>) => void;
+        onFocus?: (event: React.FocusEvent<T>) => void;
+        onBlur?: (event: React.FocusEvent<T>) => void;
     };
 }
 
@@ -59,22 +57,23 @@ export interface UseFocusWithinResult {
  *
  *   return (
  *     <span {...focusWithinProps}>
- *       <Button onClick={() => {setOpen(true);}}>Select</Button>
+ *       <Button onClick={() => {setOpen(true)}}>Select</Button>
  *       <Popup open={open}>
  *          ...
  *       </Popup>
  *     </span>
  *   );
  *  }
- * }
  */
-export function useFocusWithin(props: UseFocusWithinProps) {
+export function useFocusWithin<T extends Element = Element>(
+    props: UseFocusWithinProps<T>,
+): UseFocusWithinResult<T> {
     const {onFocusWithin, onBlurWithin, onFocusWithinChange, isDisabled} = props;
 
     const isFocusWithinRef = React.useRef(false);
 
     const onFocus = React.useCallback(
-        (event: React.FocusEvent) => {
+        (event: React.FocusEvent<T>) => {
             if (!isFocusWithinRef.current && document.activeElement === event.target) {
                 isFocusWithinRef.current = true;
 
@@ -91,7 +90,7 @@ export function useFocusWithin(props: UseFocusWithinProps) {
     );
 
     const onBlur = React.useCallback(
-        (event: React.FocusEvent) => {
+        (event: React.FocusEvent<T>) => {
             if (!isFocusWithinRef.current) {
                 return;
             }
@@ -109,7 +108,7 @@ export function useFocusWithin(props: UseFocusWithinProps) {
         [onBlurWithin, onFocusWithinChange],
     );
 
-    const {onBlur: onBlurHandler, onFocus: onFocusHandler} = useFocusEvents({
+    const {onBlur: onBlurHandler, onFocus: onFocusHandler} = useFocusEvents<T>({
         onFocus,
         onBlur,
         isDisabled,
@@ -132,13 +131,13 @@ export function useFocusWithin(props: UseFocusWithinProps) {
     };
 }
 
-function useFocusEvents({
+function useFocusEvents<T extends Element = Element>({
     onFocus,
     onBlur,
     isDisabled,
 }: {
-    onFocus: (event: React.FocusEvent) => void;
-    onBlur: (event: React.FocusEvent) => void;
+    onFocus: (event: React.FocusEvent<T>) => void;
+    onBlur: (event: React.FocusEvent<T>) => void;
     isDisabled?: boolean;
 }) {
     const capturedRef = React.useRef(false);
@@ -173,7 +172,7 @@ function useFocusEvents({
 
         window.addEventListener('focus', handleFocus, {capture: true});
         // use focusin because a focus event does not bubble and current browser
-        // implementations fire focusin events after fucus event
+        // implementations fire focusin events after focus event
         window.addEventListener('focusin', handleFocusIn);
 
         return () => {
@@ -183,7 +182,7 @@ function useFocusEvents({
     }, [isDisabled, onBlur]);
 
     const onBlurHandler = React.useCallback(
-        (event: React.FocusEvent) => {
+        (event: React.FocusEvent<T>) => {
             if (
                 document.activeElement !== event.target &&
                 (event.relatedTarget === null ||
@@ -200,7 +199,7 @@ function useFocusEvents({
     const onSyntheticFocus = useSyntheticBlurEvent(onBlur);
 
     const onFocusHandler = React.useCallback(
-        (event: React.FocusEvent) => {
+        (event: React.FocusEvent<T>) => {
             capturedRef.current = true;
             targetRef.current = event.target;
             onSyntheticFocus(event);