-
Notifications
You must be signed in to change notification settings - Fork 414
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
BUG: Vertical margins are not supported #78
Comments
Hm, yes it makes sense Margins weren't taken into account because they doesn't work for table rows anyway, but this may be useful for div, ul/ol lists |
Shall I submit a PR? |
Sure, if you have time, desire and already working solution. or you could drop a snippet here with required changes and I'll do the rest I already have an idea where fix should be applied. Additional condition to check margins here using getStyle func |
Here's the snippet: getRowsHeight: function(rows) {
var opts = this.options,
prev_item_height = opts.item_height;
opts.cluster_height = 0
if( ! rows.length) return;
var nodes = this.content_elem.children;
var node = nodes[Math.floor(nodes.length / 2)];
opts.item_height = node.offsetHeight;
// consider table's border-spacing
if(opts.tag == 'tr' && getStyle('borderCollapse', this.content_elem) != 'collapse')
opts.item_height += parseInt(getStyle('borderSpacing', this.content_elem), 10) || 0;
// consider margins (and margins collapsing)
var marginTop = parseInt(getStyle('marginTop', node), 10);
var marginBottom = parseInt(getStyle('marginBottom', node), 10);
opts.item_height += Math.max(marginTop, marginBottom);
opts.block_height = opts.item_height * opts.rows_in_block;
opts.rows_in_cluster = opts.blocks_in_cluster * opts.rows_in_block;
opts.cluster_height = opts.blocks_in_cluster * opts.block_height;
return prev_item_height != opts.item_height;
}, |
Thanks! But now we should additionally take into account margin collapsing, cases when those collapsing are not applied, etc.. |
Happy to help 😄 As for the margin collapsing, I had already investigated this problem before for react-sortable-hoc. Oh and I forgot to mention that I added the following rule to the .clusterize-extra-row{
margin: 0;
} But I'm not sure if its the best solution. |
Thanks @zaygraveyard for your contribution! Your PR did all the necessary work, only thing that I needed to add was additional condition to check if it's not |
In the same spirit of react-sortable-hoc#26
Margins are not taken into account in the
item_height
calculations.I'll be happy to submit a PR if needed (I already have the fix)
PS: Thanks for the great library 😄
The text was updated successfully, but these errors were encountered: