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

Inconsistent checkbox placement when a hidden element is between a legend and .form-check #31472

Open
tegandbiscuits opened this issue Aug 12, 2020 · 8 comments

Comments

@tegandbiscuits
Copy link

  • Operating system: macOS 10.15.6
  • Browser: Chrome 84.0.4147.105

In the alpha for 5.0.0 when a legend is followed by a hidden element (tested with .d-none and input[type="hidden"]) and div.form-check, then the checkbox will be on the same line as the legend but not the label, and also makes the checkbox not clickable (though it's possible to check by focusing on it). This seems to be the case with normal checkboxes and switches.

Example problematic markup:

<form>
  <fieldset>
    <legend>Unexpected</legend>

    <p class="d-none">Hello</p>
    <div class="form-check">
      <input class="form-check-input" type="checkbox" id="foo">
      <label class="form-check-label" for="foo">Foo</label>
    </div>
  </fieldset>
</form>

Produces this

Screen Shot 2020-08-12 at 3 34 40 PM

I would expect it to lay out like this (and also be clickable)

Screen Shot 2020-08-12 at 3 35 33 PM

Code Pen
https://codepen.io/tegandbiscuits/pen/poygjPE

The current behavior is also not what happens with checkboxes in Bootstrap 4.

Also this seems to be able to be resolved by wrapping both the hidden element and .form-check in a div.

@ffoodd
Copy link
Member

ffoodd commented Aug 18, 2020

We're basically using an uncontained float for checkboxes and radio buttons, which may cause troubles as you spotted 👍

@mdo
Copy link
Member

mdo commented Aug 18, 2020

@ffoodd I think this is more likely caused by the float on the <legend> than the checkbox.

@ffoodd
Copy link
Member

ffoodd commented Aug 19, 2020

@mdo you're right, been too qucik on this. We're trying to clear the float with legend + * { clear: left; }, which has no effect on a hidden element…

Not sure about a potential fix here:

  1. our clearfix won't work either,
  2. we do not have any clear utilities,
  3. avoiding this case with CSS would result in a bloated selector, eg:
legend + *:not(.d-none):not([hidden]):not([type="hidden"]), 
legend + .d-none + *, 
legend + [hidden] + *,
legend + [type="hidden"] + * {
  clear: left;
}

Maybe adding clear utility would be a better fix, to allow user to manually clear their floats in edge cases?

@ffoodd ffoodd removed the has-pr label Aug 19, 2020
@mdo
Copy link
Member

mdo commented Aug 19, 2020

I think we might need to look at changing the <legend> changes we made. I feel like that was done relatively recently. Won't have time to play with it more for a little bit.

@ffoodd
Copy link
Member

ffoodd commented Aug 20, 2020

Tried something using display: contents but then realized support is pretty bad and buggy

There might be a better way using grid or something, not sure yet.

TBH, I'd be in favor of dropping float: left here, since our standard build does not need it —no borders on fieldset— and it can be easily managed using .float-left utility (which would then be opt-in).

The better way around IMHO would be to add clear utilities.

@mdo
Copy link
Member

mdo commented Sep 17, 2020

Let's drop that float and that should fix it. Wanna open a PR @ffoodd or should I?

@ffoodd
Copy link
Member

ffoodd commented Sep 18, 2020

Please do @mdo, I won't be able to do it until next week or so — or simply wait for a week. ;)

@mdo
Copy link
Member

mdo commented Sep 18, 2020

Thinking about this specific example again, you might just want to add a good ol' fashioned <div class="clearfix"></div> before the hidden element. That'll fix the issue.

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

Successfully merging a pull request may close this issue.

3 participants