dimensions getting some love

atlassian · Jun 20, 2018 · b990562 · b990562
1 parent a6fe9cb
commit b990562
Show file tree

Hide file tree

Showing 8 changed files with 58 additions and 74 deletions.
diff --git a/src/view/draggable-dimension-publisher/draggable-dimension-publisher.jsx b/src/view/draggable-dimension-publisher/draggable-dimension-publisher.jsx
@@ -13,7 +13,6 @@ import type {
   DraggableId,
   DroppableId,
   TypeId,
-  BoxSizing,
 } from '../../types';
 import type { DimensionMarshal } from '../../state/dimension-marshal/dimension-marshal-types';
 
@@ -106,18 +105,14 @@ export default class DraggableDimensionPublisher extends Component<Props> {
     const client: BoxModel = calculateBox(borderBox, computedStyles);
     const page: BoxModel = withScroll(client, windowScroll);
 
-    const boxSizing: BoxSizing = computedStyles.boxSizing === 'border-box' ? 'border-box' : 'content-box';
-
     const placeholder: Placeholder = {
       client,
       tagName: targetRef.tagName.toLowerCase(),
       display: computedStyles.display,
-      boxSizing,
     };
 
     const dimension: DraggableDimension = {
       descriptor,
-      boxSizing,
       placeholder,
       client,
       page,

diff --git a/src/view/draggable/draggable-types.js b/src/view/draggable/draggable-types.js
@@ -7,8 +7,6 @@ import type {
   DraggableDimension,
   ZIndex,
   State,
-  BoxSpacing,
-  BoxSizing,
 } from '../../types';
 import {
   lift,

diff --git a/src/view/placeholder/placeholder-types.js b/src/view/placeholder/placeholder-types.js
@@ -0,0 +1,15 @@
+// @flow
+
+export type PlaceholderStyle = {|
+  display: string,
+  boxSizing: 'border-box',
+  width: number,
+  height: number,
+  marginTop: number,
+  marginRight: number,
+  marginBottom: number,
+  marginLeft: number,
+  flexShrink: '0',
+  flexGrow: '0',
+  pointerEvents: 'none',
+|}
diff --git a/src/view/placeholder/placeholder.jsx b/src/view/placeholder/placeholder.jsx
@@ -1,6 +1,7 @@
 // @flow
 import React, { PureComponent } from 'react';
 import type { Placeholder as PlaceholderType } from '../../types';
+import type { PlaceholderStyle } from './placeholder-types';
 
 type Props = {|
   placeholder: PlaceholderType,
@@ -32,16 +33,25 @@ export default class Placeholder extends PureComponent<Props> {
     const placeholder: PlaceholderType = this.props.placeholder;
     const { client, display, tagName } = placeholder;
 
-    const style = {
+    // The goal of the placeholder is to take up the same amount of space
+    // as the original draggable
+    const style: PlaceholderStyle = {
       display,
+      // ## Recreating the box model
+      // We created the borderBox and then apply the margins directly
+      // this is to maintain any margin collapsing behaviour
+
+      // creating borderBox
       boxSizing: 'border-box',
       width: client.borderBox.width,
       height: client.borderBox.height,
+      // creating marginBox
       marginTop: client.margin.top,
       marginRight: client.margin.right,
       marginBottom: client.margin.bottom,
       marginLeft: client.margin.left,
 
+      // ## Avoiding collapsing
       // Avoiding the collapsing or growing of this element when pushed by flex child siblings.
       // We have already taken a snapshot the current dimensions we do not want this element
       // to recalculate its dimensions

diff --git a/test/unit/view/draggable-dimension-publisher.spec.js b/test/unit/view/draggable-dimension-publisher.spec.js
@@ -13,7 +13,7 @@ import type {
 import { withDimensionMarshal } from '../../utils/get-context-options';
 import forceUpdate from '../../utils/force-update';
 import { setViewport } from '../../utils/viewport';
-import { getMarshalStub } from '../../utils/get-dimension-marshal';
+import { getMarshalStub } from '../../utils/dimension-marshal';
 import { offsetByPosition } from '../../../src/state/spacing';
 import type {
   DraggableId,
@@ -219,10 +219,6 @@ describe('DraggableDimensionPublisher', () => {
         x: window.pageXOffset,
         y: window.pageYOffset,
       };
-      // const windowScroll: Position = {
-      //   x: 100,
-      //   y: 200,
-      // };
       const borderBox: Spacing = {
         top: 0,
         right: 100,

diff --git a/test/unit/view/droppable-dimension-publisher.spec.js b/test/unit/view/droppable-dimension-publisher.spec.js
@@ -14,7 +14,7 @@ import { negate } from '../../../src/state/position';
 import setWindowScroll from '../../utils/set-window-scroll';
 
 import forceUpdate from '../../utils/force-update';
-import { getMarshalStub } from '../../utils/get-dimension-marshal';
+import { getMarshalStub } from '../../utils/dimension-marshal';
 import { withDimensionMarshal } from '../../utils/get-context-options';
 import getWindowScroll from '../../../src/view/window/get-window-scroll';
 import { setViewport, resetViewport } from '../../utils/viewport';
@@ -390,7 +390,8 @@ describe('DraggableDimensionPublisher', () => {
           // pull the get dimension function out
           const callbacks: DroppableCallbacks = marshal.registerDroppable.mock.calls[0][1];
           // execute it to get the dimension
-          const result: DroppableDimension = callbacks.getDimensionAndWatchScroll(preset.windowScroll, immediate);
+          const result: DroppableDimension =
+            callbacks.getDimensionAndWatchScroll(preset.windowScroll, immediate);
 
           expect(result).toEqual(expected);
         });
@@ -444,7 +445,8 @@ describe('DraggableDimensionPublisher', () => {
           // pull the get dimension function out
           const callbacks: DroppableCallbacks = marshal.registerDroppable.mock.calls[0][1];
           // execute it to get the dimension
-          const result: DroppableDimension = callbacks.getDimensionAndWatchScroll(preset.windowScroll, immediate);
+          const result: DroppableDimension =
+            callbacks.getDimensionAndWatchScroll(preset.windowScroll, immediate);
 
           expect(result).toEqual(expected);
         });
@@ -501,7 +503,8 @@ describe('DraggableDimensionPublisher', () => {
           // pull the get dimension function out
           const callbacks: DroppableCallbacks = marshal.registerDroppable.mock.calls[0][1];
           // execute it to get the dimension
-          const result: DroppableDimension = callbacks.getDimensionAndWatchScroll(preset.windowScroll, immediate);
+          const result: DroppableDimension =
+            callbacks.getDimensionAndWatchScroll(preset.windowScroll, immediate);
 
           expect(result).toEqual(expected);
         });
@@ -544,7 +547,8 @@ describe('DraggableDimensionPublisher', () => {
           // pull the get dimension function out
           const callbacks: DroppableCallbacks = marshal.registerDroppable.mock.calls[0][1];
           // execute it to get the dimension
-          const result: DroppableDimension = callbacks.getDimensionAndWatchScroll(preset.windowScroll, immediate);
+          const result: DroppableDimension =
+            callbacks.getDimensionAndWatchScroll(preset.windowScroll, immediate);
 
           expect(result).toEqual(expected);
         });
@@ -595,7 +599,8 @@ describe('DraggableDimensionPublisher', () => {
           // pull the get dimension function out
           const callbacks: DroppableCallbacks = marshal.registerDroppable.mock.calls[0][1];
           // execute it to get the dimension
-          const result: DroppableDimension = callbacks.getDimensionAndWatchScroll(preset.windowScroll, immediate);
+          const result: DroppableDimension =
+            callbacks.getDimensionAndWatchScroll(preset.windowScroll, immediate);
 
           expect(result).toEqual(expected);
         });
@@ -641,7 +646,8 @@ describe('DraggableDimensionPublisher', () => {
           // pull the get dimension function out
         const callbacks: DroppableCallbacks = marshal.registerDroppable.mock.calls[0][1];
         // execute it to get the dimension
-        const result: DroppableDimension = callbacks.getDimensionAndWatchScroll(preset.windowScroll, immediate);
+        const result: DroppableDimension =
+          callbacks.getDimensionAndWatchScroll(preset.windowScroll, immediate);
 
         expect(result).toEqual(expected);
       });
@@ -683,7 +689,8 @@ describe('DraggableDimensionPublisher', () => {
           // pull the get dimension function out
         const callbacks: DroppableCallbacks = marshal.registerDroppable.mock.calls[0][1];
         // execute it to get the dimension
-        const result: DroppableDimension = callbacks.getDimensionAndWatchScroll(preset.windowScroll, immediate);
+        const result: DroppableDimension =
+          callbacks.getDimensionAndWatchScroll(preset.windowScroll, immediate);
 
         expect(result).toEqual(expected);
       });

diff --git a/test/unit/view/unconnected-draggable.spec.js b/test/unit/view/unconnected-draggable.spec.js
@@ -8,6 +8,7 @@ import DragHandle from '../../../src/view/drag-handle/drag-handle';
 import { sloppyClickThreshold } from '../../../src/view/drag-handle/util/is-sloppy-click-threshold-exceeded';
 import Moveable from '../../../src/view/moveable/';
 import Placeholder from '../../../src/view/placeholder';
+import type { PlaceholderStyle } from '../../../src/view/placeholder/placeholder-types';
 import { subtract } from '../../../src/state/position';
 import createStyleMarshal from '../../../src/view/style-marshal/style-marshal';
 import type { StyleMarshal } from '../../../src/view/style-marshal/style-marshal-types';
@@ -22,14 +23,13 @@ import type {
 } from '../../../src/view/draggable/draggable-types';
 import type {
   DraggableDimension,
-  DroppableDimension,
   DraggableId,
   DroppableId,
   TypeId,
   ItemPositions,
   Viewport,
 } from '../../../src/types';
-import { getDraggableDimension, getDroppableDimension, getPreset } from '../../utils/dimension';
+import { getPreset } from '../../utils/dimension';
 import { combine, withStore, withDroppableId, withStyleContext, withDimensionMarshal, withCanLift, withDroppableType } from '../../utils/get-context-options';
 import { dispatchWindowMouseEvent, mouseEvent } from '../../utils/user-input-util';
 import { setViewport, resetViewport } from '../../utils/viewport';
@@ -52,40 +52,12 @@ class Item extends Component<{ provided: Provided }> {
   }
 }
 
-const draggableId: DraggableId = 'draggable1';
-const droppableId: DroppableId = 'droppable1';
-const type: TypeId = 'ITEM';
-const origin: Position = { x: 0, y: 0 };
 const preset = getPreset();
-
-const droppable: DroppableDimension = getDroppableDimension({
-  descriptor: {
-    id: droppableId,
-    type,
-  },
-  borderBox: {
-    top: 0,
-    right: 100,
-    bottom: 200,
-    left: 0,
-  },
-});
-
-const dimension: DraggableDimension = getDraggableDimension({
-  descriptor: {
-    id: draggableId,
-    droppableId,
-    type,
-    index: 0,
-  },
-  borderBox: {
-    top: 0,
-    right: 100,
-    bottom: 100,
-    left: 0,
-  },
-  // TODO: margin
-});
+const dimension: DraggableDimension = preset.inHome1;
+const draggableId: DraggableId = dimension.descriptor.id;
+const droppableId: DroppableId = dimension.descriptor.droppableId;
+const type: TypeId = dimension.descriptor.type;
+const origin: Position = { x: 0, y: 0 };
 
 const getDispatchPropsStub = (): DispatchProps => ({
   lift: jest.fn(),
@@ -382,15 +354,6 @@ describe('Draggable - unconnected', () => {
 
     describe('handling events', () => {
       describe('onLift', () => {
-        let dispatchProps;
-
-        beforeEach(() => {
-          dispatchProps = getDispatchPropsStub();
-          managedWrapper = mountDraggable({
-            dispatchProps,
-          });
-        });
-
         it('should throw if lifted when dragging is not enabled', () => {
           const customWrapper = mountDraggable({
             ownProps: disabledOwnProps,
@@ -408,6 +371,11 @@ describe('Draggable - unconnected', () => {
         });
 
         it('should lift if permitted', () => {
+          const dispatchProps = getDispatchPropsStub();
+          const wrapper = mountDraggable({
+            dispatchProps,
+          });
+
           // made up values
           const selection: Position = {
             x: 100,
@@ -423,7 +391,7 @@ describe('Draggable - unconnected', () => {
             offset: origin,
           };
 
-          executeOnLift(managedWrapper)({ selection, borderBoxCenter, viewport: preset.viewport });
+          executeOnLift(wrapper)({ selection, borderBoxCenter, viewport: preset.viewport });
 
           // $ExpectError - mock property on lift function
           expect(dispatchProps.lift).toHaveBeenCalledWith({
@@ -801,7 +769,7 @@ describe('Draggable - unconnected', () => {
 
         const props: Object = child.props();
 
-        expect(props.style).toEqual({
+        const expected: PlaceholderStyle = {
           width: dimension.placeholder.client.borderBox.width,
           height: dimension.placeholder.client.borderBox.height,
           marginTop: dimension.placeholder.client.margin.top,
@@ -813,7 +781,8 @@ describe('Draggable - unconnected', () => {
           flexGrow: '0',
           boxSizing: 'border-box',
           pointerEvents: 'none',
-        });
+        };
+        expect(props.style).toEqual(expected);
         expect(child.type()).toBe(dimension.placeholder.tagName);
       });
 

diff --git a/test/utils/dimension.js b/test/utils/dimension.js
@@ -37,7 +37,6 @@ type GetComputedSpacingArgs = {|
   padding?: Spacing,
   border ?: Spacing,
   display ?: string,
-  boxSizing ?: string,
 |}
 
 const origin: Position = { x: 0, y: 0 };
@@ -47,7 +46,6 @@ export const getComputedSpacing = ({
   padding = noSpacing,
   border = noSpacing,
   display = 'block',
-  boxSizing = 'border-box',
 }: GetComputedSpacingArgs): Object => ({
   paddingTop: `${padding.top}px`,
   paddingRight: `${padding.right}px`,
@@ -62,7 +60,6 @@ export const getComputedSpacing = ({
   borderBottomWidth: `${border.bottom}px`,
   borderLeftWidth: `${border.left}px`,
   display,
-  boxSizing,
 });
 
 export const makeScrollable = (droppable: DroppableDimension, amount?: number = 20) => {
@@ -154,7 +151,6 @@ const getPlaceholder = (client: BoxModel): Placeholder => ({
   client,
   tagName: 'div',
   display: 'block',
-  boxSizing: 'border-box',
 });
 
 export const getClosestScrollable = (droppable: DroppableDimension): Scrollable => {
@@ -191,7 +187,6 @@ export const getDraggableDimension = ({
   const result: DraggableDimension = {
     descriptor,
     client,
-    boxSizing: 'border-box',
     page: withScroll(client, windowScroll),
     placeholder: getPlaceholder(client),
   };
@@ -591,7 +586,6 @@ export const shiftDraggables = ({
           ...dimension.descriptor,
           index: dimension.descriptor.index + indexChange,
         },
-        boxSizing: dimension.boxSizing,
         client,
         page,
         placeholder: {