Skip to content

fix: Extra Checkbox and Radio focus events #8567

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

snowystinger
Copy link
Member

@snowystinger snowystinger commented Jul 17, 2025

Closes #8045

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).

Because focus happens in mousedown, we can prevent default to stop focus from going to the label, this doesn't interfere with focus moving to the input itself which happens in click.

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Test by going to RAC's checkbox/radio stories with focus/blur actions. Click the label for the component multiple times, you should only see one set of events

🧢 Your Project:

@rspbot
Copy link

rspbot commented Jul 17, 2025

labelProps,
pressProps,
useMemo(() => ({
onMouseDown: e => e.preventDefault()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

@devongovett
Copy link
Member

Can you write a description of the issue and how this solves it?

Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is fine but it technically deviates from native behavior if the label was rendered in such a way that it didn't wrap the input (aka in that case a blur on the input happens every time the label is clicked).

@rspbot
Copy link

rspbot commented Jul 26, 2025

@rspbot
Copy link

rspbot commented Jul 26, 2025

@snowystinger
Copy link
Member Author

I guess this is fine but it technically deviates from native behavior if the label was rendered in such a way that it didn't wrap the input (aka in that case a blur on the input happens every time the label is clicked).

@LFDanLu I'm not sure I follow. I've provided a useCheckbox story with a separate label and input. Let me know if you're expecting something else for the behaviour.

@rspbot
Copy link

rspbot commented Jul 26, 2025

@rspbot
Copy link

rspbot commented Jul 26, 2025

@LFDanLu
Copy link
Member

LFDanLu commented Jul 28, 2025

@snowystinger right, so at the moment clicking on the label in the story you added won't trigger extra blur events due to the preventDefault you've added. However, contrast this to https://codepen.io/lfdanlu/pen/yyYVadZ?editors=1111 where it will fire blur events on the input when you repeatedly click on the label. Just wanted to highlight the difference from native in this specific case where the checkbox isn't actually wrapped/covered by the label.

again, this is probably fine IMO

@snowystinger
Copy link
Member Author

I see, but that behaviour doesn't change whether or not the label is separate or wrapping.
So I think in terms of our component, this is the preferred behaviour

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

Successfully merging this pull request may close these issues.

RAC Checkbox and Radio unexpectedly call onBlur
4 participants