From 59d37125df545e9a26b0b7c45d9c81ca82475ab0 Mon Sep 17 00:00:00 2001 From: Brandon Liu Date: Sat, 19 Jan 2013 22:54:09 -0800 Subject: [PATCH 1/4] make notice not a global var. --- js/id/id.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/id/id.js b/js/id/id.js index ff85a68d4..ae9cadaa3 100644 --- a/js/id/id.js +++ b/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')); From 3b722ed5030f6e814b1ca3887e1bbefb5614770e Mon Sep 17 00:00:00 2001 From: Brandon Liu Date: Sat, 19 Jan 2013 22:57:37 -0800 Subject: [PATCH 2/4] Properly refer to iD.ui.confirm --- js/id/ui/save.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/id/ui/save.js b/js/id/ui/save.js index e711e4bde..821cc96e8 100644 --- a/js/id/ui/save.js +++ b/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'); @@ -68,7 +68,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.'); } }); From d83ec7f1eb4862b58b62022da76d9d809c3996ac Mon Sep 17 00:00:00 2001 From: Brandon Liu Date: Sat, 19 Jan 2013 22:29:11 -0800 Subject: [PATCH 3/4] Create method history.hasChanges --- js/id/graph/history.js | 6 ++++++ test/spec/graph/history.js | 12 ++++++++++++ 2 files changed, 18 insertions(+) diff --git a/js/id/graph/history.js b/js/id/graph/history.js index e31fa5ef9..bf7cdfc88 100644 --- a/js/id/graph/history.js +++ b/js/id/graph/history.js @@ -109,6 +109,12 @@ iD.History = function() { }; }, + hasChanges: 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( diff --git a/test/spec/graph/history.js b/test/spec/graph/history.js index 15fe10964..c3fa536f1 100644 --- a/test/spec/graph/history.js +++ b/test/spec/graph/history.js @@ -126,6 +126,18 @@ 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 any of change's values are empty", function() { + expect(history.hasChanges()).to.eql(false); + }); + }); + describe("#reset", function () { it("clears the version stack", function () { history.perform(action, "annotation"); From ded802049abeda14acd1b4657595f14820489dcf Mon Sep 17 00:00:00 2001 From: Brandon Liu Date: Sat, 19 Jan 2013 22:53:00 -0800 Subject: [PATCH 4/4] 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");