input-size is generating different CSS than I would expect #15074

Closed
caleb opened this Issue Nov 11, 2014 · 4 comments

5 participants

@caleb
caleb commented Nov 11, 2014

I'm using bootstrap-sass and in that version control sizing doesn't work (e.g. form-group-sm or form-group-lg etc, see twbs/bootstrap-sass#763)

Looking into what is causing this led me to the Less version of bootstrap, and I think something is wrong with the input-size mixin and how it's being used. While the Sass version is broken, and I think the Less version works, I believe the Less version isn't generating the intended CSS.

The less code from forms.less (https://github.com/twbs/bootstrap/blob/master/less/forms.less#L315):

.input-lg,
.form-group-lg .form-control {
  .input-size(@input-height-large; @padding-large-vertical; @padding-large-horizontal; @font-size-large; @line-height-large; @input-border-radius-large);
}

Generates this CSS code:

textarea.input-sm,
textarea.form-group-sm .form-control,
select[multiple].input-sm,
select[multiple].form-group-sm .form-control {
  height: auto;
}
…

That doesn't look like the intended CSS. I would expect this:

textarea.input-sm,
.form-group-sm textarea.form-control,
select[multiple].input-sm,
.form-group-sm select[multiple].form-control {
  height: auto;
}
…

Is this wrong? I would be interested in trying to create a patch if it is.

-Caleb

@hnrch02 hnrch02 added the css label Nov 11, 2014
@caleb caleb changed the title from Form Control Sizing not what I would expect to input-size is generating different CSS than I would expect Nov 11, 2014
@caleb
caleb commented Nov 14, 2014

This jsbin shows the bug: http://jsbin.com/nomobegivu/1/edit?html,css,output

The textarea has rows=20 set, but the output has a set height because the height: auto rule listed above isn't being matched. Remove the form-group-lg class and the text area goes back to its correct height.

@mdo mdo added this to the v3.3.2 milestone Nov 30, 2014
@mdo mdo closed this in 7c71b48 Nov 30, 2014
@glebm glebm added a commit to twbs/bootstrap-sass that referenced this issue Nov 30, 2014
@glebm glebm converter: support for twbs/bootstrap#15074 fix 5353f31
@mmiller678

I don't think the LESS fix works, but I could be mistaken. The resulting CSS I get is:
select.form-group-sm .form-control {..}
textarea.form-group-sm .form-control,
select[multiple].form-group-sm.form-control {..}

I expected:
.form-group-sm select.form-control {..}
.form-group-sm textarea.form-control,
.form-group-sm select[multiple].form-control {..}

The issue seems to be in the mixin as its pre-pending the selector. The selector is pre-pended even if the classes are nested. See this LESS example here:
http://lesscss.org/features/#parent-selectors-feature-changing-selector-order

Not sure of the fix as I played with it a bit, but by no means am I a LESS expert.

@cvrebert
Bootstrap member
cvrebert commented Dec 3, 2014

Agreed, the nonsensical selectors are still present after the "fix":
7c71b48#diff-6972cbb37f0ec48771217d4915ea7e6fR2553
CC: @mdo

@cvrebert cvrebert reopened this Dec 3, 2014
@mmiller678

This is how I got it to work without changing the mixin:

.form-group-sm .form-control 
{
    &:extend(.input-sm);
}
.form-group-sm select.form-control 
{
    &:extend(select.input-sm);
}

.form-group-sm select[multiple].form-control 
{
    &:extend(select[multiple].input-sm);
}

.form-group-sm textarea.form-control 
{
    &:extend(textarea.input-sm);
}

Do the same for lg styles. Not very elegant and a little verbose, but because of how the mixins written I don't think there are a lot of options.

@glebm glebm added a commit to twbs/bootstrap-sass that referenced this issue Dec 17, 2014
@glebm glebm converter: support for twbs/bootstrap#15074 fix 4b9095e
@cvrebert cvrebert added the confirmed label Jan 12, 2015
@glebm glebm added a commit to twbs/bootstrap-sass that referenced this issue Jan 18, 2015
@glebm glebm converter: support for twbs/bootstrap#15074 fix 60de6c5
@mdo mdo modified the milestone: v3.3.2, v3.3.3 Jan 19, 2015
@cvrebert cvrebert modified the milestone: v3.3.5, v3.3.4 Mar 15, 2015
@mdo mdo added a commit that closed this issue Mar 29, 2015
@mdo mdo Fixes #15074: Manually handle input sizing in form groups instead of …
…using mixins because nesting
df8010b
@mdo mdo closed this in df8010b Mar 29, 2015
@rroblak rroblak pushed a commit to insight-meditation-center/imc-bootstrap-sass that referenced this issue Nov 9, 2015
@glebm glebm converter: support for twbs/bootstrap#15074 fix b2f5011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment