Repository: ProcessMaker/modeler (js)

Issue:
The yellow box around the element should be around the element, not including the label
For elements that have a label outside of the shape (e.g. events and gateways), when selecting them on the grid, the highlight outline encloses the entire shape and the label:
<img width="145" alt="Screen Shot 2019-09-30 at 10 13 39 AM" src="https://user-images.githubusercontent.com/7561061/65887037-5e067c80-e36b-11e9-8d1e-0da5ec5336c8.png">
<img width="203" alt="Screen Shot 2019-09-30 at 10 13 35 AM" src="https://user-images.githubusercontent.com/7561061/65887038-5e067c80-e36b-11e9-9ecf-de85c7b6409b.png">

The outline should only be around the shape and _not_ the label:
<img width="160" alt="Screen Shot 2019-09-30 at 10 15 02 AM" src="https://user-images.githubusercontent.com/7561061/65887065-6bbc0200-e36b-11e9-90b2-ab215f2b3320.png">
<img width="193" alt="Screen Shot 2019-09-30 at 10 14 48 AM" src="https://user-images.githubusercontent.com/7561061/65887066-6bbc0200-e36b-11e9-8166-3734c9a40ace.png">

Pull request:
Highlight shape rather than shape and label
Closes #705

**Estimated appearance:** Using shape instead of a box.
![Screen Shot 2019-10-01 at 4 58 58 PM](https://user-images.githubusercontent.com/6653340/66000135-cbe69d00-e46c-11e9-8f30-10e46b2c41b2.png)

Custom highlight should only show when you click on an element and should update when resizing a pool.

**Note:** I did not alter the error highlighting box. It would revert the select highlight to the shape and label on some elements like gateways.

**Current error highlighting:**
![Screen Shot 2019-10-03 at 10 18 42 AM](https://user-images.githubusercontent.com/6653340/66135096-63f89980-e5c7-11e9-9bb4-8b38c3e4dfb7.png)


Patch:
diff --git a/src/components/Modeler.vue b/src/components/Modeler.vue
index 248b6257..92c0873f 100644
--- a/src/components/Modeler.vue
+++ b/src/components/Modeler.vue
@@ -647,12 +647,9 @@ export default {
       // Now, let's modify planeElement
       const diagram = this.nodeRegistry[control.type].diagram(this.moddle);
 
-      // Handle transform
-      const paperOrigin = this.paperManager.localToPagePoint(0, 0);
-      const scale = this.paperManager.scale;
-
-      diagram.bounds.x = (clientX - paperOrigin.x) / scale.sx;
-      diagram.bounds.y = (clientY - paperOrigin.y) / scale.sy;
+      const { x, y } = this.paperManager.clientToGridPoint(clientX, clientY);
+      diagram.bounds.x = x;
+      diagram.bounds.y = y;
 
       if (this.isBoundaryEvent(definition)) {
         this.setShapeCenterUnderCursor(diagram);
diff --git a/src/components/nodes/startEvent/index.js b/src/components/nodes/startEvent/index.js
index 0634a574..10c647be 100644
--- a/src/components/nodes/startEvent/index.js
+++ b/src/components/nodes/startEvent/index.js
@@ -1,6 +1,7 @@
 import component from './startEvent.vue';
 import idConfigSettings from '@/components/inspectors/idConfigSettings';
 import nameConfigSettings from '@/components/inspectors/nameConfigSettings';
+import { startEventDiameter } from './startEventConfig';
 
 export default {
   id: 'processmaker-modeler-start-event',
@@ -18,8 +19,8 @@ export default {
   diagram(moddle) {
     return moddle.create('bpmndi:BPMNShape', {
       bounds: moddle.create('dc:Bounds', {
-        height: 36,
-        width: 36,
+        height: startEventDiameter,
+        width: startEventDiameter,
         x: null,
         y: null,
       }),
diff --git a/src/components/nodes/startEvent/startEventConfig.js b/src/components/nodes/startEvent/startEventConfig.js
new file mode 100644
index 00000000..e0862002
--- /dev/null
+++ b/src/components/nodes/startEvent/startEventConfig.js
@@ -0,0 +1,1 @@
+export const startEventDiameter = 36;
diff --git a/src/components/nodes/task/index.js b/src/components/nodes/task/index.js
index fcb8e53a..4ceddc28 100644
--- a/src/components/nodes/task/index.js
+++ b/src/components/nodes/task/index.js
@@ -1,8 +1,8 @@
 import component from './task.vue';
 import idConfigSettings from '@/components/inspectors/idConfigSettings';
 import nameConfigSettings from '@/components/inspectors/nameConfigSettings';
+import { taskHeight, taskWidth } from './taskConfig';
 
-export const taskHeight = 76;
 export const id = 'processmaker-modeler-task';
 
 export default {
@@ -22,7 +22,7 @@ export default {
     return moddle.create('bpmndi:BPMNShape', {
       bounds: moddle.create('dc:Bounds', {
         height: taskHeight,
-        width: 116,
+        width: taskWidth,
       }),
     });
   },
diff --git a/src/components/nodes/task/taskConfig.js b/src/components/nodes/task/taskConfig.js
new file mode 100644
index 00000000..24515105
--- /dev/null
+++ b/src/components/nodes/task/taskConfig.js
@@ -0,0 +1,2 @@
+export const taskHeight = 76;
+export const taskWidth = 116;
diff --git a/src/components/paperManager.js b/src/components/paperManager.js
index 31142fdf..606231a8 100644
--- a/src/components/paperManager.js
+++ b/src/components/paperManager.js
@@ -1,20 +1,22 @@
 import { dia } from 'jointjs';
 import { highlightPadding } from '@/mixins/crownConfig';
 
-export default class {
+export default class PaperManager {
   #paper;
 
   constructor(paper) {
     this.#paper = paper;
   }
 
+  static gridSize = 10;
+
   static factory(element, interactiveFunc, model) {
     const paper = new dia.Paper({
       async: true,
       el: element,
       model,
       sorting: 'sorting-approximate',
-      gridSize: 10,
+      gridSize: PaperManager.gridSize,
       drawGrid: true,
       clickThreshold: 10,
       perpendicularLinks: true,
@@ -41,6 +43,10 @@ export default class {
     this.#paper.scale(scale);
   }
 
+  roundToNearestGridMultiple(number) {
+    return Math.round(number / PaperManager.gridSize) * PaperManager.gridSize;
+  }
+
   translate(x, y) {
     this.#paper.translate(x, y);
   }
@@ -57,10 +63,6 @@ export default class {
     this.#paper.off(eventName, callback, callbackScope);
   }
 
-  localToPagePoint(x, y) {
-    return this.#paper.localToPagePoint(x, y);
-  }
-
   setPaperDimensions(width, height) {
     this.#paper.setDimensions(width, height);
   }
@@ -70,4 +72,14 @@ export default class {
     await callback(this.#paper);
     this.#paper.unfreeze();
   }
+
+  clientToGridPoint(clientX, clientY) {
+    const paperOrigin = this.#paper.localToPagePoint(0, 0);
+    const scale = this.scale;
+
+    return {
+      x: this.roundToNearestGridMultiple((clientX - paperOrigin.x) / scale.sx),
+      y: this.roundToNearestGridMultiple((clientY - paperOrigin.y) / scale.sy),
+    };
+  }
 }
diff --git a/src/mixins/crownConfig.js b/src/mixins/crownConfig.js
index 6abaefb0..bf44899f 100644
--- a/src/mixins/crownConfig.js
+++ b/src/mixins/crownConfig.js
@@ -26,6 +26,16 @@ const errorHighlighter = {
   },
 };
 
+const defaultHighlighter = {
+  name: 'stroke',
+  options: {
+    attrs: {
+      stroke: '#feb663',
+      'stroke-width': 3,
+    },
+  },
+};
+
 export default {
   props: [
     'highlighted',
@@ -50,12 +60,12 @@ export default {
   watch: {
     highlighted(highlighted) {
       if (highlighted) {
-        this.shapeView.highlight();
+        this.shapeView.highlight(this.shapeBody, { highlighter: defaultHighlighter });
         this.addCrown();
-      } else {
-        this.shapeView.unhighlight();
-        this.removeCrown();
+        return;
       }
+      this.shapeView.unhighlight(this.shapeBody, { highlighter: defaultHighlighter });
+      this.removeCrown();
     },
     hasError() {
       if (this.isRendering) {
@@ -69,6 +79,9 @@ export default {
     shapeView() {
       return this.shape.findView(this.paper);
     },
+    shapeBody() {
+      return this.shapeView.$el.find('[joint-selector=body]');
+    },
     isPool() {
       return this.node.type === 'processmaker-modeler-pool';
     },
@@ -236,8 +249,8 @@ export default {
       this.shape.on('change:size', () => {
         if (this.highlighted) {
           /* Ensure the highlight box expands to fit element */
-          shapeView.unhighlight();
-          shapeView.highlight();
+          shapeView.unhighlight(this.shapeBody, { highlighter: defaultHighlighter });
+          shapeView.highlight(this.shapeBody, { highlighter: defaultHighlighter });
         }
       });
     },


Test patch:
diff --git a/tests/e2e/specs/BoundaryEventCommonBehaviour.spec.js b/tests/e2e/specs/BoundaryEventCommonBehaviour.spec.js
index 04ca7738..62cb1867 100644
--- a/tests/e2e/specs/BoundaryEventCommonBehaviour.spec.js
+++ b/tests/e2e/specs/BoundaryEventCommonBehaviour.spec.js
@@ -157,6 +157,8 @@ boundaryEventData.forEach(({ type, nodeType, eventXMLSnippet }) => {
 
         waitToRenderAllShapes();
 
+        getElementAtPosition(taskPosition).click();
+
         cy.wrap($boundaryEvent).should($el => {
           const { left, top } = $el.position();
           const positionErrorMargin = 2;
diff --git a/tests/e2e/specs/BoundaryTimerEvent.spec.js b/tests/e2e/specs/BoundaryTimerEvent.spec.js
index 1b630d42..95c07de5 100644
--- a/tests/e2e/specs/BoundaryTimerEvent.spec.js
+++ b/tests/e2e/specs/BoundaryTimerEvent.spec.js
@@ -4,7 +4,6 @@ import {
   getElementAtPosition,
   getGraphElements,
   getPositionInPaperCoords,
-  moveElement,
   removeIndentationAndLinebreaks,
   typeIntoTextInput,
   uploadXml,
@@ -170,6 +169,8 @@ describe('Boundary Timer Event', () => {
           .then($el => $el.find('[joint-selector="body"]'))
           .should('have.attr', 'fill', defaultNodeColor);
 
+        getElementAtPosition(startEventPosition).click();
+
         cy.wrap($boundaryEvent).should($el => {
           const { left, top } = $el.position();
           const positionErrorMargin = 2;
@@ -226,8 +227,6 @@ describe('Boundary Timer Event', () => {
                 expect(xml).to.contain(boundaryEventOnTask2Xml);
               });
 
-            const initialBoundaryEventPosition = $boundaryEvent.position();
-
             getElementAtPosition(taskPosition)
               .then(getComponentsEmbeddedInShape)
               .should($elements => {
@@ -239,24 +238,6 @@ describe('Boundary Timer Event', () => {
               .should($elements => {
                 expect($elements).to.have.lengthOf(numberOfBoundaryTimerEventsAdded);
               });
-
-            moveElement(taskPosition, 200, 400);
-            waitToRenderAllShapes();
-            cy.wrap($boundaryEvent).should($el => {
-              const { left, top } = $el.position();
-              const positionErrorMargin = 2;
-              expect(left).to.be.closeTo(initialBoundaryEventPosition.left, positionErrorMargin);
-              expect(top).to.be.closeTo(initialBoundaryEventPosition.top, positionErrorMargin);
-            });
-
-            moveElement(task2Position, 300, 500);
-            waitToRenderAllShapes();
-            cy.wrap($boundaryEvent).should($el => {
-              const { left, top } = $el.position();
-              const positionErrorMargin = 2;
-              expect(left).to.not.be.closeTo(initialBoundaryEventPosition.left, positionErrorMargin);
-              expect(top).to.not.be.closeTo(initialBoundaryEventPosition.top, positionErrorMargin);
-            });
           });
       });
     });
diff --git a/tests/e2e/specs/SequenceFlows.spec.js b/tests/e2e/specs/SequenceFlows.spec.js
index 7da4f512..028cc111 100644
--- a/tests/e2e/specs/SequenceFlows.spec.js
+++ b/tests/e2e/specs/SequenceFlows.spec.js
@@ -8,6 +8,8 @@ import {
   waitToRenderAllShapes,
 } from '../support/utils';
 import { nodeTypes } from '../support/constants';
+import { taskWidth } from '../../../src/components/nodes/task/taskConfig';
+import { startEventDiameter } from '../../../src/components/nodes/startEvent/startEventConfig';
 
 describe('Sequence Flows', () => {
   it('Can connect two elements', () => {
@@ -216,4 +218,19 @@ describe('Sequence Flows', () => {
       });
     });
   });
+
+  it('connects sequence flows with a stright line', function() {
+    const taskPosition = { x: 250, y: 250 };
+    dragFromSourceToDest(nodeTypes.task, taskPosition);
+
+    const endEventPosition = {
+      x: taskPosition.x + (taskWidth / 2) - (startEventDiameter / 2) + 1,
+      y: taskPosition.y + 200,
+    };
+    dragFromSourceToDest(nodeTypes.endEvent, endEventPosition);
+
+    connectNodesWithFlow('sequence-flow-button', taskPosition, endEventPosition);
+
+    cy.get('.main-paper [data-type="standard.Link"] [joint-selector="line"]').should('have.attr', 'd', 'M 308 326 L 308 450');
+  });
 });
diff --git a/tests/unit/paperManager.spec.js b/tests/unit/paperManager.spec.js
new file mode 100644
index 00000000..798daeea
--- /dev/null
+++ b/tests/unit/paperManager.spec.js
@@ -0,0 +1,31 @@
+import PaperManager from '@/components/paperManager';
+
+describe('Paper Manager', () => {
+  const positionData = [
+    [{ sx: 1, sy: 1 }, { x: 319, y: 77 }, 711, 320, { x: 390, y: 240 }],
+    [{ sx: 1, sy: 1 }, { x: 319, y: 77 }, 953, 486, { x: 630, y: 410 }],
+    [{ sx: 1, sy: 1 }, { x: 421, y: 289 }, 597, 258, { x: 180, y: -30 }],
+    [{ sx: 0.5, sy: 0.5 }, { x: 421, y: 289 }, 828, 166, { x: 810, y: -250 }],
+    [{ sx: 0.7, sy: 0.7 }, { x: 421, y: 289 }, 893, 246, { x: 670, y: -60 }],
+    [{ sx: 1.5, sy: 1.5 }, { x: 421, y: 289 }, 469, 385, { x: 30, y: 60 }],
+  ];
+
+  let paperManager;
+  let paper;
+
+  beforeEach(() => {
+    paper = {};
+    paperManager = new PaperManager(paper);
+  });
+
+  it.each(positionData)(
+    'Can convert client position to grid position',
+    (scale, paperOrigin, clientX, clientY, expectedPoint) => {
+      paper.scale = () => scale;
+      paper.localToPagePoint = () => paperOrigin;
+
+      const actualPoint = paperManager.clientToGridPoint(clientX, clientY);
+      expect(actualPoint).toEqual(expectedPoint);
+    });
+});
+