Skip to content

Fix heights of <select> elements to match <inputs> when using .form-control #19967

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

Closed
wants to merge 3 commits into from

Conversation

leetskills
Copy link

@leetskills leetskills commented May 25, 2016

This fixes select.form-control heights not matching input.form-control referenced in #17194

screen shot 2016-05-25 at 11 52 51

Note: to do this, I've had to change $border-width to use rem units instead of px

@cvrebert
Copy link
Collaborator

cvrebert commented May 25, 2016

Note: to do this, I've had to change $border-width to use rem units instead of px

We already deliberately changed this to use px instead of rem due to other bugs:
85f08df#diff-d8ee409a461718bfb6240710c8c73382L63

@leetskills
Copy link
Author

Ah, fair enough. In my testing (Mac Chrome, Firefox), changing the font size to anything above 0 still rounded the border-width up to 1px computed. However, I hadn't tried it on my retina!

Maybe it's worth putting a comment somewhere saying if people change $font-size-root they should be aware that $border-width would also change? At the end of the day, the browser isn't doing anything wrong by reducing the computed border width based on the rem value. Not ideal, I know...

As it is now, the $input-height variables are kinda pointless as they're loose approximations of the input heights anyway.

Any ideas?

@jacobmllr95
Copy link
Contributor

I think changing the $border-width back to rem is a bad idea too because of the reasons @cvrebert mentioned BUT the select height needs to be fixed!

What do you think about calculating the $input-height in pixels because .form-control elements have a height of 38px and that are 2.375rem what could cause issues too.

To do that we need a helper (utilities) function like:

@function rem-to-px($value) {
  @return ($value / ($value * 0 + 1)) * 16px;
}

The $input-height calculations would look like:

$input-height:    (rem-to-px(($font-size-base * $input-line-height) + ($input-padding-y * 2)) + ($input-btn-border-width * 2)) !default;
$input-height-lg: (rem-to-px(($font-size-lg * $input-line-height) + ($input-padding-y-lg * 2)) + ($input-btn-border-width * 2)) !default;
$input-height-sm: (rem-to-px(($font-size-sm * $input-line-height) + ($input-padding-y-sm * 2)) + ($input-btn-border-width * 2)) !default;

mdo added a commit that referenced this pull request Oct 10, 2016
Uses a local variable and some calc love to counteract the border-width (times 2) from the height of the select. Fixes #17194 and nullifies #19967.
@mdo
Copy link
Member

mdo commented Oct 10, 2016

Addressed in #20874.

@mdo mdo closed this Oct 10, 2016
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.

4 participants