Skip to content

RAC Checkbox and Radio unexpectedly call onBlur #8045

Open
@benwilliams140

Description

@benwilliams140

Provide a general summary of the issue here

Sequential presses to a Radio or Checkbox component will call onBlur upon the second press.

🤔 Expected Behavior?

onBlur shouldn't be called on sequential presses of the "input" element (red border)

Image

😯 Current Behavior

If a checkbox or radio is already focused, pressing on the element a second time will call onBlur (since it is technically the label getting pressed, not the actual input).

💁 Possible Solution

Other input elements (eg. TextField) leave the rendering of the Label and Input elements up to the consumer.

🔦 Context

This is technically expected based on default HTML behaviour and the structure of the components, but unexpected for a user (since it looks like they are pressing on the input).

As a side effect, my validation for these components is running unexpectedly when hooked into a form.

🖥️ Steps to Reproduce

https://codesandbox.io/p/sandbox/exciting-nightingale-wjx2jz

Version

react-aria-components@1.7.1

What browsers are you seeing the problem on?

Microsoft Edge, Safari, Chrome, Firefox

If other, please specify.

No response

What operating system are you using?

MacOS

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

Activity

snowystinger

snowystinger commented on Apr 8, 2025

@snowystinger
Member

We could probably change it to call onBlur/onFocus for focusin/focusout on the label instead. I think that'd be the behaviour you're expecting. Right now it's just using useFocusable for them

let {focusableProps} = useFocusable(mergeProps(props, {
which is being sent to the input only.
This would be an override for RAC, though, since the hook doesn't know that the label will wrap the input.

uniqueeest

uniqueeest commented on Apr 8, 2025

@uniqueeest

@snowystinger hi. i want to resolve this issue.

uniqueeest

uniqueeest commented on Apr 15, 2025

@uniqueeest

@snowystinger Is it correct that you want me to move the onFocus, onBlur event to the label? The same issue occurs even if you proceed in that way. I'd appreciate a little help with this situation.

    <label
      {...mergeProps(DOMProps, labelProps, hoverProps, renderProps)}
      ref={ref}
      onFocus={props.onFocus}
      onBlur={props.onBlur}
      data-selected={isSelected || undefined}
      data-pressed={isPressed || undefined}
      data-hovered={isHovered || undefined}
      data-focused={isFocused || undefined}
      data-focus-visible={isFocusVisible || undefined}
      data-disabled={isDisabled || undefined}
      data-readonly={state.isReadOnly || undefined}
      data-invalid={state.isInvalid || undefined}
      data-required={state.isRequired || undefined}>
      <VisuallyHidden elementType="span">
        <input {...mergeProps(_inputProps, focusProps)} ref={inputRef} /> // except onBlur, onFocus
      </VisuallyHidden>
      {renderProps.children}
    </label>
nwidynski

nwidynski commented on Apr 15, 2025

@nwidynski
Contributor

@uniqueeest Something like this should do the trick:

let {onBlur, onFocus, ...inputDOMProps} = inputProps

// ...
<label
  {...mergeProps(DOMProps, labelProps, hoverProps, renderProps)}
  ref={ref}
  onFocus={onFocus}
  onBlur={onBlur}
  data-selected={isSelected || undefined}
  data-pressed={isPressed || undefined}
  data-hovered={isHovered || undefined}
  data-focused={isFocused || undefined}
  data-focus-visible={isFocusVisible || undefined}
  data-disabled={isDisabled || undefined}
  data-readonly={state.isReadOnly || undefined}
  data-invalid={state.isInvalid || undefined}
  data-required={state.isRequired || undefined}>
  <VisuallyHidden elementType="span">
    <input {...mergeProps(inputDOMProps, focusProps)} ref={inputRef} />
  </VisuallyHidden>
  {renderProps.children}
</label>
uniqueeest

uniqueeest commented on Apr 16, 2025

@uniqueeest

@nwidynski I don't think that's a good idea, because we have two different types of focus, blur, so we have a type conflict.

nwidynski

nwidynski commented on Apr 16, 2025

@nwidynski
Contributor

@uniqueeest You can safely ignore the type errors or recast the FocusableElement. A type mismatch was expected as this is an override, as @snowystinger mentioned.

Just make sure to also adjust the type signature of the component.

uniqueeest

uniqueeest commented on Apr 16, 2025

@uniqueeest

@nwidynski

  const handleLabelFocus = (e: React.FocusEvent<HTMLLabelElement>) => {
    inputProps.onFocus?.(e as unknown as React.FocusEvent<HTMLInputElement>);
  };
  
  const handleLabelBlur = (e: React.FocusEvent<HTMLLabelElement>) => {
    inputProps.onBlur?.(e as unknown as React.FocusEvent<HTMLInputElement>);
  };

  let DOMProps = filterDOMProps(props);
  delete DOMProps.id;

  return (
    <label
      {...mergeProps(DOMProps, labelProps, hoverProps, renderProps)}
      ref={ref}
      onFocus={handleLabelFocus}
      onBlur={handleLabelBlur}
      data-selected={isSelected || undefined}
      data-pressed={isPressed || undefined}
      data-hovered={isHovered || undefined}
      data-focused={isFocused || undefined}
      data-focus-visible={isFocusVisible || undefined}
      data-disabled={isDisabled || undefined}
      data-readonly={state.isReadOnly || undefined}
      data-invalid={state.isInvalid || undefined}
      data-required={state.isRequired || undefined}>
      <VisuallyHidden elementType="span">
        <input {...mergeProps(inputProps, focusProps)} onFocus={undefined} onBlur={undefined} ref={inputRef} />
      </VisuallyHidden>
      {renderProps.children}
    </label>

If you do this, there won't be an onblur event😂

benwilliams140

benwilliams140 commented on Apr 16, 2025

@benwilliams140
Author

What about something like this? facebook/react#6410 (comment)

Note the "focusenter" and "focusleave" logs (these are the checks you'd want)

nwidynski

nwidynski commented on Apr 16, 2025

@nwidynski
Contributor

@uniqueeest I'm sorry, I read this issue wrong. What you actually want to do is:

let {labelProps, inputProps, isSelected, isDisabled, isPressed} = useRadio({
  ...removeDataAttributes<RadioProps>(props),
  onFocus: undefined,
  onBlur: undefined,
  // ReactNode type doesn't allow function children.
  children: typeof props.children === 'function' ? true : props.children
}, state, inputRef);
let {focusableProps} = useFocusable(props, ref);

//...
<label
  {...mergeProps(DOMProps, labelProps, hoverProps, focusableProps, renderProps)}
  ref={ref}

This will change the target of focus events to be the label instead of the hidden input. You can further differentiate between the event types by applying the example @benwilliams140 mentioned.

uniqueeest

uniqueeest commented on Apr 17, 2025

@uniqueeest

@nwidynski That way, the onBlur event is called right away from the approach...
As soon as I click on the label, the focus goes straight into the input and the label seems to be onblur.

benwilliams140

benwilliams140 commented on Apr 17, 2025

@benwilliams140
Author

@uniqueeest yes that's default browser behaviour (to focus the input after clicking on the label). You would need to differentiate between the event types to ignore blur events where focus is still within the label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinggood first issueGood for newcomers

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Participants

      @snowystinger@benwilliams140@nwidynski@uniqueeest

      Issue actions

        RAC Checkbox and Radio unexpectedly call onBlur · Issue #8045 · adobe/react-spectrum