diff --git a/src/containers/selection-hoc.jsx b/src/containers/selection-hoc.jsx index cb7af472..2066ed09 100644 --- a/src/containers/selection-hoc.jsx +++ b/src/containers/selection-hoc.jsx @@ -65,7 +65,7 @@ const SelectionHOC = function (WrappedComponent) { if (selectedItems.length === 0) { return; } - const colorState = getColorsFromSelection(); + const colorState = getColorsFromSelection(selectedItems); dispatch(changeFillColor(colorState.fillColor)); dispatch(changeStrokeColor(colorState.strokeColor)); dispatch(changeStrokeWidth(colorState.strokeWidth)); diff --git a/src/helper/style-path.js b/src/helper/style-path.js index 6c660017..2e27713f 100644 --- a/src/helper/style-path.js +++ b/src/helper/style-path.js @@ -82,12 +82,12 @@ const applyStrokeWidthToSelection = function (value) { /** * Get state of colors and stroke width for selection + * @param {!Array} selectedItems Selected paper items * @return {object} Object of strokeColor, strokeWidth, fillColor of the selection. * Gives MIXED when there are mixed values for a color, and null for transparent. * Gives null when there are mixed values for stroke width. */ -const getColorsFromSelection = function () { - const selectedItems = getSelectedItems(true /* recursive */); +const getColorsFromSelection = function (selectedItems) { let selectionFillColorString; let selectionStrokeColorString; let selectionStrokeWidth; diff --git a/src/reducers/fill-color.js b/src/reducers/fill-color.js index 783ef18b..fafa3016 100644 --- a/src/reducers/fill-color.js +++ b/src/reducers/fill-color.js @@ -21,7 +21,7 @@ const reducer = function (state, action) { if (!action.selectedItems || !action.selectedItems.length) { return state; } - return getColorsFromSelection().fillColor; + return getColorsFromSelection(action.selectedItems).fillColor; default: return state; } diff --git a/src/reducers/selected-items.js b/src/reducers/selected-items.js index 2ebb8140..67a2d147 100644 --- a/src/reducers/selected-items.js +++ b/src/reducers/selected-items.js @@ -1,3 +1,4 @@ +import log from '../log/log'; const CHANGE_SELECTED_ITEMS = 'scratch-paint/select/CHANGE_SELECTED_ITEMS'; const initialState = []; @@ -5,11 +6,15 @@ const reducer = function (state, action) { if (typeof state === 'undefined') state = initialState; switch (action.type) { case CHANGE_SELECTED_ITEMS: + if (!action.selectedItems || !(action.selectedItems instanceof Array)) { + log.warn(`No selected items or wrong format provided: ${action.selectedItems}`); + return state; + } // If they are not equal, return the new list of items. Else return old list if (action.selectedItems.length !== state.length) { return action.selectedItems; } - // Shallow equality check + // Shallow equality check (we may need to update this later for more granularity) for (let i = 0; i < action.selectedItems.length; i++) { if (action.selectedItems[i] !== state[i]) { return action.selectedItems; diff --git a/src/reducers/stroke-color.js b/src/reducers/stroke-color.js index 4d414447..a7ecba9e 100644 --- a/src/reducers/stroke-color.js +++ b/src/reducers/stroke-color.js @@ -21,7 +21,7 @@ const reducer = function (state, action) { if (!action.selectedItems || !action.selectedItems.length) { return state; } - return getColorsFromSelection().strokeColor; + return getColorsFromSelection(action.selectedItems).strokeColor; default: return state; } diff --git a/src/reducers/stroke-width.js b/src/reducers/stroke-width.js index 57c13926..43213b8d 100644 --- a/src/reducers/stroke-width.js +++ b/src/reducers/stroke-width.js @@ -20,7 +20,7 @@ const reducer = function (state, action) { if (!action.selectedItems || !action.selectedItems.length) { return state; } - return getColorsFromSelection().strokeWidth; + return getColorsFromSelection(action.selectedItems).strokeWidth; default: return state; } diff --git a/test/__mocks__/paperMocks.js b/test/__mocks__/paperMocks.js new file mode 100644 index 00000000..b8d9ff15 --- /dev/null +++ b/test/__mocks__/paperMocks.js @@ -0,0 +1,23 @@ +/** + * Pretend paper.Item whose parent is a layer. + * @param {object} options Item params + * @param {string} options.strokeColor Value to return for the item's stroke color + * @param {string} options.fillColor Value to return for the item's fill color + * @param {string} options.strokeWidth Value to return for the item's stroke width + * @return {object} mock item + */ +const mockPaperRootItem = function (options) { + return { + strokeColor: {toCSS: function () { + return options.strokeColor; + }}, + fillColor: {toCSS: function () { + return options.fillColor; + }}, + strokeWidth: options.strokeWidth, + parent: {className: 'Layer'}, + data: {} + }; +}; + +export {mockPaperRootItem}; diff --git a/test/unit/fill-color-reducer.test.js b/test/unit/fill-color-reducer.test.js index dddd9008..fb830431 100644 --- a/test/unit/fill-color-reducer.test.js +++ b/test/unit/fill-color-reducer.test.js @@ -1,6 +1,9 @@ /* eslint-env jest */ import fillColorReducer from '../../src/reducers/fill-color'; import {changeFillColor} from '../../src/reducers/fill-color'; +import {setSelectedItems} from '../../src/reducers/selected-items'; +import {MIXED} from '../../src/helper/style-path'; +import {mockPaperRootItem} from '../__mocks__/paperMocks'; test('initialState', () => { let defaultState; @@ -26,6 +29,22 @@ test('changeFillColor', () => { .toEqual(newFillColor); }); +test('changefillColorViaSelectedItems', () => { + let defaultState; + + const fillColor1 = 6; + const fillColor2 = null; // transparent + let selectedItems = [mockPaperRootItem({fillColor: fillColor1})]; + expect(fillColorReducer(defaultState /* state */, setSelectedItems(selectedItems) /* action */)) + .toEqual(fillColor1); + selectedItems = [mockPaperRootItem({fillColor: fillColor2})]; + expect(fillColorReducer(defaultState /* state */, setSelectedItems(selectedItems) /* action */)) + .toEqual(fillColor2); + selectedItems = [mockPaperRootItem({fillColor: fillColor1}), mockPaperRootItem({fillColor: fillColor2})]; + expect(fillColorReducer(defaultState /* state */, setSelectedItems(selectedItems) /* action */)) + .toEqual(MIXED); +}); + test('invalidChangeFillColor', () => { const origState = '#fff'; diff --git a/test/unit/selected-items-reducer.test.js b/test/unit/selected-items-reducer.test.js new file mode 100644 index 00000000..d4a30ba8 --- /dev/null +++ b/test/unit/selected-items-reducer.test.js @@ -0,0 +1,47 @@ +/* eslint-env jest */ +import selectedItemsReducer from '../../src/reducers/selected-items'; +import {setSelectedItems, clearSelectedItems} from '../../src/reducers/selected-items'; + +test('initialState', () => { + let defaultState; + + expect(selectedItemsReducer(defaultState /* state */, {type: 'anything'} /* action */)).toBeDefined(); +}); + +test('setSelectedItems', () => { + let defaultState; + + const newSelected1 = ['selected1', 'selected2']; + const newSelected2 = ['selected1', 'selected3']; + const unselected = []; + expect(selectedItemsReducer(defaultState /* state */, setSelectedItems(newSelected1) /* action */)) + .toEqual(newSelected1); + expect(selectedItemsReducer(newSelected1, setSelectedItems(newSelected2) /* action */)) + .toEqual(newSelected2); + expect(selectedItemsReducer(newSelected1, setSelectedItems(unselected) /* action */)) + .toEqual(unselected); + expect(selectedItemsReducer(defaultState, setSelectedItems(unselected) /* action */)) + .toEqual(unselected); +}); + +test('clearSelectedItems', () => { + let defaultState; + + const selectedState = ['selected1', 'selected2']; + const unselectedState = []; + expect(selectedItemsReducer(defaultState /* state */, clearSelectedItems() /* action */)) + .toHaveLength(0); + expect(selectedItemsReducer(selectedState /* state */, clearSelectedItems() /* action */)) + .toHaveLength(0); + expect(selectedItemsReducer(unselectedState /* state */, clearSelectedItems() /* action */)) + .toHaveLength(0); +}); + +test('invalidsetSelectedItems', () => { + const origState = ['selected1', 'selected2']; + + expect(selectedItemsReducer(origState /* state */, setSelectedItems() /* action */)) + .toBe(origState); + expect(selectedItemsReducer(origState /* state */, setSelectedItems('notAnArray') /* action */)) + .toBe(origState); +}); diff --git a/test/unit/stroke-color-reducer.test.js b/test/unit/stroke-color-reducer.test.js index 7f812299..e823d2e1 100644 --- a/test/unit/stroke-color-reducer.test.js +++ b/test/unit/stroke-color-reducer.test.js @@ -1,6 +1,9 @@ /* eslint-env jest */ import strokeColorReducer from '../../src/reducers/stroke-color'; import {changeStrokeColor} from '../../src/reducers/stroke-color'; +import {setSelectedItems} from '../../src/reducers/selected-items'; +import {MIXED} from '../../src/helper/style-path'; +import {mockPaperRootItem} from '../__mocks__/paperMocks'; test('initialState', () => { let defaultState; @@ -26,6 +29,22 @@ test('changeStrokeColor', () => { .toEqual(newStrokeColor); }); +test('changeStrokeColorViaSelectedItems', () => { + let defaultState; + + const strokeColor1 = 6; + const strokeColor2 = null; // transparent + let selectedItems = [mockPaperRootItem({strokeColor: strokeColor1})]; + expect(strokeColorReducer(defaultState /* state */, setSelectedItems(selectedItems) /* action */)) + .toEqual(strokeColor1); + selectedItems = [mockPaperRootItem({strokeColor: strokeColor2})]; + expect(strokeColorReducer(defaultState /* state */, setSelectedItems(selectedItems) /* action */)) + .toEqual(strokeColor2); + selectedItems = [mockPaperRootItem({strokeColor: strokeColor1}), mockPaperRootItem({strokeColor: strokeColor2})]; + expect(strokeColorReducer(defaultState /* state */, setSelectedItems(selectedItems) /* action */)) + .toEqual(MIXED); +}); + test('invalidChangeStrokeColor', () => { const origState = '#fff'; diff --git a/test/unit/stroke-width-reducer.test.js b/test/unit/stroke-width-reducer.test.js index a2189049..03fd9184 100644 --- a/test/unit/stroke-width-reducer.test.js +++ b/test/unit/stroke-width-reducer.test.js @@ -1,6 +1,8 @@ /* eslint-env jest */ import strokeWidthReducer from '../../src/reducers/stroke-width'; import {MAX_STROKE_WIDTH, changeStrokeWidth} from '../../src/reducers/stroke-width'; +import {setSelectedItems} from '../../src/reducers/selected-items'; +import {mockPaperRootItem} from '../__mocks__/paperMocks'; test('initialState', () => { let defaultState; @@ -23,6 +25,22 @@ test('changestrokeWidth', () => { .toEqual(MAX_STROKE_WIDTH); }); +test('changeStrokeWidthViaSelectedItems', () => { + let defaultState; + + const strokeWidth1 = 6; + let strokeWidth2; // no outline + let selectedItems = [mockPaperRootItem({strokeWidth: strokeWidth1})]; + expect(strokeWidthReducer(defaultState /* state */, setSelectedItems(selectedItems) /* action */)) + .toEqual(strokeWidth1); + selectedItems = [mockPaperRootItem({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})]; + expect(strokeWidthReducer(defaultState /* state */, setSelectedItems(selectedItems) /* action */)) + .toEqual(null); // null indicates mixed for stroke width +}); + test('invalidChangestrokeWidth', () => { const origState = {strokeWidth: 1};