rviscomi / trunk8

jQuery Truncation Plugin -- THIS PROJECT IS NO LONGER MAINTAINED
MIT License
703 stars 95 forks source link

utils.getLineHeight is including padding and other css styles into the computed hight. #25

Open niemyjski opened 11 years ago

niemyjski commented 11 years ago

Hello,

I'm looking into a bug and possible fix for an issue where utils.getLineHeight is including the css styles when computing if it should truncate. As such chrome renders it wrong and (still looking into it) Ie doesn't render at all.

trunk8

niemyjski commented 11 years ago

IE10 on left, chrome on right

niemyjski commented 11 years ago

Okay so there is a bug..

This line: line_height = $('#' + wrapper_id).innerHeight(); returns 0 in IE and is should be calling height() instead of innerHeight() as innerHeight also includes padding. http://midnight-coding.blogspot.com/2012/09/jquery-height-width-inner-and-outer.html

Also, I had to put a style in my css to override the padding for now. But it would be nice if the wrapper that was inserted ensured the padding was 0 and margin was 0.

Also it seems like during these calls IE returns a different value then firefox or chrome. Try this test (http://jsfiddle.net/VWBrf/1/) in each browser (http://stackoverflow.com/questions/9292529/jquery-height-returns-0-on-a-visible-div-why)

rviscomi commented 11 years ago

@niemyjski thanks for filing this issue.

innerHeight() may include padding, but there is no padding on the wrapper element.

It's possible that the padding/margin of the truncated element is affecting the line height calculation. Similar to the float/position fix, we may want to reset the spacing temporarily. See https://github.com/rviscomi/trunk8/blob/master/trunk8.js#L216

I don't have IE 10 but screenshots from this test show that innerHeight() works as expected: http://www.webpagetest.org/result/130321_47_c712037a66cbb227fa88bbc0ffcc1119/1/screen_shot/

niemyjski commented 11 years ago

Hello,

I found that it was inheriting the padding on the element. I had to do this to get it to report correctly:

line-height-test td {

padding: 0;
margin: 0;

}

I went into the IE tools and tried IE9 and IE8 as well and they both had the same exact issue. I used a bootstrap table that has 8px of padding on the td.

If you wan't I can give you access to a website that can reproduce this behavior.

rviscomi commented 11 years ago

A different approach could be separating the content (to be truncated) from the layout. So instead of your markup looking like this:

<td class="truncate padding">Lorem ipsum</td>

You may want to distinguish the two "truncate" and "padding" responsibilities:

<td class="padding"><span class="truncate">Lorem ipsum</span></td>

I do think that the plugin should handle spacing more appropriately, but this fix might work for you too.

niemyjski commented 11 years ago

I'm not exactly sure I'm following you. I have a plain jane table and my bootstrap styles are being applied. The text is just inside of my tables td.

rviscomi commented 11 years ago

However the padding/margin styles are being applied, whether by tag name or class name, my suggestion was to separate the element being styled from the element being truncated.

I added class "padding" just to demonstrate that the spacing styles do not apply to the inner span.

If you provide a code snippet I'd be happy to show you what I mean.

niemyjski commented 11 years ago

Hello,

I was able to get this to work by adding a span inside of my td's (to get this to work in ie). However, it would be nice if I could just add the truncate class to my td/th.

rviscomi commented 11 years ago

@niemyjski I've marked this as a bug and queued it for fixing.

Original thoughts on bug:

It's possible that the padding/margin of the truncated element is affecting the line height calculation. Similar to the float/position fix, we may want to reset the spacing temporarily. See https://github.com/rviscomi/trunk8/blob/master/trunk8.js#L216 [https://github.com/rviscomi/trunk8/issues/25#issuecomment-15265057]