Height Issues For Certain Browsers #1418
Conversation
I added this in my own personal implementation to fix an issue where the pagerSavedHeightSpacer's height was being calculated incorrectly due to it only taking into account the height of tbody and the height of the trs. Another thing in a table that can add height is the User Agent Style Sheet of the browser adding a border-spacing property (which is usually 2px). This property has been accounted for in the following code. If that property does not exist, it will simply do the same calculation it always has.
|
Hi @DoctorWhite! Thanks for the PR! I really do appreciate the help 😸 The code doesn't appear to work... I tried testing it in this demo which is using rawgit to point to the code changes. |
| @@ -328,7 +328,11 @@ | |||
| if (p.fixedHeight && !p.isDisabled) { | |||
| h = $.data(table, 'pagerSavedHeight'); | |||
| if (h) { | |||
| d = h - $b.height(); | |||
| var bs = 0; | |||
| if($(table).css('border-spacing').split(" ").length>0){ | |||
There was a problem hiding this comment.
This if statement should always be true.... is there a particular browser where it isn't? When I use $('table').css('border-spacing') on a table without any css changes, I will see 2px 2px.
| d = h - $b.height(); | ||
| var bs = 0; | ||
| if($(table).css('border-spacing').split(" ").length>0){ | ||
| bs = $(table).css('border-spacing').split(" ")[0].replace(/[^-\d\.]/g, ''); |
There was a problem hiding this comment.
The second border-spacing value should be used here because it contains the vertical size (ref)
/* border-spacing: horizontal vertical */
border-spacing: 1cm 2em;
| if($(table).css('border-spacing').split(" ").length>0){ | ||
| bs = $(table).css('border-spacing').split(" ")[0].replace(/[^-\d\.]/g, ''); | ||
| } | ||
| d = h - $b.height()+(bs*p.size)-bs; |
There was a problem hiding this comment.
The demo doesn't appear to work for me... it is because of the -bs on the end?
|
Oh you're right about me needing to use the second value not the first. I'll make that change. The -bs is necessary, though it can be reduced to |
|
Also, I was being overly cautious with the if statement. While I'm not aware of a browser that exists that doesn't have that property defined for tables, it seems like a silly thing to let the entire plugin crash over. It also appears your demo is using the original version of the pager file. Not the one with the change implemented. I updated it for you. https://jsfiddle.net/Lr6p93xu/17/ |
|
Hehe, overly cautious is good too :D Oh, yeah, I forgot to point it to the branch... derp. Thanks again, it looks good for merging! |
I added this in my own personal implementation to fix an issue where the pagerSavedHeightSpacer's height was being calculated incorrectly due to it only taking into account the height of tbody and the height of the trs. Another thing in a table that can add height is the User Agent Style Sheet of the browser adding a border-spacing property (which is usually 2px). This property has been accounted for in the following code. If that property does not exist, it will simply do the same calculation it always has.