Merge pull request #446 from bdon/master

various fixes, refactors

js/id/graph/history.js

@@ -109,6 +109,16 @@ iD.History = function() {
             };
         },
 
+        hasChanges: function() {
+            return !!this.numChanges();
+        },
+
+        numChanges: function() {
+            return d3.sum(d3.values(this.changes()).map(function(c) {
+                return c.length;
+            }));
+        },
+
         imagery_used: function(source) {
             if (source) imagery_used = source;
             else return _.without(

js/id/id.js

@@ -50,7 +50,7 @@ window.iD = function(container) {
             }
         }
 
-        notice = iD.ui.notice(limiter
+        var notice = iD.ui.notice(limiter
             .append('div')
             .attr('class', 'notice'));
 
@@ -98,13 +98,7 @@ window.iD = function(container) {
             .call(iD.ui.save().map(map).controller(controller));
 
         history.on('change.warn-unload', function() {
-            var changes = history.changes(),
-
-                has_changes = !!d3.sum(d3.values(changes).map(function(c) {
-                    return c.length;
-                }));
-
-            window.onbeforeunload = has_changes ? function() {
+            window.onbeforeunload = history.hasChanges() ? function() {
                 return 'You have unsaved changes.';
             } : null;
         });

js/id/ui/save.js

@@ -22,7 +22,7 @@ iD.ui.save = function() {
                         history.reset();
                         map.flush().redraw();
                         if (err) {
-                            var desc = iD.confirm()
+                            var desc = iD.ui.confirm()
                                 .select('.description');
                             desc.append('h2')
                                 .text('An error occurred while trying to save');
@@ -42,12 +42,8 @@ iD.ui.save = function() {
                         }
                     });
                 }
-                var changes = history.changes();
-                var has_changes = d3.sum(d3.values(changes).map(function(c) {
-                    return c.length;
-                })) > 0;
 
-                if (has_changes) {
+                if (history.hasChanges()) {
                     connection.authenticate(function(err) {
                         var modal = iD.ui.modal();
                         var changes = history.changes();
@@ -68,7 +64,7 @@ iD.ui.save = function() {
                                 .on('save', commit));
                     });
                 } else {
-                    iD.confirm().select('.description')
+                    iD.ui.confirm().select('.description')
                         .append('h3').text('You don\'t have any changes to save.');
                 }
             });
@@ -77,16 +73,13 @@ iD.ui.save = function() {
             .attr('class', 'count');
 
         history.on('change.save-button', function() {
-            var changes = history.changes(),
-                num_changes = d3.sum(d3.values(changes).map(function(c) {
-                    return c.length;
-                }));
+            var hasChanges = history.hasChanges();
 
             selection
-                .property('disabled', num_changes === 0)
-                .classed('has-count', num_changes > 0)
+                .property('disabled', !hasChanges)
+                .classed('has-count', hasChanges)
                 .select('span.count')
-                    .text(num_changes);
+                    .text(history.numChanges());
         });
 
     }

test/spec/graph/history.js

@@ -126,6 +126,34 @@ describe("iD.History", function () {
         });
     });
 
+    describe("#hasChanges", function() {
+        it("is true when any of change's values are nonempty", function() {
+            var node = iD.Node();
+            history.perform(function (graph) { return graph.replace(node); });
+            expect(history.hasChanges()).to.eql(true);
+        });
+
+        it("is false when all of change's values are empty", function() {
+            expect(history.hasChanges()).to.eql(false);
+        });
+    });
+
+    describe("#numChanges", function() {
+        it("is 0 when there are no changes", function() {
+            expect(history.numChanges()).to.eql(0);
+        });
+
+        it("is the sum of all types of changes", function() {
+            var node1 = iD.Node({id: "n1"}),
+                node2 = iD.Node();
+            history.merge(iD.Graph([node1]));
+            history.perform(function (graph) { return graph.remove(node1); });
+            expect(history.numChanges()).to.eql(1);
+            history.perform(function (graph) { return graph.replace(node2); });
+            expect(history.numChanges()).to.eql(2);
+        });
+    });
+
     describe("#reset", function () {
         it("clears the version stack", function () {
             history.perform(action, "annotation");