Make stroke width 0 when transparent (#695)

src/containers/stroke-color-indicator.jsx

@@ -3,13 +3,14 @@ import PropTypes from 'prop-types';
 import React from 'react';
 import bindAll from 'lodash.bindall';
 import {changeStrokeColor} from '../reducers/stroke-color';
+import {changeStrokeWidth} from '../reducers/stroke-width';
 import {openStrokeColor, closeStrokeColor} from '../reducers/modals';
 import Modes from '../lib/modes';
 import Formats from '../lib/format';
 import {isBitmap} from '../lib/format';
 
 import StrokeColorIndicatorComponent from '../components/stroke-color-indicator.jsx';
-import {applyStrokeColorToSelection} from '../helper/style-path';
+import {applyStrokeColorToSelection, applyStrokeWidthToSelection} from '../helper/style-path';
 
 class StrokeColorIndicator extends React.Component {
     constructor (props) {
@@ -31,10 +32,17 @@ class StrokeColorIndicator extends React.Component {
         }
     }
     handleChangeStrokeColor (newColor) {
+        if (this.props.strokeColor === null && newColor !== null) {
+            this._hasChanged = applyStrokeWidthToSelection(1, this.props.textEditTarget) || this._hasChanged;
+            this.props.onChangeStrokeWidth(1);
+        } else if (this.props.strokeColor !== null && newColor === null) {
+            this._hasChanged = applyStrokeWidthToSelection(0, this.props.textEditTarget) || this._hasChanged;
+            this.props.onChangeStrokeWidth(0);
+        }
         // Apply color and update redux, but do not update svg until picker closes.
-        const isDifferent =
+        this._hasChanged =
-            applyStrokeColorToSelection(newColor, isBitmap(this.props.format), this.props.textEditTarget);
+            applyStrokeColorToSelection(newColor, isBitmap(this.props.format), this.props.textEditTarget) ||
-        this._hasChanged = this._hasChanged || isDifferent;
+            this._hasChanged;
         this.props.onChangeStrokeColor(newColor);
     }
     handleCloseStrokeColor () {
@@ -68,6 +76,9 @@ const mapDispatchToProps = dispatch => ({
     onChangeStrokeColor: strokeColor => {
         dispatch(changeStrokeColor(strokeColor));
     },
+    onChangeStrokeWidth: strokeWidth => {
+        dispatch(changeStrokeWidth(strokeWidth));
+    },
     onOpenStrokeColor: () => {
         dispatch(openStrokeColor());
     },
@@ -77,10 +88,10 @@ const mapDispatchToProps = dispatch => ({
 });
 
 StrokeColorIndicator.propTypes = {
-    disabled: PropTypes.bool.isRequired,
     format: PropTypes.oneOf(Object.keys(Formats)),
     isEyeDropping: PropTypes.bool.isRequired,
     onChangeStrokeColor: PropTypes.func.isRequired,
+    onChangeStrokeWidth: PropTypes.func.isRequired,
     onCloseStrokeColor: PropTypes.func.isRequired,
     onUpdateImage: PropTypes.func.isRequired,
     strokeColor: PropTypes.string,

src/containers/stroke-width-indicator.jsx

@@ -2,10 +2,16 @@ import {connect} from 'react-redux';
 import PropTypes from 'prop-types';
 import React from 'react';
 import bindAll from 'lodash.bindall';
+import parseColor from 'parse-color';
+import {changeStrokeColor} from '../reducers/stroke-color';
 import {changeStrokeWidth} from '../reducers/stroke-width';
 import StrokeWidthIndicatorComponent from '../components/stroke-width-indicator.jsx';
-import {applyStrokeWidthToSelection} from '../helper/style-path';
+import {getSelectedLeafItems} from '../helper/selection';
+import {applyStrokeColorToSelection, applyStrokeWidthToSelection, getColorsFromSelection, MIXED}
+    from '../helper/style-path';
 import Modes from '../lib/modes';
+import Formats from '../lib/format';
+import {isBitmap} from '../lib/format';
 
 class StrokeWidthIndicator extends React.Component {
     constructor (props) {
@@ -15,10 +21,20 @@ class StrokeWidthIndicator extends React.Component {
         ]);
     }
     handleChangeStrokeWidth (newWidth) {
-        if (applyStrokeWidthToSelection(newWidth, this.props.textEditTarget)) {
+        let changed = applyStrokeWidthToSelection(newWidth, this.props.textEditTarget);
-            this.props.onUpdateImage();
+        if ((!this.props.strokeWidth || this.props.strokeWidth === 0) && newWidth > 0) {
+            let currentColor = getColorsFromSelection(getSelectedLeafItems(), isBitmap(this.props.format)).strokeColor;
+            if (currentColor === null) {
+                changed = applyStrokeColorToSelection('#000', isBitmap(this.props.format), this.props.textEditTarget) ||
+                    changed;
+                currentColor = '#000';
+            } else if (currentColor !== MIXED) {
+                currentColor = parseColor(currentColor).hex;
+            }
+            this.props.onChangeStrokeColor(currentColor);
         }
         this.props.onChangeStrokeWidth(newWidth);
+        if (changed) this.props.onUpdateImage();
     }
     render () {
         return (
@@ -35,10 +51,14 @@ const mapStateToProps = state => ({
     disabled: state.scratchPaint.mode === Modes.BRUSH ||
         state.scratchPaint.mode === Modes.TEXT ||
         state.scratchPaint.mode === Modes.FILL,
+    format: state.scratchPaint.format,
     strokeWidth: state.scratchPaint.color.strokeWidth,
     textEditTarget: state.scratchPaint.textEditTarget
 });
 const mapDispatchToProps = dispatch => ({
+    onChangeStrokeColor: strokeColor => {
+        dispatch(changeStrokeColor(strokeColor));
+    },
     onChangeStrokeWidth: strokeWidth => {
         dispatch(changeStrokeWidth(strokeWidth));
     }
@@ -46,6 +66,8 @@ const mapDispatchToProps = dispatch => ({
 
 StrokeWidthIndicator.propTypes = {
     disabled: PropTypes.bool.isRequired,
+    format: PropTypes.oneOf(Object.keys(Formats)),
+    onChangeStrokeColor: PropTypes.func.isRequired,
     onChangeStrokeWidth: PropTypes.func.isRequired,
     onUpdateImage: PropTypes.func.isRequired,
     strokeWidth: PropTypes.number,

src/helper/style-path.js

@@ -1,6 +1,6 @@
 import paper from '@scratch/paper';
 import {getSelectedLeafItems} from './selection';
-import {isPGTextItem, isPointTextItem} from './item';
+import {isPointTextItem} from './item';
 import {isGroup} from './group';
 import {getItems} from './selection';
 import GradientTypes from '../lib/gradient-types';
@@ -111,7 +111,7 @@ const applyFillColorToSelection = function (colorString, colorIndex, isSolidGrad
         }
 
         // In bitmap mode, fill color applies to the stroke if there is a stroke
-        if (bitmapMode && item.strokeColor !== null && item.strokeWidth !== 0) {
+        if (bitmapMode && item.strokeColor !== null && item.strokeWidth) {
             if (!_colorMatch(item.strokeColor, colorString)) {
                 changed = true;
                 item.strokeColor = colorString;
@@ -306,32 +306,7 @@ const applyStrokeColorToSelection = function (colorString, bitmapMode, textEditT
         if (item.parent instanceof paper.CompoundPath) {
             item = item.parent;
         }
-        if (isPGTextItem(item)) {
+        if (!_colorMatch(item.strokeColor, colorString)) {
-            if (item.children) {
-                for (const child of item.children) {
-                    if (child.children) {
-                        for (const path of child.children) {
-                            if (!path.data.isPGGlyphRect) {
-                                if (!_colorMatch(path.strokeColor, colorString)) {
-                                    changed = true;
-                                    path.strokeColor = colorString;
-                                }
-                            }
-                        }
-                    } else if (!child.data.isPGGlyphRect) {
-                        if (child.strokeColor !== colorString) {
-                            changed = true;
-                            child.strokeColor = colorString;
-                        }
-                    }
-                }
-            } else if (!item.data.isPGGlyphRect) {
-                if (!_colorMatch(item.strokeColor, colorString)) {
-                    changed = true;
-                    item.strokeColor = colorString;
-                }
-            }
-        } else if (!_colorMatch(item.strokeColor, colorString)) {
             changed = true;
             item.strokeColor = colorString;
         }
@@ -428,10 +403,12 @@ const getColorsFromSelection = function (selectedItems, bitmapMode) {
                 } else if (item.strokeColor.type === 'gradient') {
                     itemStrokeColorString = MIXED;
                 } else {
-                    itemStrokeColorString = item.strokeColor.alpha === 0 ?
+                    itemStrokeColorString = item.strokeColor.alpha === 0 || !item.strokeWidth ?
                         null :
                         item.strokeColor.toCSS();
                 }
+            } else {
+                itemStrokeColorString = null;
             }
             // check every style against the first of the items
             if (firstChild) {
@@ -440,7 +417,7 @@ const getColorsFromSelection = function (selectedItems, bitmapMode) {
                 selectionFillColor2String = itemFillColor2String;
                 selectionStrokeColorString = itemStrokeColorString;
                 selectionGradientType = itemGradientType;
-                selectionStrokeWidth = item.strokeWidth;
+                selectionStrokeWidth = itemStrokeColorString ? item.strokeWidth : 0;
                 if (item.strokeWidth && item.data && item.data.zoomLevel) {
                     selectionThickness = item.strokeWidth / item.data.zoomLevel;
                 }
@@ -459,13 +436,14 @@ const getColorsFromSelection = function (selectedItems, bitmapMode) {
             if (itemStrokeColorString !== selectionStrokeColorString) {
                 selectionStrokeColorString = MIXED;
             }
-            if (selectionStrokeWidth !== item.strokeWidth) {
+            const itemStrokeWidth = itemStrokeColorString ? item.strokeWidth : 0;
+            if (selectionStrokeWidth !== itemStrokeWidth) {
                 selectionStrokeWidth = null;
             }
         }
     }
     // Convert selection gradient type from horizontal to vertical if first item is exactly vertical
-    if (selectionGradientType !== GradientTypes.SOLID) {
+    if (selectedItems && selectedItems.length && selectionGradientType !== GradientTypes.SOLID) {
         let firstItem = selectedItems[0];
         if (firstItem.parent instanceof paper.CompoundPath) firstItem = firstItem.parent;
         const direction = firstItem.fillColor.destination.subtract(firstItem.fillColor.origin);

src/reducers/stroke-color.js

@@ -1,6 +1,7 @@
 import log from '../log/log';
 import {CHANGE_SELECTED_ITEMS} from './selected-items';
-import {getColorsFromSelection} from '../helper/style-path';
+import {CHANGE_STROKE_WIDTH} from './stroke-width';
+import {getColorsFromSelection, MIXED} from '../helper/style-path';
 
 const CHANGE_STROKE_COLOR = 'scratch-paint/stroke-color/CHANGE_STROKE_COLOR';
 const initialState = '#000';
@@ -10,8 +11,13 @@ const regExp = /^#([0-9a-f]{3}){1,2}$/i;
 const reducer = function (state, action) {
     if (typeof state === 'undefined') state = initialState;
     switch (action.type) {
+    case CHANGE_STROKE_WIDTH:
+        if (Math.max(0, action.strokeWidth) === 0) {
+            return null;
+        }
+        return state;
     case CHANGE_STROKE_COLOR:
-        if (!regExp.test(action.strokeColor) && action.strokeColor !== null) {
+        if (!regExp.test(action.strokeColor) && action.strokeColor !== null && action.strokeColor !== MIXED) {
             log.warn(`Invalid hex color code: ${action.fillColor}`);
             return state;
         }

src/reducers/stroke-width.js

@@ -41,5 +41,6 @@ const changeStrokeWidth = function (strokeWidth) {
 export {
     reducer as default,
     changeStrokeWidth,
+    CHANGE_STROKE_WIDTH,
     MAX_STROKE_WIDTH
 };

test/unit/stroke-color-reducer.test.js

@@ -34,17 +34,29 @@ test('changeStrokeColorViaSelectedItems', () => {
 
     const strokeColor1 = 6;
     const strokeColor2 = null; // transparent
-    let selectedItems = [mockPaperRootItem({strokeColor: strokeColor1})];
+    let selectedItems = [mockPaperRootItem({strokeColor: strokeColor1, strokeWidth: 1})];
     expect(strokeColorReducer(defaultState /* state */, setSelectedItems(selectedItems) /* action */))
         .toEqual(strokeColor1);
-    selectedItems = [mockPaperRootItem({strokeColor: strokeColor2})];
+    selectedItems = [mockPaperRootItem({strokeColor: strokeColor2, strokeWidth: 1})];
     expect(strokeColorReducer(defaultState /* state */, setSelectedItems(selectedItems) /* action */))
         .toEqual(strokeColor2);
-    selectedItems = [mockPaperRootItem({strokeColor: strokeColor1}), mockPaperRootItem({strokeColor: strokeColor2})];
+    selectedItems = [mockPaperRootItem({strokeColor: strokeColor1, strokeWidth: 1}),
+        mockPaperRootItem({strokeColor: strokeColor2, strokeWidth: 1})];
     expect(strokeColorReducer(defaultState /* state */, setSelectedItems(selectedItems) /* action */))
         .toEqual(MIXED);
 });
 
+test('showNoStrokeColorIfNoStrokeWidth', () => {
+    let defaultState;
+
+    let selectedItems = [mockPaperRootItem({strokeColor: '#fff', strokeWidth: null})];
+    expect(strokeColorReducer(defaultState /* state */, setSelectedItems(selectedItems) /* action */))
+        .toEqual(null);
+    selectedItems = [mockPaperRootItem({strokeColor: '#fff', strokeWidth: 0})];
+    expect(strokeColorReducer(defaultState /* state */, setSelectedItems(selectedItems) /* action */))
+        .toEqual(null);
+});
+
 test('invalidChangeStrokeColor', () => {
     const origState = '#fff';
 

test/unit/stroke-width-reducer.test.js

@@ -30,17 +30,26 @@ test('changeStrokeWidthViaSelectedItems', () => {
 
     const strokeWidth1 = 6;
     let strokeWidth2; // no outline
-    let selectedItems = [mockPaperRootItem({strokeWidth: strokeWidth1})];
+    let selectedItems = [mockPaperRootItem({strokeColor: '#000', strokeWidth: strokeWidth1})];
     expect(strokeWidthReducer(defaultState /* state */, setSelectedItems(selectedItems) /* action */))
         .toEqual(strokeWidth1);
-    selectedItems = [mockPaperRootItem({strokeWidth: strokeWidth2})];
+    selectedItems = [mockPaperRootItem({strokeColor: '#000', strokeWidth: strokeWidth2})];
     expect(strokeWidthReducer(defaultState /* state */, setSelectedItems(selectedItems) /* action */))
         .toEqual(0); // Convert no outline to stroke width 0
-    selectedItems = [mockPaperRootItem({strokeWidth: strokeWidth1}), mockPaperRootItem({strokeWidth: strokeWidth2})];
+    selectedItems = [mockPaperRootItem({strokeColor: '#000', strokeWidth: strokeWidth1}),
+        mockPaperRootItem({strokeColor: '#000', strokeWidth: strokeWidth2})];
     expect(strokeWidthReducer(defaultState /* state */, setSelectedItems(selectedItems) /* action */))
         .toEqual(null); // null indicates mixed for stroke width
 });
 
+test('showNoStrokeWidthIfNoStrokeColor', () => {
+    let defaultState;
+
+    const selectedItems = [mockPaperRootItem({strokeColor: null, strokeWidth: 10})];
+    expect(strokeWidthReducer(defaultState /* state */, setSelectedItems(selectedItems) /* action */))
+        .toEqual(0);
+});
+
 test('invalidChangestrokeWidth', () => {
     const origState = {strokeWidth: 1};