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

Form labels for check boxes might be misbehaving #18210

Open
rbrishabh opened this issue Nov 5, 2019 · 10 comments
Milestone

Comments

@rbrishabh
Copy link
Contributor

@rbrishabh rbrishabh commented Nov 5, 2019

For example, in https://material-ui.com/components/checkboxes/ clicking on checkboxes focuses on the form label "Assign responsibility" at irregular times.

ezgif com-video-to-gif (1)

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Form label focus keeps changing irregularly.

Expected Behavior 🤔

Form label doesn't change focus like this for checkboxes.

Steps to Reproduce 🕹

  1. Go to https://material-ui.com/components/checkboxes/
  2. Scroll down to 'Checkboxes with FormGroup'
  3. Click on random checkboxes
  4. See error

Your Environment 🌎

Found while contributing to Taskclutster, Mozilla. Reproducible in Chrome with both Windows and Mac devices. Not reproduced in Mozilla Firefox.

@oliviertassinari

This comment has been minimized.

Copy link
Member

@oliviertassinari oliviertassinari commented Nov 5, 2019

@rbrishabh Hum, I'm wondering, the behavior seems strictly speaking correct. The input is blurred. What's wrong with it?

@rbrishabh

This comment has been minimized.

Copy link
Contributor Author

@rbrishabh rbrishabh commented Nov 6, 2019

@oliviertassinari, notice the Assign responsibility label in the gif in the above comment, on clicking any of the checkboxes, it becomes blue and then again becomes white at irregular times.

When I looked into developer options, the label gets a new class whenever one of the checkboxes is being clicked. That is when the click is active on one of the checkboxes, the color of the label changes, (in this case becomes blue). I also noticed, when frequently clicking on the checkboxes randomly, the label becomes blue and white even more irregularly.

I also feel that the label shouldn't change on one of the checkboxes is being clicked because it doesn't look right to me. That's just my opinion.

Could I explain the issue clearly? If not, I will try explaining it again! Thanks.

@oliviertassinari

This comment has been minimized.

Copy link
Member

@oliviertassinari oliviertassinari commented Nov 6, 2019

@rbrishabh Maybe we should disable this focused style altogether for checkboxes, radios, and switches?

@rbrishabh

This comment has been minimized.

Copy link
Contributor Author

@rbrishabh rbrishabh commented Nov 6, 2019

Yes! Exactly what I was trying to point out!

Would it be okay if I make a PR?
@oliviertassinari

@oliviertassinari

This comment has been minimized.

Copy link
Member

@oliviertassinari oliviertassinari commented Nov 6, 2019

@rbrishabh I have no idea where to start looking at; but if you have some time, sure, I think that it would be great to explore :)

@rbrishabh

This comment has been minimized.

Copy link
Contributor Author

@rbrishabh rbrishabh commented Nov 6, 2019

Definitely! I'd love to take a look.
I'll make sure to ask if I get stuck.

Thanks!

@eps1lon

This comment has been minimized.

Copy link
Member

@eps1lon eps1lon commented Nov 11, 2019

I don't think that is caused by focused but the shared state. The second one displays an error when the number of checked checkboxes is not equal to two.

@rbrishabh

This comment has been minimized.

Copy link
Contributor Author

@rbrishabh rbrishabh commented Nov 11, 2019

Hey @eps1lon! Please find more information about the issue in this (#18210 (comment))

@eps1lon

This comment has been minimized.

Copy link
Member

@eps1lon eps1lon commented Nov 12, 2019

Right there is an issue with the focused state but it is caused by our demo wrapper having a negative tabindex. These elements can be focused on mousedown while a label moves focus to it's corresponding input in mouseup.

We originally added it to test tabbing behavior better (#15521 (comment)). The easiest workaround would be to move this further down but I think we should expose some functionality that is also visible to visitors. Testing tab behavior is not just something useful for maintainers but for users that evaluate this library. I think we can be more confident by exposing this testing functionality.

@oliviertassinari

This comment has been minimized.

Copy link
Member

@oliviertassinari oliviertassinari commented Nov 13, 2019

Testing tab behavior is not just something useful for maintainers but when evaluating. I think we can be more confident by exposing this testing functionality.

💯

@oliviertassinari oliviertassinari added this to the v5 milestone Nov 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.