From c6fb28a69c1538532edd246ec7ff519c74ca0971 Mon Sep 17 00:00:00 2001 From: DD Liu Date: Wed, 26 Sep 2018 13:08:47 -0400 Subject: [PATCH] Make stroke width 0 when transparent (#695) --- src/containers/stroke-color-indicator.jsx | 21 +++++++++--- src/containers/stroke-width-indicator.jsx | 28 +++++++++++++-- src/helper/style-path.js | 42 ++++++----------------- src/reducers/stroke-color.js | 10 ++++-- src/reducers/stroke-width.js | 1 + test/unit/stroke-color-reducer.test.js | 18 ++++++++-- test/unit/stroke-width-reducer.test.js | 15 ++++++-- 7 files changed, 87 insertions(+), 48 deletions(-) diff --git a/src/containers/stroke-color-indicator.jsx b/src/containers/stroke-color-indicator.jsx index 965999bc..ea78e0cb 100644 --- a/src/containers/stroke-color-indicator.jsx +++ b/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 = - applyStrokeColorToSelection(newColor, isBitmap(this.props.format), this.props.textEditTarget); - this._hasChanged = this._hasChanged || isDifferent; + this._hasChanged = + applyStrokeColorToSelection(newColor, isBitmap(this.props.format), this.props.textEditTarget) || + 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, diff --git a/src/containers/stroke-width-indicator.jsx b/src/containers/stroke-width-indicator.jsx index 692fb9a9..d224f44d 100644 --- a/src/containers/stroke-width-indicator.jsx +++ b/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)) { - this.props.onUpdateImage(); + let changed = applyStrokeWidthToSelection(newWidth, this.props.textEditTarget); + 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, diff --git a/src/helper/style-path.js b/src/helper/style-path.js index 040dc7b4..2f5ecb84 100644 --- a/src/helper/style-path.js +++ b/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 (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)) { + 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); diff --git a/src/reducers/stroke-color.js b/src/reducers/stroke-color.js index 8e6d3a39..50cad337 100644 --- a/src/reducers/stroke-color.js +++ b/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; } diff --git a/src/reducers/stroke-width.js b/src/reducers/stroke-width.js index a1daae44..6d504748 100644 --- a/src/reducers/stroke-width.js +++ b/src/reducers/stroke-width.js @@ -41,5 +41,6 @@ const changeStrokeWidth = function (strokeWidth) { export { reducer as default, changeStrokeWidth, + CHANGE_STROKE_WIDTH, MAX_STROKE_WIDTH }; diff --git a/test/unit/stroke-color-reducer.test.js b/test/unit/stroke-color-reducer.test.js index e823d2e1..25b55222 100644 --- a/test/unit/stroke-color-reducer.test.js +++ b/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'; diff --git a/test/unit/stroke-width-reducer.test.js b/test/unit/stroke-width-reducer.test.js index 03fd9184..f4ad3061 100644 --- a/test/unit/stroke-width-reducer.test.js +++ b/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};