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

Updates the native and custom checks to use flexbox #25184

Closed
wants to merge 2 commits into from
Closed

Conversation

mdo
Copy link
Member

@mdo mdo commented 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.

- 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.
@ysds
Copy link
Member

ysds commented Jan 3, 2018

Please add flex-shrink: 0 to .custom-control-label::before for long label.

image

display: block;
min-height: (1rem * $line-height-base);
padding-left: $custom-control-gutter;
display: flex;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the same display: flex in my pull request, but now I'm trying to remember the reason for even having it. Does removing this cause any negative effect? Or does it have a purpose for inclusion? I fiddled around a bit and didn't see a reason for just leaving the display property off... what do you think?

margin-right: $form-check-inline-margin-x;

// Undo .form-check-input defaults and add some `margin-right`.
.form-check-input {
position: static;
margin-top: 0;
margin-right: $form-check-inline-input-margin-x;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should $form-check-inline-input-margin-x be consolidated with $form-check-input-gutter?
If not, should the default value in _variables be changed so that they are equal?

@@ -87,6 +84,10 @@
background-position: center center;
background-size: $custom-control-indicator-bg-size;
}

&:not(:empty)::before {
margin-right: $custom-control-gutter;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.custom-control-label will never be empty because of the before and after pseudo-elements. It will always have margin-right.

@mdo
Copy link
Member Author

mdo commented Jan 21, 2018

Note to myself, check #25337.

@tmorehouse
Copy link
Contributor

@mdo, To allow the indicator to scale with the text size of the label, I would suggest that the indicator width/height (and possibly padding/gutter) be switched to em units instead of rem units, as it would then inherit the size from the label text size, and scale quite nicely.

This would then allow easy support for custom-control-sm and custom-control-lg sizing classes (with no special handling for the indicators needed).

@mdo
Copy link
Member Author

mdo commented Dec 17, 2018

Punting on this per #25174 (comment).

Using flex on these checks isn't really doable without more complications—we'd need row direction for the input and label, but for any nested content that's a sibling of the input and label (like help text), it'd require column direction.

@mdo mdo closed this Dec 17, 2018
@mdo mdo deleted the flex-checks branch December 17, 2018 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants