[Edit] Avoid extra commits

Avoid issuing extra commit calls from the Edit toolbar;
this can cause disruption of state from other views (such
as Fixed Position), WTD-881.
This commit is contained in:
Victor Woeltjen 2015-02-24 09:47:00 -08:00
parent 3d86871c1d
commit 1e76264d82
4 changed files with 56 additions and 6 deletions

View File

@ -34,14 +34,20 @@ define(
// Update value for this property in all elements of the // Update value for this property in all elements of the
// selection which have this property. // selection which have this property.
function updateProperties(property, value) { function updateProperties(property, value) {
var changed = false;
// Update property in a selected element // Update property in a selected element
function updateProperty(selected) { function updateProperty(selected) {
// Ignore selected elements which don't have this property // Ignore selected elements which don't have this property
if (selected[property] !== undefined) { if (selected[property] !== undefined) {
// Check if this is a setter, or just assignable // Check if this is a setter, or just assignable
if (typeof selected[property] === 'function') { if (typeof selected[property] === 'function') {
changed =
changed || (selected[property]() !== value);
selected[property](value); selected[property](value);
} else { } else {
changed =
changed || (selected[property] !== value);
selected[property] = value; selected[property] = value;
} }
} }
@ -49,6 +55,9 @@ define(
// Update property in all selected elements // Update property in all selected elements
selection.forEach(updateProperty); selection.forEach(updateProperty);
// Return whether or not anything changed
return changed;
} }
// Look up the current value associated with a property // Look up the current value associated with a property
@ -220,7 +229,7 @@ define(
* @param value the new value to convey to the selection * @param value the new value to convey to the selection
*/ */
updateState: function (index, value) { updateState: function (index, value) {
updateProperties(properties[index], value); return updateProperties(properties[index], value);
} }
}; };
} }

View File

@ -51,11 +51,17 @@ define(
// Update selection models to match changed toolbar state // Update selection models to match changed toolbar state
function updateState(state) { function updateState(state) {
// Update underlying state based on toolbar changes // Update underlying state based on toolbar changes
(state || []).forEach(function (value, index) { var changed = (state || []).map(function (value, index) {
toolbar.updateState(index, value); return toolbar.updateState(index, value);
}); }).reduce(function (a, b) {
// Commit the changes. return a || b;
commit("Changes from toolbar."); }, false);
// Only commit if something actually changed
if (changed) {
// Commit the changes.
commit("Changes from toolbar.");
}
} }
// Initialize toolbar (expose object to parent scope) // Initialize toolbar (expose object to parent scope)

View File

@ -91,6 +91,32 @@ define(
// Should have updated the original object // Should have updated the original object
expect(testObject.k).toEqual(456); 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();
}); });
}); });

View File

@ -143,6 +143,15 @@ define(
expect(testABC.a).toHaveBeenCalledWith("new value"); 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 () { it("removes inapplicable items", function () {
// First, verify with all items // First, verify with all items
toolbar.setSelection([ testABC ]); toolbar.setSelection([ testABC ]);