Skip to content
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

[Label] hover effect should match label element #1233

Closed
penx opened this issue Mar 8, 2022 · 6 comments · Fixed by #1686
Closed

[Label] hover effect should match label element #1233

penx opened this issue Mar 8, 2022 · 6 comments · Fixed by #1686
Assignees
Labels
Package: react/label Resolution: Needs Investigation This issue needs more investigation Topic: Accessibility This issue has to do with accessibility Type: Enhancement Small enhancement to existing primitive/feature

Comments

@penx
Copy link
Contributor

penx commented Mar 8, 2022

Bug report

Current Behavior

If you hover over a <label /> associated with a Radix Checkbox (or <input />), the checkbox's hover state is triggered.

If you hover over a <LabelPrimitive.Root /> associated with a checkbox (or <input />), the checkbox's hover state is not triggered.

Expected behavior

The behaviour of <LabelPrimitive.Root /> should match <label /> .

If you follow the documentation for Checkbox:

https://www.radix-ui.com/docs/primitives/components/checkbox

It shows usage with a standard label element, including a hover effect. If you then switch to using Radix' label component, you lose the hover effect.

Reproducible example

Hover over the labels in this example:

https://codesandbox.io/s/ecstatic-rosalind-eky4tl

Your environment

Software Name(s) Version
Radix Package(s) react-checkbox, react-label 0.1.5
React 17.0.2
Browser Chromium 98.0.4758.109
Operating System macOS 11.6
@penx penx changed the title Label Primitive hover effect should match label element Label - hover effect should match label element Mar 8, 2022
@penx penx changed the title Label - hover effect should match label element [Label] hover effect should match label element Mar 8, 2022
@benoitgrelard
Copy link
Collaborator

Hi @penx,

Thanks for raising this.

Whilst I agree it would be nice to match that, I don't think it is possible to force a CSS state.
The only way I think we could get "close" would be to introduce our own data-hover state on controls which users would have to style on top of :hover. Something like that.

:hover, [data-hover] { … }

Which is not ideal and not really obvious for users.

The easiest thing to do if you have control might be to nest the control inside the Label and style the hover this way:

.label:hover .control, .control:hover { … }

Can you think of other ideas?

✌️

@penx
Copy link
Contributor Author

penx commented Mar 30, 2022

Can you think of other ideas?

Use a label element? 😁

I'd be keen to know why this isn't a label element already, I'm sure there's good reason but I wasn't able to figure it out from the past PRs I looked at.

@penx
Copy link
Contributor Author

penx commented Mar 30, 2022

Another thought is that browser vendors should match the label hover effect when role="label" is used, so unless this has already been decided against by vendors (or is part of a spec) perhaps it's worth raising issues against webkit, chromium and firefox.

I don't see any relevant open or closed issues in any of these trackers when searching for "role label hover"

@jjenzz
Copy link
Contributor

jjenzz commented Mar 30, 2022

historically, it needed to be role="label" because the native one wouldn't work with the custom button controls, but those controls have since been rejigged (checkbox etc.) to work better with native label so isn't likely necessary for those anymore.

there have been a few separate occasions i've mentioned it can probably switch to label but team would need to look into it to make sure.

the only case it might be needed is custom select so that clicking label focuses custom select but could add a focus listener to the native one perhaps and redirect it.

@benoitgrelard
Copy link
Collaborator

Ok cool, we'll do another pass to see if we can improve things here.

@keul
Copy link

keul commented Sep 21, 2022

Happy to see there's an open discussion around this topic! I really appreciate it.
I was a bit surprised of seeing span as label and the mouse pointer behaviour.

Let not forget the first rule of ARIA: https://www.a11y-collective.com/the-first-rule-for-using-aria/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: react/label Resolution: Needs Investigation This issue needs more investigation Topic: Accessibility This issue has to do with accessibility Type: Enhancement Small enhancement to existing primitive/feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants