Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Calculate width of cell using outerWidth() #157

Closed
wants to merge 1 commit into from

2 participants

@am0d

Using .width() was giving incorrect results for me on specific columns
(e.g. columns with no title and a set width). This corrects the issue.

@am0d am0d Calculate width of cell using outerWidth()
Using .width() was giving incorrect results for me on specific columns
(e.g. columns with no title and a set width). This corrects the issue.
c490659
@DataTables
Owner

Really sorry I didn't ask for this at the time, but can you link me to a test case showing a table which has this problem? I've not noticed using width rather than outerWidth being incorrect for me. Indeed, given that it is the inner width that the property applies to, I would have expected width() to be correct.

Having said that, I have noticed that Safari 5.x and Chrome around that same time does something very odd with width on columns. Safari 6 and newer Chrome don't appear to have that issue.

@am0d

Okay, I have managed to repro it. See here: http://jsfiddle.net/gccZ6/2/

The key things seem to be: I have box-sizing: border-box, and I have the padding-right: 12px on the table header cells to make room for the image on the right. (the red borders are just to show where the edges of the columns are).
Using the current version of DataTables, this produces incorrect results. Making the change I made above, this appears to give the correct results.

I repro'd this on Chrome Beta and Firefox Beta (2013-06-19).

@DataTables
Owner

Fantastic - thanks for this! let me spend a little bit of time looking at this in a bit more details and I'll get back to you.

@DataTables DataTables referenced this pull request from a commit
@DataTables New: Support for `box-sizing: border-box;`
- Setting `box-sizing: border-box;` for table cells would cause
  DataTables to incorrectly calculate the size of the element when
  applying the scrolling draw. This is because jQuery's $().width()
  always returns the content width (taking into account box-sizing).

- One possible fix was to detect the box model used and switch between
  width() and outerWidth(), but a much better fix is to use
  $().css('width') as this does take into account the box-model and
  allows DataTables to draw the scrolling table columns correctly,
  regardless of the box model. It should actually also improve
  performance, since jQuery doesn't need to look the box model up
  itself.

- This fixes issue #157
95be456
@DataTables
Owner

I've just had a bit of a look at this and while outerWidth fixes it for a border box, it breaks it for a content box.

It looks like it is better to use $().css('width') which always returns the size DataTables needs, regardless of the box model used. It should also give a nice little performance increase since jQuery doesn't need to look up the box model itself.

Fixed in 95be456.

Thanks for flagging this up.

@DataTables DataTables closed this
@am0d

Thanks Allan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 23, 2013
  1. @am0d

    Calculate width of cell using outerWidth()

    am0d authored
    Using .width() was giving incorrect results for me on specific columns
    (e.g. columns with no title and a set width). This corrects the issue.
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 1 deletion.
  1. +1 −1  media/src/core/core.scrolling.js
View
2  media/src/core/core.scrolling.js
@@ -318,7 +318,7 @@ function _fnScrollDraw ( o )
// Read all widths in next pass. Forces layout only once because we do not change
// any DOM properties.
_fnApplyToChildren( function(nSizer) {
- aApplied.push( _fnStringToCss( $(nSizer).width() ) );
+ aApplied.push( _fnStringToCss( $(nSizer).outerWidth() ) );
}, anHeadSizers );
// Apply all widths in final pass. Invalidates layout only once because we do not
Something went wrong with that request. Please try again.