Properly handle gap values in grid and flex layouts

addon/src/modifiers/sortable-group.js

@@ -16,6 +16,7 @@ import { defaultA11yAnnouncementConfig } from '../utils/defaults';
 import { next, schedule, scheduleOnce, later } from '@ember/runloop';
 import { inject as service } from '@ember/service';
 import { registerDestructor, isDestroyed } from '@ember/destroyable';
+import { getGap } from '../utils/css-calculation';
 
 const NO_MODEL = {};
 
@@ -652,6 +653,8 @@ export default class SortableGroupModifier extends Modifier {
       axis = this.firstItemPosition;
     }
 
+    const gap = getGap(this.element);
+
     let direction = this.direction;
 
     let position = 0;
@@ -689,23 +692,24 @@ export default class SortableGroupModifier extends Modifier {
         position += item.spacing * 2;
       }
 
-      let dimension;
+      let dimension, gapSize;
 
       if (direction === 'grid') {
         dimension = 'width';
+        gapSize = gap['horizontal'];
 
         if (item.height > maxPrevHeight) {
-          maxPrevHeight = item.height;
+          maxPrevHeight = item.height + gap['vertical'];
         }
-      }
-      if (direction === 'x') {
+      } else if (direction === 'x') {
+        gapSize = gap['horizontal'];
         dimension = 'width';
-      }
-      if (direction === 'y') {
+      } else if (direction === 'y') {
+        gapSize = gap['vertical'];
         dimension = 'height';
       }
 
-      position += item[dimension];
+      position += item[dimension] + gapSize;
     });
   }
 

addon/src/modifiers/sortable-item.js

@@ -894,7 +894,7 @@ export default class SortableItemModifier extends Modifier {
     let width = el.offsetWidth;
     let elStyles = getComputedStyle(el);
 
-    width += parseInt(elStyles.marginLeft) + parseInt(elStyles.marginRight); // equal to jQuery.outerWidth(true)
+    width += parseFloat(elStyles.marginLeft) + parseFloat(elStyles.marginRight);
 
     width += getBorderSpacing(el).horizontal;
 
@@ -910,15 +910,17 @@ export default class SortableItemModifier extends Modifier {
     let el = this.element;
     let height = el.offsetHeight;
     let elStyles = getComputedStyle(el);
+    let parentDisplay = getComputedStyle(el.parentElement).display;
 
-    // This is needed atm only for grid, to fix jumping on drag-start.
-    // In test-app it looks like there is a side-effect when we activate also for direction vertical.
-    // If any user will anytime report a jumping in vertical direction, we should activate for every direction and fix our test-app
-    if (this.direction === 'grid') {
-      height += parseFloat(elStyles.marginTop);
-    }
+    let marginTop = parseFloat(elStyles.marginTop);
+    let marginBottom = parseFloat(elStyles.marginBottom);
 
-    height += parseFloat(elStyles.marginBottom);
+    // Account for collapsing margins in CSS flow layout
+    if (parentDisplay == 'block') {
+      height += Math.max(marginTop, marginBottom);
+    } else {
+      height += marginTop + marginBottom;
+    }
 
     height += getBorderSpacing(el).vertical;
 

addon/src/utils/css-calculation.js

@@ -15,3 +15,31 @@ export function getBorderSpacing(el) {
     vertical: parseFloat(vertical),
   };
 }
+
+/**
+  Gets numeric gap values for a given element
+
+  @method getGap
+  @param {Element} element
+  @return {Object}
+  @private
+*/
+export function getGap(el) {
+  let { columnGap, rowGap } = getComputedStyle(el);
+
+  columnGap = parseFloat(columnGap); // '0px' or 'normal'
+  rowGap = parseFloat(rowGap); // '0px' or 'normal'
+
+  if (isNaN(columnGap)) {
+    columnGap = 0;
+  }
+
+  if (isNaN(rowGap)) {
+    rowGap = 0;
+  }
+
+  return {
+    horizontal: columnGap,
+    vertical: rowGap,
+  };
+}