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

Give radio + checkboxes same input/label markup hierarchy #22840

Closed
tevyaw opened this issue Feb 23, 2019 · 6 comments
Closed

Give radio + checkboxes same input/label markup hierarchy #22840

tevyaw opened this issue Feb 23, 2019 · 6 comments
Labels
needs: votes Feature request awaiting community feedback. [auto] type: refactor The issue/PR is related to refactoring.

Comments

@tevyaw
Copy link

tevyaw commented Feb 23, 2019

I'm not a developer, so forgive me if I'm doing it wrong.

Describe the bug
On the checkout page, the payment method radio buttons are followed by , as sibling elements:

<input type="radio"></input>
<label></label>

But the terms checkbox has the label with the input inside of it, as a child element:

<label>
  <input type="checkbox"></input>
</label>

This is inconsistent, confusing, and makes it difficult to style radio buttons and checkboxes in a consistent way. In my experience the radio button method (sibling elements) is the more common method, and preferred. Please fix the terms checkbox (and any others), by moving the input element above the label, as a sibling.

To Reproduce
Steps to reproduce the behavior:

  1. Use Dev tools to inspect

Expected behavior
Consistent structure and relationship between input fields and labels, at least for checkboxes and radio buttons.

The Storefront theme uses a :before element on the radio buttons to style them. It could use a simpler "input[type=radio] + label:before" to select all radio buttons and give them a consistent style, but then you couldn't use the same for the checkboxes, because of their altered structure.

In fact, it's impossible with current CSS (I believe) to select this properly, because there's no "has child" selector, to select all labels that have a checkbox/radio input child. It makes it much more complicated to display custom checkboxes and radio buttons.

Isolating the problem (mark completed items with an [x]):

  • [✓] I have deactivated other plugins and confirmed this bug occurs when only WooCommerce plugin is active.
  • [✓] This bug happens with a default WordPress theme active, or Storefront.
  • [✓] I can reproduce this bug consistently using the steps above.

WordPress Environment

5.1
@tevyaw
Copy link
Author

tevyaw commented Feb 23, 2019

On more investigation: I believe it's actually impossible with current CSS to insert a custom checkbox/radio when the input is nested inside the label (like the terms checkbox) because we can't style the :checked pseudo-class of a parent label.

And we can't insert the :before pseudo-class with the custom checkbox/radio onto the input itself. That breaks standards. It works in Chrome, but shouldn't, and doesn't work in FF and Safari.

So best solution is ALWAYS put the input as a sibling element, before the label.

@tevyaw tevyaw changed the title Inconsistent radio button & checkboxes input/label relationship Can't style checkboxes because of inconsistent input/label hierarchy Feb 23, 2019
@mikejolley
Copy link
Member

@tevyaw I took a quick look at current usage of radios + checkboxes. Whilst you're right radios are not marked up in the same way as checkboxes, all radios are consistent with radios, and all checkboxes are consistent with checkboxes.

Since changing one of the other would break current implementations/themes I don't think we should update this. But we should ensure they are flexible enough to be styled.

makes it difficult to style radio buttons and checkboxes in a consistent way

Since they are different input types, they may be similarly styled but both required different handling anyway. Also the inconsistency itself between radio and checkbox has nothing to do with the ability to style either as your issue suggests.

I believe it's actually impossible with current CSS to insert a custom checkbox/radio when the input is nested inside the label

It's true you cannot style the parent element, however, all checkboxes have this format:

<label>
  <input type="checkbox">
  <span>Label text</span>
</label>

So you can style these with label input[type=checkbox] + span:before.

I'd go that route; changing the markup won't be possible without a major release due to disrupting existing styling in themes. Let me know if any of the above isn't clear/does not work.

Thanks

@mikejolley mikejolley added type: refactor The issue/PR is related to refactoring. needs: votes Feature request awaiting community feedback. [auto] labels Mar 1, 2019
@woocommercebot
Copy link
Collaborator

Thanks for the suggestion @tevyaw.

We'll keep an eye on the popularity of this request :)

@mikejolley mikejolley changed the title Can't style checkboxes because of inconsistent input/label hierarchy Give radio + checkboxes same input/label markup hierarchy Mar 1, 2019
@tevyaw
Copy link
Author

tevyaw commented Mar 1, 2019

Hi Mike. Thank you for the thoughtful and detailed reply. I actually realized that same thing after my last comment above and was able to update my CSS to reflect that. But I forgot to come back here and update this issue.

I still think the ideal would be consistency between the two, since they're displayed in similar ways. But I completely understand why you wouldn't want to change that, and break it for everyone who's written CSS, etc based on the current structure. Thanks again.

@rahuldhangar
Copy link

Thanks @tevyaw for creating this issue with detailed explanation. I am also facing exactly the same problem you have faced and there was no other official note on this. I felt relieved after reading your bug report and that I am not alone who struggled with this.

@tevyaw
Copy link
Author

tevyaw commented Aug 29, 2019

@rahuldhangar while it would be nice to have consistency, as @mikejolley pointed out, there is a way to target those. So you can target both kinds of checkboxes/radio buttons that WC uses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: votes Feature request awaiting community feedback. [auto] type: refactor The issue/PR is related to refactoring.
Projects
None yet
Development

No branches or pull requests

4 participants