diff --git a/src/containers/fill-color-indicator.jsx b/src/containers/fill-color-indicator.jsx index 73757694..89068f40 100644 --- a/src/containers/fill-color-indicator.jsx +++ b/src/containers/fill-color-indicator.jsx @@ -15,9 +15,22 @@ class FillColorIndicator extends React.Component { bindAll(this, [ 'handleChangeFillColor' ]); + + // Flag to track whether an svg-update-worthy change has been made + this._hasChanged = false; + } + componentWillReceiveProps (newProps) { + const {fillColorModalVisible, onUpdateSvg} = this.props; + if (fillColorModalVisible && !newProps.fillColorModalVisible) { + // Submit the new SVG, which also stores a single undo/redo action. + if (this._hasChanged) onUpdateSvg(); + this._hasChanged = false; + } } handleChangeFillColor (newColor) { - applyFillColorToSelection(newColor, this.props.onUpdateSvg); + // Apply color and update redux, but do not update svg until picker closes. + const isDifferent = applyFillColorToSelection(newColor); + this._hasChanged = this._hasChanged || isDifferent; this.props.onChangeFillColor(newColor); } render () { @@ -51,6 +64,7 @@ const mapDispatchToProps = dispatch => ({ FillColorIndicator.propTypes = { disabled: PropTypes.bool.isRequired, fillColor: PropTypes.string, + fillColorModalVisible: PropTypes.bool.isRequired, onChangeFillColor: PropTypes.func.isRequired, onUpdateSvg: PropTypes.func.isRequired }; diff --git a/src/containers/stroke-color-indicator.jsx b/src/containers/stroke-color-indicator.jsx index 240be3ee..dfbaa6e9 100644 --- a/src/containers/stroke-color-indicator.jsx +++ b/src/containers/stroke-color-indicator.jsx @@ -15,9 +15,22 @@ class StrokeColorIndicator extends React.Component { bindAll(this, [ 'handleChangeStrokeColor' ]); + + // Flag to track whether an svg-update-worthy change has been made + this._hasChanged = false; + } + componentWillReceiveProps (newProps) { + const {strokeColorModalVisible, onUpdateSvg} = this.props; + if (strokeColorModalVisible && !newProps.strokeColorModalVisible) { + // Submit the new SVG, which also stores a single undo/redo action. + if (this._hasChanged) onUpdateSvg(); + this._hasChanged = false; + } } handleChangeStrokeColor (newColor) { - applyStrokeColorToSelection(newColor, this.props.onUpdateSvg); + // Apply color and update redux, but do not update svg until picker closes. + const isDifferent = applyStrokeColorToSelection(newColor); + this._hasChanged = this._hasChanged || isDifferent; this.props.onChangeStrokeColor(newColor); } render () { @@ -52,7 +65,8 @@ StrokeColorIndicator.propTypes = { disabled: PropTypes.bool.isRequired, onChangeStrokeColor: PropTypes.func.isRequired, onUpdateSvg: PropTypes.func.isRequired, - strokeColor: PropTypes.string + strokeColor: PropTypes.string, + strokeColorModalVisible: PropTypes.bool.isRequired }; export default connect( diff --git a/src/helper/style-path.js b/src/helper/style-path.js index a5b62360..d7330e25 100644 --- a/src/helper/style-path.js +++ b/src/helper/style-path.js @@ -8,9 +8,9 @@ const MIXED = 'scratch-paint/style-path/mixed'; /** * Called when setting fill color * @param {string} colorString New color, css format - * @param {!function} onUpdateSvg A callback to call when the image visibly changes + * @return {boolean} Whether the color application actually changed visibly. */ -const applyFillColorToSelection = function (colorString, onUpdateSvg) { +const applyFillColorToSelection = function (colorString) { const items = getSelectedLeafItems(); let changed = false; for (const item of items) { @@ -45,9 +45,7 @@ const applyFillColorToSelection = function (colorString, onUpdateSvg) { } } } - if (changed) { - onUpdateSvg(); - } + return changed; }; const _strokeColorMatch = function (item, incomingColor) { @@ -58,9 +56,9 @@ const _strokeColorMatch = function (item, incomingColor) { /** * Called when setting stroke color * @param {string} colorString New color, css format - * @param {!function} onUpdateSvg A callback to call when the image visibly changes + * @return {boolean} Whether the color application actually changed visibly. */ -const applyStrokeColorToSelection = function (colorString, onUpdateSvg) { +const applyStrokeColorToSelection = function (colorString) { const items = getSelectedLeafItems(); let changed = false; for (const item of items) { @@ -94,9 +92,7 @@ const applyStrokeColorToSelection = function (colorString, onUpdateSvg) { item.strokeColor = colorString; } } - if (changed) { - onUpdateSvg(); - } + return changed; }; /** @@ -132,11 +128,11 @@ const getColorsFromSelection = function (selectedItems) { let selectionStrokeColorString; let selectionStrokeWidth; let firstChild = true; - + for (const item of selectedItems) { let itemFillColorString; let itemStrokeColorString; - + // handle pgTextItems differently by going through their children if (isPGTextItem(item)) { for (const child of item.children) {