draggables with margin-top or margin-left will now be positioned corr…

…ectly (#45)

README.md

@@ -655,17 +655,17 @@ It is a contract of this library that it owns the positioning logic of the dragg
 type DraggableStyle = DraggingStyle | NotDraggingStyle;
 
 type DraggingStyle = {|
-  position: 'fixed',
-  boxSizing: 'border-box',
-  // allow scrolling of the element behind the dragging element
   pointerEvents: 'none',
-  zIndex: ZIndex,
+  position: 'fixed',
   width: number,
   height: number,
+  boxSizing: 'border-box',
   top: number,
   left: number,
+  margin: 0,
   transform: ?string,
-|};
+  zIndex: ZIndex,
+|}
 
 type NotDraggingStyle = {|
   transition: ?string,

src/view/draggable/draggable-types.js

@@ -28,16 +28,40 @@ import type {
 } from '../drag-handle/drag-handle-types';
 
 export type DraggingStyle = {|
-  position: 'fixed',
-  boxSizing: 'border-box',
-  // allow scrolling of the element behind the dragging element
+  // Allow scrolling of the element behind the dragging element
   pointerEvents: 'none',
-  zIndex: ZIndex,
+
+  // `position: fixed` is used to ensure that the element is always positioned
+  // in the correct position and ignores the surrounding position:relative parents
+  position: 'fixed',
+
+  // When we do `position: fixed` the element looses its normal dimensions,
+  // especially if using flexbox. We set the width and height manually to
+  // ensure the element has the same dimensions as before it started dragging
   width: number,
   height: number,
+
+  // When we set the width and height they include the padding on the element.
+  // We use box-sizing: border-box to ensure that the width and height are inclusive of the padding
+  boxSizing: 'border-box',
+
+  // We initially position the element in the same visual spot as when it started.
+  // To do this we give the element the top / left position with the margins considered
   top: number,
   left: number,
+
+  // We clear any top or left margins on the element to ensure it does not push
+  // the element positioned with the top/left position.
+  // We also clear the margin right / bottom. This has no positioning impact,
+  // but it is cleanest to just remove all the margins rather than only the top and left.
+  margin: 0,
+
+  // Move the element in response to a user dragging
   transform: ?string,
+
+  // When dragging or dropping we control the z-index to ensure that
+  // the layering is correct
+  zIndex: ZIndex,
 |}
 
 export type NotDraggingStyle = {|

src/view/draggable/draggable.jsx

@@ -221,6 +221,7 @@ export default class Draggable extends Component {
       left: number,
       isDropAnimating: boolean,
       movementStyle: MovementStyle): DraggingStyle => {
+      // For an explanation of properties see `draggable-types`.
       const style: DraggingStyle = {
         position: 'fixed',
         boxSizing: 'border-box',
@@ -230,6 +231,7 @@ export default class Draggable extends Component {
         height,
         top,
         left,
+        margin: 0,
         transform: movementStyle.transform ? `${movementStyle.transform}` : null,
       };
       return style;
@@ -273,7 +275,9 @@ export default class Draggable extends Component {
         }
         invariant(dimension, 'draggable dimension required for dragging');
 
+        // Margins are accounted for. See `draggable-types` for explanation
         const { width, height, top, left } = dimension.client.withoutMargin;
+
         return this.getDraggingStyle(width, height, top, left, isDropAnimating, movementStyle);
       })();
 

test/unit/view/unconnected-draggable.spec.js

@@ -887,6 +887,7 @@ describe('Draggable - unconnected', () => {
         height: dimension.page.withMargin.height,
         top: dimension.page.withMargin.top,
         left: dimension.page.withMargin.left,
+        margin: 0,
         transform: null,
         pointerEvents: 'none',
       };
@@ -919,6 +920,7 @@ describe('Draggable - unconnected', () => {
         height: dimension.page.withMargin.height,
         top: dimension.page.withMargin.top,
         left: dimension.page.withMargin.left,
+        margin: 0,
         transform: `translate(${offset.x}px, ${offset.y}px)`,
         pointerEvents: 'none',
       };
@@ -955,6 +957,7 @@ describe('Draggable - unconnected', () => {
         height: dimension.page.withMargin.height,
         top: dimension.page.withMargin.top,
         left: dimension.page.withMargin.left,
+        margin: 0,
       };
 
       mountDraggable({
@@ -1083,6 +1086,7 @@ describe('Draggable - unconnected', () => {
         height: dimension.page.withMargin.height,
         top: dimension.page.withMargin.top,
         left: dimension.page.withMargin.left,
+        margin: 0,
         transform: `translate(${offset.x}px, ${offset.y}px)`,
         pointerEvents: 'none',
       };