diff --git a/platform/commonUI/edit/src/representers/EditToolbar.js b/platform/commonUI/edit/src/representers/EditToolbar.js index 522a808aaf..8d5881a5a5 100644 --- a/platform/commonUI/edit/src/representers/EditToolbar.js +++ b/platform/commonUI/edit/src/representers/EditToolbar.js @@ -34,14 +34,20 @@ define( // Update value for this property in all elements of the // selection which have this property. function updateProperties(property, value) { + var changed = false; + // Update property in a selected element function updateProperty(selected) { // Ignore selected elements which don't have this property if (selected[property] !== undefined) { // Check if this is a setter, or just assignable if (typeof selected[property] === 'function') { + changed = + changed || (selected[property]() !== value); selected[property](value); } else { + changed = + changed || (selected[property] !== value); selected[property] = value; } } @@ -49,6 +55,9 @@ define( // Update property in all selected elements selection.forEach(updateProperty); + + // Return whether or not anything changed + return changed; } // Look up the current value associated with a property @@ -220,7 +229,7 @@ define( * @param value the new value to convey to the selection */ updateState: function (index, value) { - updateProperties(properties[index], value); + return updateProperties(properties[index], value); } }; } diff --git a/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js b/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js index b17c7f7d88..a1852e3bcd 100644 --- a/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js +++ b/platform/commonUI/edit/src/representers/EditToolbarRepresenter.js @@ -51,11 +51,17 @@ define( // Update selection models to match changed toolbar state function updateState(state) { // Update underlying state based on toolbar changes - (state || []).forEach(function (value, index) { - toolbar.updateState(index, value); - }); - // Commit the changes. - commit("Changes from toolbar."); + var changed = (state || []).map(function (value, index) { + return toolbar.updateState(index, value); + }).reduce(function (a, b) { + return a || b; + }, false); + + // Only commit if something actually changed + if (changed) { + // Commit the changes. + commit("Changes from toolbar."); + } } // Initialize toolbar (expose object to parent scope) diff --git a/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js b/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js index 56093b5e86..b3c5c64a1f 100644 --- a/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js +++ b/platform/commonUI/edit/test/representers/EditToolbarRepresenterSpec.js @@ -91,6 +91,32 @@ define( // Should have updated the original object expect(testObject.k).toEqual(456); + + // Should have committed the change + expect(mockScope.commit).toHaveBeenCalled(); + }); + + it("does not commit if nothing changed", function () { + var testObject = { k: 123 }; + + // Provide a view which has a toolbar + representer.represent({ + toolbar: { sections: [ { items: [ { property: 'k' } ] } ] } + }); + + // Update the selection + mockScope.selection.push(testObject); + expect(mockScope.$watchCollection.mostRecentCall.args[0]) + .toEqual('selection'); // Make sure we're using right watch + mockScope.$watchCollection.mostRecentCall.args[1]([testObject]); + + // Invoke the first watch (assumed to be for toolbar state) + mockScope.$watchCollection.calls[0].args[1]( + mockScope.$parent.testToolbar.state + ); + + // Should have committed the change + expect(mockScope.commit).not.toHaveBeenCalled(); }); }); diff --git a/platform/commonUI/edit/test/representers/EditToolbarSpec.js b/platform/commonUI/edit/test/representers/EditToolbarSpec.js index ea2fa928f6..452b59cefd 100644 --- a/platform/commonUI/edit/test/representers/EditToolbarSpec.js +++ b/platform/commonUI/edit/test/representers/EditToolbarSpec.js @@ -143,6 +143,15 @@ define( expect(testABC.a).toHaveBeenCalledWith("new value"); }); + it("provides a return value describing update status", function () { + // Should return true if actually updated, otherwise false + var key; + toolbar.setSelection([ testABC ]); + key = toolbar.getStructure().sections[0].items[0].key; + expect(toolbar.updateState(key, testABC.a)).toBeFalsy(); + expect(toolbar.updateState(key, "new value")).toBeTruthy(); + }); + it("removes inapplicable items", function () { // First, verify with all items toolbar.setSelection([ testABC ]);