Update save commit dialog to use a feature list

js/id/core/history.js

@@ -41,6 +41,10 @@ iD.History = function(context) {
             return stack[index].graph;
         },
 
+        base: function() {
+            return stack[0].graph;
+        },
+
         merge: function(entities, extent) {
 
             var base = stack[0].graph.base(),

js/id/ui/commit.js

@@ -3,12 +3,14 @@ iD.ui.Commit = function(context) {
         presets = context.presets();
 
     function commit(selection) {
-        var changes = context.history().changes(),
-            allChanges = _.flatten(d3.values(changes));
+        var changes = context.history().changes();
 
-        function zoomToEntity(entity) {
-            context.map().zoomTo(entity);
-            context.enter(iD.modes.Select(context, [entity.id]));
+        function zoomToEntity(change) {
+            var entity = change.entity;
+            if (change.changeType !== 'deleted') {
+                context.map().zoomTo(entity);
+                context.enter(iD.modes.Select(context, [entity.id]));
+            }
         }
 
         var header = selection.append('div')
@@ -114,7 +116,7 @@ iD.ui.Commit = function(context) {
 
         var entity = entityList.selectAll('.feature-list-item')
             .data(function() {
-                return iD.util.relevantChanges(context.graph(), allChanges);
+                return iD.util.relevantChanges(context.graph(), changes, context.history().base());
             });
 
         var entityEnter = entity.enter().append('button')
@@ -127,15 +129,28 @@ iD.ui.Commit = function(context) {
             .attr('class', 'label');
 
         label.append('span')
-            .attr('class', function(d) { return context.geometry(d.id) + ' icon icon-pre-text'; });
+            .attr('class', function(d) {
+                return d.entity.geometry(context.graph()) + ' icon icon-pre-text';
+        });
+
+        label.append('span')
+            .attr('class', 'entity-change-type')
+            .text(function(d) {
+                // need to determine if we're doing some kind of changetype icon or text
+                // + or - icon? red/green/yellow tinted geometry type icons?
+                // for deleted: maybe cross out (like no smoking signs) the same geometry icon
+                return d.changeType + ' ';
+            });
 
         label.append('span')
             .attr('class', 'entity-type')
-            .text(function(d) { return context.geometry(d.id); });
+            .text(function(d) {
+                return context.presets().match(d.entity, context.graph()).name();
+            });
 
         label.append('span')
             .attr('class', 'entity-name')
-            .text(function(d) { return iD.util.displayName(d); });
+            .text(function(d) { return iD.util.displayName(d.entity) || ''; });
 
         entityEnter.style('opacity', 0)
             .transition()
@@ -147,7 +162,6 @@ iD.ui.Commit = function(context) {
 
         function mouseover(d) {
             if (d.entity) {
-                // need to get the extent for this entity some how
                 context.surface().selectAll(iD.util.entityOrMemberSelector([d.entity.id], context.graph()))
                     .classed('hover', true);
             }
@@ -159,11 +173,15 @@ iD.ui.Commit = function(context) {
         }
 
         function warningClick(d) {
-            if (d.entity) {
-                context.enter(iD.modes.Select(context, [d.entity.id]));
-            }
+            if (d.entity) context.enter(iD.modes.Select(context, [d.entity.id]));
         }
     }
 
     return d3.rebind(commit, event, 'on');
 };
+
+// TODO:
+// indicate changetype
+// indicate changed geo/tags
+// check for and indicate if entity is a member of a multipolygon
+    // there's probably something somewhere for doing that

js/id/ui/selection_list.js

@@ -46,7 +46,7 @@ iD.ui.SelectionList = function(context, selectedIDs) {
             var items = list.selectAll('.feature-list-item')
                 .data(results, function(d) { return d.id; });
 
-            var enter = items.enter().insert('button', '.geocode-item')
+            var enter = items.enter().append('button')
                 .attr('class', 'feature-list-item')
                 .on('mouseover', mouseover)
                 .on('mouseout', mouseout)

js/id/util/relevant_changes.js

@@ -1,18 +1,35 @@
 // filters out verticies where the parent entity is already present
 // for simpler changeset listing
-iD.util.relevantChanges = function(graph, entities) {
+iD.util.relevantChanges = function(graph, changes, base) {
     var relevant = {};
-    for (var i = entities.length - 1; i >= 0; i--) {
-        var entity = entities[i];
-        if (entity.geometry(graph) === 'vertex') {
-            var parents = graph.parentWays(entity);
-            for (var j = parents.length - 1; j >= 0; j--) {
-                var parent = parents[j];
-                relevant[parent.id] = parent;
-            }
-        } else {
-            relevant[entity.id] = entity;
+
+    function addEntity(entity, changeType) {
+        relevant[entity.id] = {
+            entity: entity,
+            changeType: changeType
+        };
+    }
+
+    function addParents(entity) {
+        var parents = graph.parentWays(entity);
+        for (var j = parents.length - 1; j >= 0; j--) {
+            var parent = parents[j];
+            if (!(parent.id in relevant)) addEntity(parent, 'modified');
         }
     }
+
+    _.each(changes, function(entities, change) {
+        _.each(entities, function(entity) {
+            if (entity.geometry(change === 'deleted' ? base : graph) === 'vertex') {
+                addParents(entity);
+                if (change === 'modified' && (entity.tags !== base.entity(entity.id).tags)) {
+                    addEntity(entity, change);
+                }
+            } else {
+                addEntity(entity, change);
+            }
+        });
+    });
+
     return d3.values(relevant);
 };

test/spec/util/relevant_changes.js

@@ -1,5 +1,5 @@
 describe("iD.util.relevantChanges", function() {
-    var graph = iD.Graph({
+    var base = iD.Graph({
         'a': iD.Node({id: 'a', loc: [0, 0]}),
         'b': iD.Node({id: 'b', loc: [2, 0]}),
         'c': iD.Node({id: 'c', loc: [2, 2]}),
@@ -9,21 +9,88 @@ describe("iD.util.relevantChanges", function() {
     });
 
     it("returns a way that changed", function() {
-        var entities = [graph.entity('-')],
-            a = iD.util.relevantChanges(graph, entities);
-        expect(a).to.eql(entities);
+        var way = iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'a']}),
+            graph = base.replace(way),
+            changes = { modified: [way] },
+            a = iD.util.relevantChanges(graph, changes, base);
+        expect(a).to.eql([{
+            changeType: 'modified',
+            entity: way
+        }]);
     });
 
-    it("just returns the way that changed, leaving out the verticies", function() {
-        var entities = [
-            graph.entity('a'),
-            graph.entity('b'),
-            graph.entity('c'),
-            graph.entity('d'),
-            graph.entity('e'),
-            graph.entity('-')],
-            a = iD.util.relevantChanges(graph, entities),
-            way = [graph.entity('-')];
-        expect(a).to.eql(way);
+    it("reports an existing modified way, leaving out the verticies", function() {
+        var way = iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'e', 'a']}),
+            vertex = iD.Node({id: 'e', loc: [0, 3]}),
+            graph = base.replace(way).replace(vertex),
+            changes = { modified: [way, vertex] },
+            a = iD.util.relevantChanges(graph, changes, base);
+        expect(a).to.eql([{
+            changeType: 'modified',
+            entity: way
+        }]);
+    });
+
+    it("reports an existing way as modified when a member vertex is modified", function() {
+        var vertex = base.entity('e').move([0,3]),
+            graph = base.replace(vertex),
+            changes = { modified: [vertex], deleted: [] },
+            a = iD.util.relevantChanges(graph, changes, base);
+        expect(a).to.eql([{
+            changeType: 'modified',
+            entity: graph.entity('-')
+        }]);
+    });
+
+    it("reports a created way containing a moved vertex as being created", function() {
+        var vertex = base.entity('e').move([0,3]),
+            way = iD.Way({id: '+', nodes: ['e']}),
+            graph = base.replace(way).replace(vertex),
+            changes = { created: [way], modified: [vertex, graph.entity('-')] },
+            a = iD.util.relevantChanges(graph, changes, base);
+        expect(a).to.eql([{
+            changeType: 'created',
+            entity: way
+        }, {
+            changeType: 'modified',
+            entity: graph.entity('-')
+        }]);
+    });
+
+    it("reports an existing way with an added vertex as being modified", function() {
+        var vertex = iD.Node({id: 'f'}),
+            graph = base.replace(vertex).replace(base.entity('-').addNode('f'));
+            var changes = {
+                created: [vertex],
+                modified: [graph.entity('-')]
+            },
+            a = iD.util.relevantChanges(graph, changes, base);
+        expect(a).to.eql([{
+            changeType: 'modified',
+            entity: graph.entity('-')
+        }]);
+    });
+
+    it("reports a created way with a created vertex as being created", function() {
+        var vertex = iD.Node({id: 'f'}),
+            way = iD.Way({id: '+', nodes: ['f']}),
+            graph = base.replace(vertex).replace(way),
+            changes = { created: [way, vertex] },
+            a = iD.util.relevantChanges(graph, changes, base);
+        expect(a).to.eql([{
+            changeType: 'created',
+            entity: way
+        }]);
+    });
+
+    it("reports an existing vertex with added tags as modified", function() {
+        var vertex = iD.Node({id: 'f', tags: {yes: 'it works'}}),
+            graph = base.replace(vertex),
+            changes = { modified: [vertex] },
+            a = iD.util.relevantChanges(graph, changes, base);
+        expect(a).to.eql([{
+            changeType: 'modified',
+            entity: vertex
+        }]);
     });
 });