BUG: Vertical margins are not supported

[zaygraveyard]

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 :smile:

[NeXTs]

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

[zaygraveyard]

Shall I submit a PR?

[NeXTs]

Sure, if you have time, desire and already working solution. I'll definitely accept your PR and made additional changes by myself, if needed.

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

[zaygraveyard]

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;
    },

[NeXTs]

Thanks!

But now we should additionally take into account margin collapsing, cases when those collapsing are not applied, etc.. I'll investigate it at free time, approximately at these weekend

[zaygraveyard]

Happy to help :smile:

As for the margin collapsing, I had already investigated this problem before for react-sortable-hoc. And found that its a very hard problem to solve completely but if we consider that all items have the same margins then the result of margin collapsing will be a simple max operation (as in the snippet). For more info you can check-out the comments on the PR react-sortable-hoc#27.

Oh and I forgot to mention that I added the following rule to the clusterize.css:

.clusterize-extra-row{
  margin: 0;
}

But I'm not sure if its the best solution.

[NeXTs]

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 table row since margins don't affect them so that calculations should be made accordingly