trunk8 icon indicating copy to clipboard operation
trunk8 copied to clipboard

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

Open niemyjski opened this issue 12 years ago • 9 comments

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 avatar Mar 20 '13 22:03 niemyjski

IE10 on left, chrome on right

niemyjski avatar Mar 20 '13 22:03 niemyjski

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)

niemyjski avatar Mar 21 '13 17:03 niemyjski

@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/

rviscomi avatar Mar 21 '13 20:03 rviscomi

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.

niemyjski avatar Mar 21 '13 21:03 niemyjski

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.

rviscomi avatar Mar 22 '13 17:03 rviscomi

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.

niemyjski avatar Mar 27 '13 16:03 niemyjski

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.

rviscomi avatar Mar 27 '13 19:03 rviscomi

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.

niemyjski avatar Apr 08 '13 17:04 niemyjski

@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]

rviscomi avatar Apr 08 '13 20:04 rviscomi