adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.87k stars 1.11k forks source link

Allow users to unselect radio group by double-clicking current radio button #4971

Closed crutchcorn closed 6 months ago

crutchcorn commented 1 year ago

Provide a general summary of the feature here

It would be nice to have the ability to double-click on a radio button group:

https://react-spectrum.adobe.com/react-aria/useRadioGroup.html

And have the currently selected item unselect back to nothing being selected.

πŸ€” Expected Behavior?

<RadioButtonGroup
    testId={"sort-order-group-sidebar"}
    className={styles.buttonsContainer}
    value={sort}
    label={"Sort order"}
    onChange={(val) => {
        if (sort === val) {
            setSort(null);
            return;
        }

        setSort(val as "newest");
    }}
>
    <RadioButton value={"newest"}>Newest</RadioButton>
    <RadioButton value={"oldest"}>Oldest</RadioButton>
</RadioButtonGroup>

Should unselect the currently selected radio button

😯 Current Behavior

Due to this line of code:

https://github.com/adobe/react-spectrum/blob/main/packages/%40react-stately/utils/src/useControlledState.ts#L36

The onChange is not triggered when the state is the same

πŸ’ Possible Solution

Maybe we could pass a alwaysTriggerOnChange property or something akin to preserve backwards compatible behavior.

I'd be happy to make a PR that introduces this or any other workaround we could land on.

πŸ”¦ Context

We have a search page that has a sidebar of controls that allow users to fine-tune their search experience. Among that list is a "newest" or "oldest" radio button group. While we initially had "Newest" as default, we quickly realized that this removed our backend's "search prioritization" by default

πŸ’» Examples

image

🧒 Your Company/Team

Unicorn Utterances

πŸ•· Tracking Issue

https://github.com/unicorn-utterances/unicorn-utterances/issues/661

crutchcorn commented 1 year ago

On second glance, there's a bit more complexity involved. onChange doesn't trigger on the same value being selected. To work around this (for now) I've implemented:

import {
    AriaRadioProps,
    mergeProps,
    useFocusRing,
    useRadio,
    useRadioGroup,
    VisuallyHidden,
} from "react-aria";
import { Button } from "components/button/button";
import {
    ChangeEvent,
    createContext,
    PropsWithChildren,
    useContext,
} from "preact/compat";
import { useLayoutEffect, useMemo, useRef } from "preact/hooks";
import {
    RadioGroupProps,
    RadioGroupState,
    useRadioGroupState,
} from "react-stately";

const RadioContext = createContext<RadioGroupState>(null);

interface RadioButtonGroupProps extends PropsWithChildren<RadioGroupProps> {
    class?: string;
    className?: string;
    testId?: string;
}

export function RadioButtonGroup(props: RadioButtonGroupProps) {
    const {
        children,
        label,
        class: className = "",
        className: classNameName = "",
        testId,
        value,
        onChange,
        ...rest
    } = props;
    const _state = useRadioGroupState(rest);

    /**
     * Remove this when the following is fixed:
     * https://github.com/adobe/react-spectrum/issues/4971
     */
    const state = useMemo(() => {
        return Object.assign({}, _state, {
            setSelectedValue: (value: string) => {
                _state.setSelectedValue(value);
                onChange(value);
            },
        });
    }, [_state]);

    const { radioGroupProps, labelProps } = useRadioGroup(props, state);

    useLayoutEffect(() => {
        _state.setSelectedValue(value);
    }, [value, _state]);

    return (
        <div
            {...radioGroupProps}
            class={`${className} ${classNameName}`}
            data-testid={testId}
        >
            <VisuallyHidden>
                <span {...labelProps}>{label}</span>
            </VisuallyHidden>
            <RadioContext.Provider value={state}>{children}</RadioContext.Provider>
        </div>
    );
}

export function RadioButton(props: AriaRadioProps) {
    const { children } = props;
    const state = useContext(RadioContext);
    const ref = useRef(null);
    const { inputProps, isSelected } = useRadio(props, state, ref);
    const { isFocusVisible, focusProps } = useFocusRing();

    const mergedProps = mergeProps(inputProps, focusProps, {
        onChange: (e: ChangeEvent) => {
            e.preventDefault();
        },
        onClick: (e: MouseEvent) => {
            setTimeout(() => {
                if (e.defaultPrevented) return;
                state.setSelectedValue(props.value);
            }, 0);
        },
    });

    return (
        <label>
            <VisuallyHidden>
                <input {...mergedProps} ref={ref} />
            </VisuallyHidden>
            <Button
                tag="div"
                variant={isSelected ? "primary-emphasized" : "primary"}
                isFocusVisible={isFocusVisible}
            >
                {children}
            </Button>
        </label>
    );
}

It's ugly and likely doesn't work on SSR/SSG, but for our client-side-only (for now) needs it works fine.

Still more than happy to implement a longer-term solution upstream so we can remove this hack πŸ˜…

reidbarber commented 1 year ago

I'm not sure this is something we would want to implement, since it goes against native radio behavior:

At all times, exactly one of the radio buttons in a set is checked. If none of the elements of a set of radio buttons specifies `CHECKED', then the user agent must check the first radio button of the set initially.

Source: https://www.w3.org/TR/html401/interact/forms.html#radio

Could you consider a ListBox for this use case?

ktabors commented 6 months ago

Closing and we can reopen if it's still a problem.