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

Simpler custom control markup #25174

Closed
719media opened this issue Jan 2, 2018 · 6 comments
Closed

Simpler custom control markup #25174

719media opened this issue Jan 2, 2018 · 6 comments

Comments

@719media
Copy link
Contributor

719media commented Jan 2, 2018

I was running into issues with the custom-control (input and checkbox) padding and generally just not being as friendly as I would have liked.

  1. Instead of padding-left:1.5rem, why not padding-left:1rem, and then create margin-left:0.5, which allows for the checkbox itself to be rendered cleanly. Useful for me when putting a checkbox into a container (say, a <td> of a <table>) and want it to look clean and centered. Obviously I could just create my own class here, but I feel that this type of styling is more clean.

  2. Where possible, I like to stay away from position: absolute when creating something meant to be extending/used globally (e.g., the bootstrap library :) ). One benefit of avoiding absolute is that sizing the custom input becomes a little easier, because all you need to worry about at that point is the top offset (instead of having to worry about the padding-left spacing to make room for the absolute)

Anyway, I've put my ideas here:

https://jsfiddle.net/fqxcdp8h/

I hope you would at least consider implementing the fix for #1.

@719media
Copy link
Contributor Author

719media commented Jan 2, 2018

For example:

#25175

Please note that the pull request changes the semantic meaning of $custom-control-gutter to mean the actual "gutter" between the custom-control checkbox/radio instead of the total space reserved for it. I think this actually happens to also be more accurate of what a "gutter" is.

The default in _variables.css would be updated to 0.5rem instead of 1.5rem if this request was applied I imagine.

@mdo
Copy link
Member

mdo commented Jan 3, 2018

Hmm, this might be doable. I played with flexbox styles on the checkboxes before I rewrote them for Beta 3, but I couldn't make it work well. With the reduced markup, this gets easier. Lemme try this out on both the default and custom checks.

@mdo
Copy link
Member

mdo commented Jan 3, 2018

Only downside I see to this is, within the same form there being inconsistent spacing should your radio and checkbox be different dimensions. I don't know off the top of my head if they are yet, but I'll take a look shortly.

mdo added a commit that referenced this issue Jan 3, 2018
- Removes absolute positioning and relies on flexbox for input+label layout and spacing
- Adjusts the gutter variables to be true gutters, not gutters plus space for input
- Updates inline variants accordingly

This is an implementation of #25174, and a more complete attempt than #25175 at implementing it across all our checks.
@mdo
Copy link
Member

mdo commented Jan 3, 2018

Fresh pass for both input types at #25184.

@mdo mdo changed the title cleaner custom markup Simpler custom control markup Jan 4, 2018
@mdo mdo added the has-pr label Jan 4, 2018
@tmorehouse
Copy link
Contributor

I was playing around with making sizing available to custom radio/checkboxes, and figured that rather than using rem units for the ::before and ::after pseudo elements, and for the left padding and margin, using em units allows the indicator to scale with the size of the label text (which can be left in rem units).

@mdo
Copy link
Member

mdo commented Dec 17, 2018

I started playing with some new stuff again for v5, but I don't think we'll be significantly changing the markup here. See the Codepen I put together.

In both of your modification, the reduced padding-left means you'd have to adjust the padding-left or margin-left of any nested elements within .custom-check. That'd affect things like help text or any other custom HTML you'd want to add. Not exactly a deal breaker, but not the best.

Closing this out and slating some stuff for v5 though which will ideally combine those native and default checks for a lighter codebase and easier customization.

@mdo mdo closed this as completed Dec 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants