From ded802049abeda14acd1b4657595f14820489dcf Mon Sep 17 00:00:00 2001
From: Brandon Liu <bdon@bdon.org>
Date: Sat, 19 Jan 2013 22:53:00 -0800
Subject: [PATCH] add history.numChanges method.

Use hasChanges/numChanges to toggle Save button and show confirmation dialog when navigating away from page.
---
 js/id/graph/history.js     |  8 ++++++--
 js/id/id.js                |  8 +-------
 js/id/ui/save.js           | 17 +++++------------
 test/spec/graph/history.js | 18 +++++++++++++++++-
 4 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/js/id/graph/history.js b/js/id/graph/history.js
index bf7cdfc88..3d027d042 100644
--- a/js/id/graph/history.js
+++ b/js/id/graph/history.js
@@ -110,9 +110,13 @@ iD.History = function() {
         },
 
         hasChanges: function() {
-            return !!d3.sum(d3.values(this.changes()).map(function(c) {
+            return !!this.numChanges();
+        },
+
+        numChanges: function() {
+            return d3.sum(d3.values(this.changes()).map(function(c) {
                 return c.length;
-            }));;
+            }));
         },
 
         imagery_used: function(source) {
diff --git a/js/id/id.js b/js/id/id.js
index ae9cadaa3..7a6fc6665 100644
--- a/js/id/id.js
+++ b/js/id/id.js
@@ -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;
         });
diff --git a/js/id/ui/save.js b/js/id/ui/save.js
index 821cc96e8..afaab5023 100644
--- a/js/id/ui/save.js
+++ b/js/id/ui/save.js
@@ -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();
@@ -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());
         });
 
     }
diff --git a/test/spec/graph/history.js b/test/spec/graph/history.js
index c3fa536f1..554b22a82 100644
--- a/test/spec/graph/history.js
+++ b/test/spec/graph/history.js
@@ -133,11 +133,27 @@ describe("iD.History", function () {
             expect(history.hasChanges()).to.eql(true);
         });
 
-        it("is false when any of change's values are empty", function() {
+        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");