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

btn's height differs between different tags #11358

Closed
xavier83ar opened this issue Nov 4, 2013 · 25 comments
Closed

btn's height differs between different tags #11358

xavier83ar opened this issue Nov 4, 2013 · 25 comments
Labels

Comments

@xavier83ar
Copy link

I know that this has been reported before, I've read all issues about this bug, but all of them are for 2.x and previous versions, and the newest bug reported is about 1 year's old.

However this bug is still present on bootstrap 3.0.1. I've made a test case to reproduce it, and a possible fix to it.

The issue

Here you can see the issue (screenshot)
https://www.dropbox.com/s/bns0wdl6cmzgmzr/Screenshot%20at%202013-11-04%2010%3A55%3A36.png

Here you can see the real test case:
https://dl.dropboxusercontent.com/u/1715488/bootstrap/btn-height-test.html

Bug fix proposal

I've removed vertical paddings. Instead, I've fixed button's height and line-height. It's been done inside of button-size(...) mixins, this is the result:

// Button sizes
// -------------------------
.button-size(@padding-vertical; @padding-horizontal; @font-size; @line-height; @border-radius) {
  @line-height-computed: floor(@font-size * @line-height); // ~20px
  // forzamos el height
  @btn-height: @line-height-computed + 2 * @padding-vertical + 2;

  padding: 0 @padding-horizontal;
  font-size: @font-size;
  line-height: @btn-height - 2;
  height: @btn-height;
  border-radius: @border-radius;
}

Screenshot:
https://www.dropbox.com/s/i8l0kj85xju315q/Screenshot%20at%202013-11-04%2011%3A07%3A09.png

Test case:
https://dl.dropboxusercontent.com/u/1715488/bootstrap/btn-height-fix.html

Notes

I've only tested in chrome, firefox and opera for linux, and only in desktops, but I've been using this method of "zero vertical paddings" for buttons since a few years with excelent results.
In fact I've been using bootstrap since 2.0 and I've always applied a patch to fix this (using this method) without any regressions or other issues, and I know that works ok in windows browsers, ios, and android browsers, but as I said I didn't make any serious test with them.

@cvrebert
Copy link
Collaborator

cvrebert commented Nov 4, 2013

It's a known limitation/bug in Firefox; see the "Cross-browser rendering" box on http://getbootstrap.com/css/#buttons
We also don't officially support old Presto-based Opera at this point.

@zlatanvasovic
Copy link
Contributor

This is classic problem in CSS. Browsers renders elements differenty and same class can differ between elements that use it.

A tip: Browsers renders elements differenty, but Chrome renders them the best. ;)

@zlatanvasovic
Copy link
Contributor

Possible duplicate of #8700.

@xavier83ar
Copy link
Author

I know that this is a known issue on firefox (and other browsers too), and that it is a common css problem rendering form elements, but if there is a solution that avoid those problems and differencies... I would at least try it.
I mean, what's the point of using tools like this, if it doesn't help to deal with browser's differences?, It's like if jquery's ajax function only work on some browsers and doesn't do it on others because of "browser implements ajax in a different way".

I mean, I think one of the main features of a ui css toolkit framework it's the cross browser consistency, I don't think that "the problem is firefox", or "it's opera" would be a good excuse.
And about Opera, for linux, presto based Opera is the latest version... but never mind, just to say, I know we are only a few :(

@zlatanvasovic
Copy link
Contributor

@xavier83ar I have same thing as you, but this styling strategy (with height instead of vertical padding) isn't well known and isn't good for default version of any CSS toolkit. :(

@xavier83ar
Copy link
Author

Oh, I didn't know. Do you know why? What's wrong with it? I don't mean that it's the best strategy, it just was something we (me and a designer friend) find and start using it 'cause we found it very usefull, but I've never read about that's was right or wrong...
I know that this approach is not good when you have text that wraps in more than one line, but in case of buttons (bootstrap buttons) have a rule to explicity avoid text wrapping, so I guess this is not that case.

@stephcurt
Copy link

Correct me if I am wrong, but lets look at the example on the http://getbootstrap.com/components/#input-groups-sizing page for the lg size.

The result is 1.33 on the line height and 10px on the padding. The span/input heights are both set to 45. If you actually do the math in this context, the height should be 45.9333px (as shows in the computed for the span) based on the line-height calculaton + 10px * 2 (vertical padding), the input element seems to floor the result to 45px, the span element is computing the actual result and then rounding up to 46 pixels.

I think we need to apply more logic to the line-height or the padding to ensure that the result ceilings or floors on the preprocessor level to make sure that the computed value will be the same on the css level, regardless of browser interpretation. Line-Height + Vertical Padding * 2 should equal a whole number value, which equals the height of the element.

This is my first comment on a git public project so please go easy if I am wrong here.

@stephcurt
Copy link

Sorry, line-height + padding + border

@stephcurt
Copy link

23.933 px on the line-height for the span vs 21 px on the line-height for the input

@stephcurt
Copy link

The font-size for both is 18. 18 * 1.33 is actually 23.933, not 21. In Chrome both compute as 23 (flooring).

@stephcurt
Copy link

So even if FireFox fixes their line-height calculation bug for input fields. why send over a number to the browser that can be interpreted ambiguously? Why not instead do the calculation is LESS and send over a whole pixel-number for line-height, and any other value that will end up rendering as a pixel?

@mdo
Copy link
Member

mdo commented Nov 30, 2013

For the time being, we won't be setting any height on buttons.

@mdo mdo closed this as completed Nov 30, 2013
@stephcurt
Copy link

I'm confused, how does that address the issue?

@stephcurt
Copy link

Not to be rude, just curious.

@mdo
Copy link
Member

mdo commented Dec 2, 2013

@stephenlcurtis Setting a height for one browser's mistake doesn't make sense to me. It doesn't address the issue, but I don't want to go down that path. Form controls are super inconsistent across all browsers and platforms, hence the extra CSS there.

@stephcurt
Copy link

That's not what I was implying at all. If you look over what I said, it turns out Firefox is the only browser doing to math properly on the 1.33, it is the rest that aren't. Rather than using a multiple of the current font size, which won't produce whole pixel results and can vary from browser to browser, using length (px) across the board so it is consistent.

@carasmo
Copy link

carasmo commented Dec 2, 2013

Are you saying that if you use a whole pixel value on the line-height on the .btn the rendering will be consistent in FireFox between input, button, and a.btn? That's not what I've found. Can you make a fiddle or Bin? I'd like to fix this as long as no other .btn in other stuff is affected and it doesn't require adding a height, which messes up all uses of the .btn inside groups, justified groups, vertical, and so on...

Thanks!
http://jsbin.com/eLofepE/2

@mdo
Copy link
Member

mdo commented Dec 2, 2013

Pretty sure we worked out the kinks in master the last few days, but do check out http://jsbin.com/ObUlUcI/4/ to see where we stand now.

Your fix does involve setting a fixed height though, and I'm not going to go down that route. Same with setting a super tall line-height to do the vertical centering of the text—that makes it just a tad too difficult to customize the contents of your buttons.

@carasmo
Copy link

carasmo commented Dec 2, 2013

@mdo : it works so much better. Input on btn-lg is great, the default sizes and smaller don't have the same height as the a.btn and but it's much closer than before.

http://jsbin.com/INOgalOl/1/

Thank you!!!

@stephcurt
Copy link

but .input-group-lg > .form-control, .input-group-lg > .input-group-addon, .input-group-lg > .input-group-btn > .btn already has a fixed font-size and height in px. It doesn't make sense for line-height to be a multiple when everything else around it is fixed in size. Changing it to 1.2 in your link fixes the rounding error for these font-sizes, but if you change them later you could run into the same issue and have to adjust the line-height multiple to prevent the rounding issue. So I don't see how making the line-height a multiple in this case affords you any more freedom.

@mdo
Copy link
Member

mdo commented Dec 2, 2013

We could use the computed line-heights in pixels, but that makes customization by just adding padding super rough. It's a trade-off, and I think we gain more by leaving it as-is.

@stephcurt
Copy link

I guess you have a point. It would be cleaner, but more difficult for the general web developer.

@sirNemanjapro
Copy link

When someone says Chrome has the best rendering, compared to Firefox, that person is not truly aware how stupid that statement is.

Temp solution for different sizes is to set a fixed line height or leave it to the browser to determine.

Edit #1
Also, I would like to request re-opening of this issue. It is not solved.

Edit #2
To be noted, if page iz zoomed in or out, Firefox is the only browser that wont destroy the buttons or input sizes.

@zlatanvasovic
Copy link
Contributor

@sirNemanjapro I say it. I don't see a point of your aggression.

Chrome has best rendering because it doesn't change height and doesn't add redundant styles. @xavier83ar's screenshot proves it.

@MrRhino
Copy link

MrRhino commented Aug 28, 2014

Great tip. Thanks

@twbs twbs locked and limited conversation to collaborators Aug 28, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

8 participants