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

Custom control gutter fix #27797

Merged
merged 7 commits into from
Dec 14, 2018
Merged

Conversation

gijsbotje
Copy link
Contributor

The current $custom-control-gutter defines the gutter + the indicator size. This works great if the indicator is the same size for each custom control that uses this variable. While working on the switch element in #23004 i realized that i had to do weird calculations to get the same gutter as for the radio and checkbox. The actual gutter is .5rem, but is now 1.5rem. This PR changes that so the gutter variable is actually the space between the indicator and the label.

gijsbotje and others added 3 commits April 12, 2018 16:43
pull from official package
- changed gutter definition
- changed left and padding left on the custom checkboxes and radios
@gijsbotje gijsbotje requested a review from a team as a code owner December 7, 2018 08:10
Copy link
Member

@MartijnCuppens MartijnCuppens left a comment

Choose a reason for hiding this comment

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

This LGTM because the variable name now represents what is does.

@gijsbotje
Copy link
Contributor Author

this might introduce a breaking change for people that use the sass source code. though i doubt they would use this variable.

@MartijnCuppens
Copy link
Member

Indeed. But maybe we should document this in the release notes? @mdo, what do you think?

@XhmikosR XhmikosR requested review from mdo and removed request for mdo December 8, 2018 15:01
@mdo mdo mentioned this pull request Dec 14, 2018
@XhmikosR XhmikosR merged commit c618f67 into twbs:v4-dev Dec 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants