Merge pull request #1243 from fsih/gradientOutlines2

Gradient outlines more fixes

src/containers/oval-mode.jsx

@@ -4,11 +4,12 @@ import React from 'react';
 import {connect} from 'react-redux';
 import bindAll from 'lodash.bindall';
 import Modes from '../lib/modes';
-import ColorStyleProptype from '../lib/color-style-proptype';
 import {MIXED} from '../helper/style-path';
+import ColorStyleProptype from '../lib/color-style-proptype';
+import GradientTypes from '../lib/gradient-types';
 
-import {changeFillColor, DEFAULT_COLOR} from '../reducers/fill-style';
+import {changeFillColor, clearFillGradient, DEFAULT_COLOR} from '../reducers/fill-style';
-import {changeStrokeColor} from '../reducers/stroke-style';
+import {changeStrokeColor, clearStrokeGradient} from '../reducers/stroke-style';
 import {changeMode} from '../reducers/modes';
 import {clearSelectedItems, setSelectedItems} from '../reducers/selected-items';
 import {setCursor} from '../reducers/cursor';
@@ -22,7 +23,8 @@ class OvalMode extends React.Component {
         super(props);
         bindAll(this, [
             'activateTool',
-            'deactivateTool'
+            'deactivateTool',
+            'validateColorState'
         ]);
     }
     componentDidMount () {
@@ -54,23 +56,8 @@ class OvalMode extends React.Component {
     }
     activateTool () {
         clearSelection(this.props.clearSelectedItems);
-        // If fill and stroke color are both mixed/transparent/absent, set fill to default and stroke to transparent.
+        this.validateColorState();
-        // If exactly one of fill or stroke color is set, set the other one to transparent.
+
-        // This way the tool won't draw an invisible state, or be unclear about what will be drawn.
-        const {strokeWidth} = this.props.colorState;
-        const fillColor = this.props.colorState.fillColor.primary;
-        const strokeColor = this.props.colorState.strokeColor.primary;
-        const fillColorPresent = fillColor !== MIXED && fillColor !== null;
-        const strokeColorPresent =
-            strokeColor !== MIXED && strokeColor !== null && strokeWidth !== null && strokeWidth !== 0;
-        if (!fillColorPresent && !strokeColorPresent) {
-            this.props.onChangeFillColor(DEFAULT_COLOR);
-            this.props.onChangeStrokeColor(null);
-        } else if (!fillColorPresent && strokeColorPresent) {
-            this.props.onChangeFillColor(null);
-        } else if (fillColorPresent && !strokeColorPresent) {
-            this.props.onChangeStrokeColor(null);
-        }
         this.tool = new OvalTool(
             this.props.setSelectedItems,
             this.props.clearSelectedItems,
@@ -85,6 +72,51 @@ class OvalMode extends React.Component {
         this.tool.remove();
         this.tool = null;
     }
+    validateColorState () {
+        // Make sure that at least one of fill/stroke is set, and that MIXED is not one of the colors.
+        // If fill and stroke color are both missing, set fill to default and stroke to transparent.
+        // If exactly one of fill or stroke color is set, set the other one to transparent.
+        const {strokeWidth} = this.props.colorState;
+        const fillColor1 = this.props.colorState.fillColor.primary;
+        let fillColor2 = this.props.colorState.fillColor.secondary;
+        let fillGradient = this.props.colorState.fillColor.gradientType;
+        const strokeColor1 = this.props.colorState.strokeColor.primary;
+        let strokeColor2 = this.props.colorState.strokeColor.secondary;
+        let strokeGradient = this.props.colorState.strokeColor.gradientType;
+
+        if (fillColor2 === MIXED) {
+            this.props.clearFillGradient();
+            fillColor2 = null;
+            fillGradient = GradientTypes.SOLID;
+        }
+        if (strokeColor2 === MIXED) {
+            this.props.clearStrokeGradient();
+            strokeColor2 = null;
+            strokeGradient = GradientTypes.SOLID;
+        }
+
+        const fillColorMissing = fillColor1 === MIXED ||
+            (fillGradient === GradientTypes.SOLID && fillColor1 === null) ||
+            (fillGradient !== GradientTypes.SOLID && fillColor1 === null && fillColor2 === null);
+        const strokeColorMissing = strokeColor1 === MIXED ||
+            strokeWidth === null ||
+            strokeWidth === 0 ||
+            (strokeGradient === GradientTypes.SOLID && strokeColor1 === null) ||
+            (strokeGradient !== GradientTypes.SOLID && strokeColor1 === null && strokeColor2 === null);
+
+        if (fillColorMissing && strokeColorMissing) {
+            this.props.onChangeFillColor(DEFAULT_COLOR);
+            this.props.clearFillGradient();
+            this.props.onChangeStrokeColor(null);
+            this.props.clearStrokeGradient();
+        } else if (fillColorMissing && !strokeColorMissing) {
+            this.props.onChangeFillColor(null);
+            this.props.clearFillGradient();
+        } else if (!fillColorMissing && strokeColorMissing) {
+            this.props.onChangeStrokeColor(null);
+            this.props.clearStrokeGradient();
+        }
+    }
     render () {
         return (
             <OvalModeComponent
@@ -96,6 +128,8 @@ class OvalMode extends React.Component {
 }
 
 OvalMode.propTypes = {
+    clearFillGradient: PropTypes.func.isRequired,
+    clearStrokeGradient: PropTypes.func.isRequired,
     clearSelectedItems: PropTypes.func.isRequired,
     colorState: PropTypes.shape({
         fillColor: ColorStyleProptype,
@@ -121,6 +155,12 @@ const mapDispatchToProps = dispatch => ({
     clearSelectedItems: () => {
         dispatch(clearSelectedItems());
     },
+    clearFillGradient: () => {
+        dispatch(clearFillGradient());
+    },
+    clearStrokeGradient: () => {
+        dispatch(clearStrokeGradient());
+    },
     setCursor: cursorString => {
         dispatch(setCursor(cursorString));
     },

src/containers/rect-mode.jsx

@@ -4,11 +4,12 @@ import React from 'react';
 import {connect} from 'react-redux';
 import bindAll from 'lodash.bindall';
 import Modes from '../lib/modes';
-import ColorStyleProptype from '../lib/color-style-proptype';
 import {MIXED} from '../helper/style-path';
+import ColorStyleProptype from '../lib/color-style-proptype';
+import GradientTypes from '../lib/gradient-types';
 
-import {changeFillColor, DEFAULT_COLOR} from '../reducers/fill-style';
+import {changeFillColor, clearFillGradient, DEFAULT_COLOR} from '../reducers/fill-style';
-import {changeStrokeColor} from '../reducers/stroke-style';
+import {changeStrokeColor, clearStrokeGradient} from '../reducers/stroke-style';
 import {changeMode} from '../reducers/modes';
 import {clearSelectedItems, setSelectedItems} from '../reducers/selected-items';
 import {setCursor} from '../reducers/cursor';
@@ -22,7 +23,8 @@ class RectMode extends React.Component {
         super(props);
         bindAll(this, [
             'activateTool',
-            'deactivateTool'
+            'deactivateTool',
+            'validateColorState'
         ]);
     }
     componentDidMount () {
@@ -54,23 +56,8 @@ class RectMode extends React.Component {
     }
     activateTool () {
         clearSelection(this.props.clearSelectedItems);
-        // If fill and stroke color are both mixed/transparent/absent, set fill to default and stroke to transparent.
+        this.validateColorState();
-        // If exactly one of fill or stroke color is set, set the other one to transparent.
+
-        // This way the tool won't draw an invisible state, or be unclear about what will be drawn.
-        const {strokeWidth} = this.props.colorState;
-        const fillColor = this.props.colorState.fillColor.primary;
-        const strokeColor = this.props.colorState.strokeColor.primary;
-        const fillColorPresent = fillColor !== MIXED && fillColor !== null;
-        const strokeColorPresent =
-            strokeColor !== MIXED && strokeColor !== null && strokeWidth !== null && strokeWidth !== 0;
-        if (!fillColorPresent && !strokeColorPresent) {
-            this.props.onChangeFillColor(DEFAULT_COLOR);
-            this.props.onChangeStrokeColor(null);
-        } else if (!fillColorPresent && strokeColorPresent) {
-            this.props.onChangeFillColor(null);
-        } else if (fillColorPresent && !strokeColorPresent) {
-            this.props.onChangeStrokeColor(null);
-        }
         this.tool = new RectTool(
             this.props.setSelectedItems,
             this.props.clearSelectedItems,
@@ -80,6 +67,51 @@ class RectMode extends React.Component {
         this.tool.setColorState(this.props.colorState);
         this.tool.activate();
     }
+    validateColorState () { // TODO move to shared class
+        // Make sure that at least one of fill/stroke is set, and that MIXED is not one of the colors.
+        // If fill and stroke color are both missing, set fill to default and stroke to transparent.
+        // If exactly one of fill or stroke color is set, set the other one to transparent.
+        const {strokeWidth} = this.props.colorState;
+        const fillColor1 = this.props.colorState.fillColor.primary;
+        let fillColor2 = this.props.colorState.fillColor.secondary;
+        let fillGradient = this.props.colorState.fillColor.gradientType;
+        const strokeColor1 = this.props.colorState.strokeColor.primary;
+        let strokeColor2 = this.props.colorState.strokeColor.secondary;
+        let strokeGradient = this.props.colorState.strokeColor.gradientType;
+
+        if (fillColor2 === MIXED) {
+            this.props.clearFillGradient();
+            fillColor2 = null;
+            fillGradient = GradientTypes.SOLID;
+        }
+        if (strokeColor2 === MIXED) {
+            this.props.clearStrokeGradient();
+            strokeColor2 = null;
+            strokeGradient = GradientTypes.SOLID;
+        }
+
+        const fillColorMissing = fillColor1 === MIXED ||
+            (fillGradient === GradientTypes.SOLID && fillColor1 === null) ||
+            (fillGradient !== GradientTypes.SOLID && fillColor1 === null && fillColor2 === null);
+        const strokeColorMissing = strokeColor1 === MIXED ||
+            strokeWidth === null ||
+            strokeWidth === 0 ||
+            (strokeGradient === GradientTypes.SOLID && strokeColor1 === null) ||
+            (strokeGradient !== GradientTypes.SOLID && strokeColor1 === null && strokeColor2 === null);
+
+        if (fillColorMissing && strokeColorMissing) {
+            this.props.onChangeFillColor(DEFAULT_COLOR);
+            this.props.clearFillGradient();
+            this.props.onChangeStrokeColor(null);
+            this.props.clearStrokeGradient();
+        } else if (fillColorMissing && !strokeColorMissing) {
+            this.props.onChangeFillColor(null);
+            this.props.clearFillGradient();
+        } else if (!fillColorMissing && strokeColorMissing) {
+            this.props.onChangeStrokeColor(null);
+            this.props.clearStrokeGradient();
+        }
+    }
     deactivateTool () {
         this.tool.deactivateTool();
         this.tool.remove();
@@ -96,6 +128,8 @@ class RectMode extends React.Component {
 }
 
 RectMode.propTypes = {
+    clearFillGradient: PropTypes.func.isRequired,
+    clearStrokeGradient: PropTypes.func.isRequired,
     clearSelectedItems: PropTypes.func.isRequired,
     colorState: PropTypes.shape({
         fillColor: ColorStyleProptype,
@@ -121,6 +155,12 @@ const mapDispatchToProps = dispatch => ({
     clearSelectedItems: () => {
         dispatch(clearSelectedItems());
     },
+    clearFillGradient: () => {
+        dispatch(clearFillGradient());
+    },
+    clearStrokeGradient: () => {
+        dispatch(clearStrokeGradient());
+    },
     setSelectedItems: () => {
         dispatch(setSelectedItems(getSelectedLeafItems(), false /* bitmapMode */));
     },

src/helper/style-path.js

@@ -298,17 +298,25 @@ const applyGradientTypeToSelection = function (gradientType, applyToStroke, text
             continue;
         }
 
-        if (!hasGradient && applyToStroke) {
+        // If this is a stroke, we don't display it as having a gradient in the color picker
-            const noStrokeOriginally = item.strokeWidth === 0 || !itemColor ||
+        // if there's no stroke width. Then treat it as if it doesn't have a gradient.
+        let hasDisplayGradient = hasGradient;
+        if (applyToStroke) hasDisplayGradient = hasGradient && item.strokeWidth > 0;
+        if (!hasDisplayGradient) {
+            const noColorOriginally = !itemColor ||
                 (itemColor.gradient &&
                 itemColor.gradient.stops &&
-                itemColor.gradient.stops.length === 2 &&
+                itemColor.gradient.stops[0].color.alpha === 0);
-                itemColor.gradient.stops[0].color.alpha === 0 &&
+            const addingStroke = applyToStroke && item.strokeWidth === 0;
-                itemColor.gradient.stops[1].color.alpha === 0);
             const hasGradientNow = itemColor1 || itemColor2;
-            if (noStrokeOriginally && hasGradientNow) {
+            if ((noColorOriginally || addingStroke) && hasGradientNow) {
-                // Make outline visible
+                if (applyToStroke) {
-                item.strokeWidth = 1;
+                    // Make outline visible
+                    item.strokeWidth = 1;
+                }
+                // Make the gradient black to white
+                itemColor1 = 'black';
+                itemColor2 = 'white';
             }
         }
 
@@ -323,18 +331,18 @@ const applyGradientTypeToSelection = function (gradientType, applyToStroke, text
         // If the item's gradient type differs from the gradient type we want to apply, then we change it
         switch (gradientType) {
         case GradientTypes.RADIAL: {
-            const hasRadialGradient = hasGradient && itemColor.gradient.radial;
+            const hasRadialGradient = hasDisplayGradient && itemColor.gradient.radial;
             gradientTypeDiffers = !hasRadialGradient;
             break;
         }
         case GradientTypes.HORIZONTAL: {
-            const hasHorizontalGradient = hasGradient && !itemColor.gradient.radial &&
+            const hasHorizontalGradient = hasDisplayGradient && !itemColor.gradient.radial &&
                 Math.abs(itemColor.origin.y - itemColor.destination.y) < 1e-8;
             gradientTypeDiffers = !hasHorizontalGradient;
             break;
         }
         case GradientTypes.VERTICAL: {
-            const hasVerticalGradient = hasGradient && !itemColor.gradient.radial &&
+            const hasVerticalGradient = hasDisplayGradient && !itemColor.gradient.radial &&
                 Math.abs(itemColor.origin.x - itemColor.destination.x) < 1e-8;
             gradientTypeDiffers = !hasVerticalGradient;
             break;
@@ -462,11 +470,14 @@ const getColorsFromSelection = function (selectedItems, bitmapMode) {
 
                     let strokeColorString = primary;
                     const strokeColor2String = secondary;
-                    const strokeGradientType = gradientType;
+                    let strokeGradientType = gradientType;
 
                     // If the item's stroke width is 0, pretend the stroke color is null
                     if (!item.strokeWidth) {
                         strokeColorString = null;
+                        // Hide the second color. This way if you choose a second color, remove
+                        // the gradient, and re-add it, your second color selection is preserved.
+                        strokeGradientType = GradientTypes.SOLID;
                     }
 
                     // Stroke color is fill color in bitmap