Fix line tool

src/containers/line-mode.jsx

@@ -4,11 +4,12 @@ import React from 'react';
 import {connect} from 'react-redux';
 import bindAll from 'lodash.bindall';
 import Modes from '../modes/modes';
-import {clearSelection, getSelectedLeafItems} from '../helper/selection';
+import {clearSelection} from '../helper/selection';
+import {endPointHit, touching} from '../helper/snapping';
+import {drawHitPoint, removeHitPoint} from '../helper/guides';
 import {MIXED} from '../helper/style-path';
 import {changeMode} from '../reducers/modes';
-import {changeStrokeWidth} from '../reducers/stroke-width';
+import {clearSelectedItems} from '../reducers/selected-items';
-import {clearSelectedItems, setSelectedItems} from '../reducers/selected-items';
 
 import LineModeComponent from '../components/line-mode.jsx';
 
@@ -21,13 +22,11 @@ class LineMode extends React.Component {
         bindAll(this, [
             'activateTool',
             'deactivateTool',
+            'drawHitPoint',
             'onMouseDown',
             'onMouseMove',
             'onMouseDrag',
-            'onMouseUp',
+            'onMouseUp'
-            'toleranceSquared',
-            'findLineEnd',
-            'onScroll'
         ]);
     }
     componentDidMount () {
@@ -47,7 +46,6 @@ class LineMode extends React.Component {
     }
     activateTool () {
         clearSelection(this.props.clearSelectedItems);
-        this.props.canvas.addEventListener('mousewheel', this.onScroll);
         this.tool = new paper.Tool();
         
         this.path = null;
@@ -73,20 +71,16 @@ class LineMode extends React.Component {
         this.tool.activate();
     }
     onMouseDown (event) {
-        // Deselect old path
+        if (event.event.button > 0) return; // only first mouse button
-        if (this.path) {
-            this.path.setSelected(false);
-            this.path = null;
-        }
 
         // If you click near a point, continue that line instead of making a new line
-        this.hitResult = this.findLineEnd(event.point);
+        this.hitResult = endPointHit(event.point, LineMode.SNAP_TOLERANCE);
         if (this.hitResult) {
             this.path = this.hitResult.path;
+            this.stylePath(this.path);
             if (this.hitResult.isFirst) {
                 this.path.reverse();
             }
-            this.path.lastSegment.setSelected(true);
             this.path.add(this.hitResult.segment); // Add second point, which is what will move when dragged
             this.path.lastSegment.handleOut = null; // Make sure line isn't curvy
             this.path.lastSegment.handleIn = null;
@@ -95,99 +89,89 @@ class LineMode extends React.Component {
         // If not near other path, start a new path
         if (!this.path) {
             this.path = new paper.Path();
+            this.stylePath(this.path);
 
-            this.path.setStrokeColor(
-                this.props.colorState.strokeColor === MIXED ? 'black' : this.props.colorState.strokeColor);
-            // Make sure a visible line is drawn
-            this.path.setStrokeWidth(
-                this.props.colorState.strokeWidth === null || this.props.colorState.strokeWidth === 0 ?
-                    1 : this.props.colorState.strokeWidth);
-
-            this.path.setSelected(true);
             this.path.add(event.point);
             this.path.add(event.point); // Add second point, which is what will move when dragged
             paper.view.draw();
         }
     }
+    stylePath (path) {
+        // Make sure a visible line is drawn
+        path.setStrokeColor(
+            (this.props.colorState.strokeColor === MIXED || this.props.colorState.strokeColor === null) ?
+                'black' : this.props.colorState.strokeColor);
+        path.setStrokeWidth(
+            this.props.colorState.strokeWidth === null || this.props.colorState.strokeWidth === 0 ?
+                1 : this.props.colorState.strokeWidth);
+    }
+    drawHitPoint (hitResult) {
+        // If near another path's endpoint, draw hit point to indicate that paths would merge
+        if (hitResult) {
+            const hitPath = hitResult.path;
+            if (hitResult.isFirst) {
+                drawHitPoint(hitPath.firstSegment.point);
+            } else {
+                drawHitPoint(hitPath.lastSegment.point);
+            }
+        }
+    }
     onMouseMove (event) {
+        removeHitPoint();
+        this.hitResult = endPointHit(event.point, LineMode.SNAP_TOLERANCE);
+        this.drawHitPoint(this.hitResult);
+    }
+    onMouseDrag (event) {
+        if (event.event.button > 0) return; // only first mouse button
+
         // If near another path's endpoint, or this path's beginpoint, clip to it to suggest
         // joining/closing the paths.
-        if (this.hitResult) {
+        removeHitPoint();
-            this.hitResult.path.setSelected(false);
         this.hitResult = null;
-        }
 
         if (this.path &&
                 !this.path.closed &&
-                this.path.firstSegment.point.getDistance(event.point, true) < this.toleranceSquared()) {
-            this.hitResult = {
-                path: this.path,
-                segment: this.path.firstSegment,
-                isFirst: true
-            };
-        } else {
-            this.hitResult = this.findLineEnd(event.point);
-        }
-
-        if (this.hitResult) {
-            const hitPath = this.hitResult.path;
-            hitPath.setSelected(true);
-            if (this.hitResult.isFirst) {
-                hitPath.firstSegment.setSelected(true);
-            } else {
-                hitPath.lastSegment.setSelected(true);
-            }
-        }
-    }
-    onMouseDrag (event) {
-        // If near another path's endpoint, or this path's beginpoint, clip to it to suggest
-        // joining/closing the paths.
-        if (this.hitResult && this.hitResult.path !== this.path) this.hitResult.path.setSelected(false);
-        this.hitResult = null;
-
-        if (this.path &&
                 this.path.segments.length > 3 &&
-                this.path.firstSegment.point.getDistance(event.point, true) < this.toleranceSquared()) {
+                touching(this.path.firstSegment.point, event.point, LineMode.SNAP_TOLERANCE)) {
             this.hitResult = {
                 path: this.path,
                 segment: this.path.firstSegment,
                 isFirst: true
             };
         } else {
-            this.hitResult = this.findLineEnd(event.point, this.path);
+            this.hitResult = endPointHit(event.point, LineMode.SNAP_TOLERANCE, this.path);
-            if (this.hitResult) {
-                const hitPath = this.hitResult.path;
-                hitPath.setSelected(true);
-                if (this.hitResult.isFirst) {
-                    hitPath.firstSegment.setSelected(true);
-                } else {
-                    hitPath.lastSegment.setSelected(true);
-                }
-            }
         }
 
         // snapping
-        if (this.path) {
         if (this.hitResult) {
+            this.drawHitPoint(this.hitResult);
             this.path.lastSegment.point = this.hitResult.segment.point;
         } else {
             this.path.lastSegment.point = event.point;
         }
     }
-    }
     onMouseUp (event) {
+        debugger;
+        if (event.event.button > 0) return; // only first mouse button
+
+        removeHitPoint();
+
         // If I single clicked, don't do anything
         if (this.path.segments.length < 2 ||
                 (this.path.segments.length === 2 &&
-                    this.path.firstSegment.point.getDistance(event.point, true) < this.toleranceSquared())) {
+                touching(this.path.firstSegment.point, event.point, LineMode.SNAP_TOLERANCE))) {
             this.path.remove();
             this.path = null;
             // TODO don't erase the line if both ends are snapped to different points
             return;
         } else if (
-            this.path.lastSegment.point.getDistance(this.path.segments[this.path.segments.length - 2].point, true) <
+            // Single click on an existing path end point
-                this.toleranceSquared()) {
+            touching(
+                this.path.lastSegment.point,
+                this.path.segments[this.path.segments.length - 2].point,
+                LineMode.SNAP_TOLERANCE)) {
             this.path.removeSegment(this.path.segments.length - 1);
+            this.path = null;
             return;
         }
         
@@ -197,8 +181,8 @@ class LineMode extends React.Component {
             if (this.path.firstSegment === this.hitResult.segment) {
                 // close path
                 this.path.closed = true;
-                this.path.setSelected(false);
             } else {
+                debugger;
                 // joining two paths
                 if (!this.hitResult.isFirst) {
                     this.hitResult.path.reverse();
@@ -208,62 +192,21 @@ class LineMode extends React.Component {
             this.hitResult = null;
         }
         
-        this.props.setSelectedItems();
         if (this.path) {
             this.props.onUpdateSvg();
+            this.path = null;
         }
     }
-    toleranceSquared () {
-        return Math.pow(LineMode.SNAP_TOLERANCE / paper.view.zoom, 2);
-    }
-    findLineEnd (point, excludePath) {
-        const lines = paper.project.getItems({
-            class: paper.Path
-        });
-        // Prefer more recent lines
-        for (let i = lines.length - 1; i >= 0; i--) {
-            if (lines[i].closed) {
-                continue;
-            }
-            if (excludePath && lines[i] === excludePath) {
-                continue;
-            }
-            if (lines[i].firstSegment &&
-                    lines[i].firstSegment.point.getDistance(point, true) < this.toleranceSquared()) {
-                return {
-                    path: lines[i],
-                    segment: lines[i].firstSegment,
-                    isFirst: true
-                };
-            }
-            if (lines[i].lastSegment && lines[i].lastSegment.point.getDistance(point, true) < this.toleranceSquared()) {
-                return {
-                    path: lines[i],
-                    segment: lines[i].lastSegment,
-                    isFirst: false
-                };
-            }
-        }
-        return null;
-    }
     deactivateTool () {
         this.props.canvas.removeEventListener('mousewheel', this.onScroll);
         this.tool.remove();
         this.tool = null;
+        removeHitPoint();
         this.hitResult = null;
         if (this.path) {
-            this.path.setSelected(false);
             this.path = null;
         }
     }
-    onScroll (event) {
-        if (event.deltaY < 0) {
-            this.props.changeStrokeWidth(this.props.colorState.strokeWidth + 1);
-        } else if (event.deltaY > 0 && this.props.colorState.strokeWidth > 1) {
-            this.props.changeStrokeWidth(this.props.colorState.strokeWidth - 1);
-        }
-        return true;
-    }
     render () {
         return (
             <LineModeComponent onMouseDown={this.props.handleMouseDown} />
@@ -273,7 +216,6 @@ class LineMode extends React.Component {
 
 LineMode.propTypes = {
     canvas: PropTypes.instanceOf(Element).isRequired,
-    changeStrokeWidth: PropTypes.func.isRequired,
     clearSelectedItems: PropTypes.func.isRequired,
     colorState: PropTypes.shape({
         fillColor: PropTypes.string,
@@ -282,8 +224,7 @@ LineMode.propTypes = {
     }).isRequired,
     handleMouseDown: PropTypes.func.isRequired,
     isLineModeActive: PropTypes.bool.isRequired,
-    onUpdateSvg: PropTypes.func.isRequired,
+    onUpdateSvg: PropTypes.func.isRequired
-    setSelectedItems: PropTypes.func.isRequired
 };
 
 const mapStateToProps = state => ({
@@ -291,15 +232,9 @@ const mapStateToProps = state => ({
     isLineModeActive: state.scratchPaint.mode === Modes.LINE
 });
 const mapDispatchToProps = dispatch => ({
-    changeStrokeWidth: strokeWidth => {
-        dispatch(changeStrokeWidth(strokeWidth));
-    },
     clearSelectedItems: () => {
         dispatch(clearSelectedItems());
     },
-    setSelectedItems: () => {
-        dispatch(setSelectedItems(getSelectedLeafItems()));
-    },
     handleMouseDown: () => {
         dispatch(changeMode(Modes.LINE));
     }

src/helper/guides.js

@@ -58,12 +58,8 @@ const rectSelect = function (event, color) {
     return rect;
 };
 
-const getGuideColor = function (colorName) {
+const getGuideColor = function () {
-    if (colorName === 'blue') {
     return GUIDE_BLUE;
-    } else if (colorName === 'grey') {
-        return GUIDE_GREY;
-    }
 };
 
 const _removePaperItemsByDataTags = function (tags) {
@@ -96,12 +92,30 @@ const removeAllGuides = function () {
     _removePaperItemsByTags(['guide']);
 };
 
+const removeHitPoint = function () {
+    _removePaperItemsByDataTags(['isHitPoint']);
+};
+
+const drawHitPoint = function (point) {
+    removeHitPoint();
+    if (point) {
+        const hitPoint = paper.Path.Circle(point, 4 /* radius */);
+        hitPoint.strokeColor = GUIDE_BLUE;
+        hitPoint.fillColor = new paper.Color(1, 1, 1, 0.5);
+        hitPoint.parent = getGuideLayer();
+        hitPoint.data.isHitPoint = true;
+        hitPoint.data.isHelperItem = true;
+    }
+};
+
 export {
     hoverItem,
     hoverBounds,
     rectSelect,
     removeAllGuides,
     removeHelperItems,
+    drawHitPoint,
+    removeHitPoint,
     getGuideColor,
     setDefaultGuideStyle
 };

src/helper/selection-tools/bounding-box-tool.js

@@ -170,7 +170,7 @@ class BoundingBoxTool {
                     noSelect: true,
                     noHover: true
                 };
-                rotHandle.fillColor = getGuideColor('blue');
+                rotHandle.fillColor = getGuideColor();
                 rotHandle.parent = getGuideLayer();
                 this.boundsRotHandles[index] = rotHandle;
             }
@@ -186,7 +186,7 @@ class BoundingBoxTool {
                         noHover: true
                     },
                     size: [size / paper.view.zoom, size / paper.view.zoom],
-                    fillColor: getGuideColor('blue'),
+                    fillColor: getGuideColor(),
                     parent: getGuideLayer()
                 });
         }

src/helper/snapping.js

@@ -0,0 +1,58 @@
+import paper from '@scratch/paper';
+
+/**
+ * @param {paper.Point} point1 point 1
+ * @param {paper.Point} point2 point 2
+ * @param {number} tolerance Distance allowed between points that are "touching"
+ * @return {boolean} true if points are within the tolerance distance.
+ */
+const touching = function (point1, point2, tolerance) {
+    return point1.getDistance(point2, true) < Math.pow(tolerance / paper.view.zoom, 2);
+};
+
+/**
+ * @param {!paper.Point} point Point to check line endpoint hits against
+ * @param {!number} tolerance Distance within which it counts as a hit
+ * @param {?paper.Path} excludePath Path to exclude from hit test, if any. For instance, you
+ *     are drawing a line and don't want it to snap to its own start point.
+ * @return {object} data about the end point of an unclosed path, if any such point is within the
+ *     tolerance distance of the given point, or null if none exists.
+ */
+const endPointHit = function (point, tolerance, excludePath) {
+    const lines = paper.project.getItems({
+        class: paper.Path
+    });
+    // Prefer more recent lines
+    for (let i = lines.length - 1; i >= 0; i--) {
+        if (lines[i].closed) {
+            continue;
+        }
+        if (!(lines[i].parent instanceof paper.Layer)) {
+            // Don't connect to lines inside of groups
+            continue;
+        }
+        if (excludePath && lines[i] === excludePath) {
+            continue;
+        }
+        if (lines[i].firstSegment && touching(lines[i].firstSegment.point, point, tolerance)) {
+            return {
+                path: lines[i],
+                segment: lines[i].firstSegment,
+                isFirst: true
+            };
+        }
+        if (lines[i].lastSegment && touching(lines[i].lastSegment.point, point, tolerance)) {
+            return {
+                path: lines[i],
+                segment: lines[i].lastSegment,
+                isFirst: false
+            };
+        }
+    }
+    return null;
+};
+
+export {
+    endPointHit,
+    touching
+};