From b74ba194f297f2bf95640c519ea81bae7f1a2d6b Mon Sep 17 00:00:00 2001 From: Aaron Lidman Date: Wed, 16 Oct 2013 12:43:25 -0400 Subject: [PATCH 01/19] Added ability to zoom to changeset list items --- css/app.css | 4 ++++ js/id/modes/save.js | 6 ------ js/id/ui/commit.js | 47 ++++++++++++++++----------------------------- 3 files changed, 21 insertions(+), 36 deletions(-) diff --git a/css/app.css b/css/app.css index 609ced9f3..11475c7ac 100644 --- a/css/app.css +++ b/css/app.css @@ -2449,6 +2449,10 @@ img.wiki-image { color:#555; } +.mode-save .commit-section .changeset-list button { + border-left: 1px solid #CCC; +} + .changeset-list li span.count:before { content: '('; } .changeset-list li span.count:after { content: ')'; } diff --git a/js/id/modes/save.js b/js/id/modes/save.js index daaaafed2..80d78db4d 100644 --- a/js/id/modes/save.js +++ b/js/id/modes/save.js @@ -1,18 +1,12 @@ iD.modes.Save = function(context) { var ui = iD.ui.Commit(context) .on('cancel', cancel) - .on('fix', fix) .on('save', save); function cancel() { context.enter(iD.modes.Browse(context)); } - function fix(d) { - context.map().zoomTo(d.entity); - context.enter(iD.modes.Select(context, [d.entity.id])); - } - function save(e) { var loading = iD.ui.Loading(context) .message(t('save.uploading')) diff --git a/js/id/ui/commit.js b/js/id/ui/commit.js index 0968cabda..49b8b649f 100644 --- a/js/id/ui/commit.js +++ b/js/id/ui/commit.js @@ -1,32 +1,17 @@ iD.ui.Commit = function(context) { - var event = d3.dispatch('cancel', 'save', 'fix'), + var event = d3.dispatch('cancel', 'save'), presets = context.presets(); - function zipSame(d) { - var c = {}, n = -1; - for (var i = 0; i < d.length; i++) { - var desc = { - name: d[i].tags.name || presets.match(d[i], context.graph()).name(), - geometry: d[i].geometry(context.graph()), - count: 1, - tagText: iD.util.tagText(d[i]) - }; - - var fingerprint = desc.name + desc.tagText; - if (c[fingerprint]) { - c[fingerprint].count++; - } else { - c[fingerprint] = desc; - } - } - return _.values(c); - } - function commit(selection) { var changes = context.history().changes(); function changesLength(d) { return changes[d].length; } + function zoomToEntity(entity) { + context.map().zoomTo(entity); + context.enter(iD.modes.Select(context, [entity.id])); + } + var header = selection.append('div') .attr('class', 'header fillL'); @@ -121,7 +106,9 @@ iD.ui.Commit = function(context) { warningLi.filter(function(d) { return d.entity; }) .append('button') .attr('class', 'minor') - .on('click', event.fix) + .on('click', function(d) { + zoomToEntity(d.entity); + }) .append('span') .attr('class', 'icon warning'); @@ -144,23 +131,23 @@ iD.ui.Commit = function(context) { var li = section.append('ul') .attr('class', 'changeset-list') .selectAll('li') - .data(function(d) { return zipSame(changes[d]); }) + .data(function(d) { return changes[d]; }) .enter() .append('li'); li.append('strong') - .text(function(d) { - return d.geometry + ' '; + .text(function(entity) { + return entity.geometry(context.graph()) + ' '; }); li.append('span') - .text(function(d) { return d.name; }) - .attr('title', function(d) { return d.tagText; }); + .text(function(entity) { return iD.util.displayName(entity); }); - li.filter(function(d) { return d.count > 1; }) + li.append('button') + .attr('class', 'minor') + .on('click', zoomToEntity) .append('span') - .attr('class', 'count') - .text(function(d) { return d.count; }); + .attr('class', 'icon warning'); } return d3.rebind(commit, event, 'on'); From 1ae060e35fe0c6305a2c70a2925c73faeacb2300 Mon Sep 17 00:00:00 2001 From: Aaron Lidman Date: Wed, 16 Oct 2013 15:23:20 -0400 Subject: [PATCH 02/19] Excluding verticies where appropriate on save sidebar --- index.html | 1 + js/id/ui/commit.js | 2 +- js/id/util/relevant_changes.js | 18 ++++++++++++++++++ test/index.html | 2 ++ test/index_packaged.html | 1 + test/spec/util/relevant_changes.js | 29 +++++++++++++++++++++++++++++ 6 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 js/id/util/relevant_changes.js create mode 100644 test/spec/util/relevant_changes.js diff --git a/index.html b/index.html index c88bace38..dae31a0fd 100644 --- a/index.html +++ b/index.html @@ -37,6 +37,7 @@ + diff --git a/js/id/ui/commit.js b/js/id/ui/commit.js index 49b8b649f..c9b02a875 100644 --- a/js/id/ui/commit.js +++ b/js/id/ui/commit.js @@ -131,7 +131,7 @@ iD.ui.Commit = function(context) { var li = section.append('ul') .attr('class', 'changeset-list') .selectAll('li') - .data(function(d) { return changes[d]; }) + .data(function(d) { return iD.util.relevantChanges(context.graph(), changes[d]); }) .enter() .append('li'); diff --git a/js/id/util/relevant_changes.js b/js/id/util/relevant_changes.js new file mode 100644 index 000000000..c2e4bce80 --- /dev/null +++ b/js/id/util/relevant_changes.js @@ -0,0 +1,18 @@ +// filters out verticies where the parent entity is already present +// for simpler changeset listing +iD.util.relevantChanges = function(graph, entities) { + 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; + } + } + return d3.values(relevant); +}; diff --git a/test/index.html b/test/index.html index 14fb4087e..170cc5ea4 100644 --- a/test/index.html +++ b/test/index.html @@ -192,6 +192,7 @@ + @@ -273,6 +274,7 @@ + diff --git a/test/index_packaged.html b/test/index_packaged.html index 356e4b687..650a0b886 100644 --- a/test/index_packaged.html +++ b/test/index_packaged.html @@ -91,6 +91,7 @@ + diff --git a/test/spec/util/relevant_changes.js b/test/spec/util/relevant_changes.js new file mode 100644 index 000000000..2a5432bd9 --- /dev/null +++ b/test/spec/util/relevant_changes.js @@ -0,0 +1,29 @@ +describe("iD.util.relevantChanges", function() { + var graph = 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]}), + 'd': iD.Node({id: 'd', loc: [0, 2]}), + 'e': iD.Node({id: 'e', loc: [0, 2]}), + '-': iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'e', 'a']}) + }); + + it("returns a way that changed", function() { + var entities = [graph.entity('-')], + a = iD.util.relevantChanges(graph, entities); + expect(a).to.eql(entities); + }); + + 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); + }); +}); From fe8042131db4d71a19b5981cd27c8a535950b044 Mon Sep 17 00:00:00 2001 From: Aaron Lidman Date: Wed, 16 Oct 2013 18:23:23 -0400 Subject: [PATCH 03/19] Changed save commit summary to a feature list --- js/id/ui/commit.js | 125 +++++++++++++++++++++++++-------------------- js/id/validate.js | 2 +- 2 files changed, 71 insertions(+), 56 deletions(-) diff --git a/js/id/ui/commit.js b/js/id/ui/commit.js index c9b02a875..b0b2165ea 100644 --- a/js/id/ui/commit.js +++ b/js/id/ui/commit.js @@ -3,9 +3,8 @@ iD.ui.Commit = function(context) { presets = context.presets(); function commit(selection) { - var changes = context.history().changes(); - - function changesLength(d) { return changes[d].length; } + var changes = context.history().changes(), + allChanges = _.flatten(d3.values(changes)); function zoomToEntity(entity) { context.map().zoomTo(entity); @@ -86,68 +85,84 @@ iD.ui.Commit = function(context) { .attr('class', 'label') .text(t('commit.save')); - var warnings = body.selectAll('div.warning-section') - .data(iD.validate(changes, context.graph())) - .enter() - .append('div') - .attr('class', 'modal-section warning-section fillL2'); + // Warnings + var warningList = body.append('div') + .attr('class', 'feature-list cf'); - warnings.append('h3') - .text(t('commit.warnings')); + var warning = warningList.selectAll('.feature-list-item') + .data(iD.validate(changes, context.graph())); - var warningLi = warnings.append('ul') - .attr('class', 'changeset-list') - .selectAll('li') - .data(function(d) { return d; }) - .enter() - .append('li'); + var warningEnter = warning.enter().append('button') + .attr('class', 'feature-list-item') + .on('mouseover', mouseover) + .on('mouseout', mouseout) + .on('click', warningClick); - // only show the fix icon when an entity is given - warningLi.filter(function(d) { return d.entity; }) - .append('button') - .attr('class', 'minor') - .on('click', function(d) { - zoomToEntity(d.entity); - }) - .append('span') - .attr('class', 'icon warning'); + var label = warningEnter.append('div') + .attr('class', 'label'); - warningLi.append('strong').text(function(d) { - return d.message; - }); + label.append('span') + .attr('class', 'alert icon icon-pre-text'); - var section = body.selectAll('div.commit-section') - .data(['modified', 'deleted', 'created'].filter(changesLength)) - .enter() - .append('div') - .attr('class', 'commit-section modal-section fillL2'); + label.append('span') + .attr('class', 'entity-type strong') + .text(function(d) { return d.message; }); - section.append('h3') - .text(function(d) { return t('commit.' + d); }) - .append('small') - .attr('class', 'count') - .text(changesLength); + // Entities + var entityList = body.append('div') + .attr('class', 'feature-list cf'); - var li = section.append('ul') - .attr('class', 'changeset-list') - .selectAll('li') - .data(function(d) { return iD.util.relevantChanges(context.graph(), changes[d]); }) - .enter() - .append('li'); - - li.append('strong') - .text(function(entity) { - return entity.geometry(context.graph()) + ' '; + var entity = entityList.selectAll('.feature-list-item') + .data(function() { + return iD.util.relevantChanges(context.graph(), allChanges); }); - li.append('span') - .text(function(entity) { return iD.util.displayName(entity); }); + var entityEnter = entity.enter().append('button') + .attr('class', 'feature-list-item') + .on('mouseover', mouseover) + .on('mouseout', mouseout) + .on('click', zoomToEntity); - li.append('button') - .attr('class', 'minor') - .on('click', zoomToEntity) - .append('span') - .attr('class', 'icon warning'); + label = entityEnter.append('div') + .attr('class', 'label'); + + label.append('span') + .attr('class', function(d) { return context.geometry(d.id) + ' icon icon-pre-text'; }); + + label.append('span') + .attr('class', 'entity-type') + .text(function(d) { return context.geometry(d.id); }); + + label.append('span') + .attr('class', 'entity-name') + .text(function(d) { return iD.util.displayName(d); }); + + entityEnter.style('opacity', 0) + .transition() + .style('opacity', 1); + + warningEnter.style('opacity', 0) + .transition() + .style('opacity', 1); + + 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); + } + } + + function mouseout() { + context.surface().selectAll('.hover') + .classed('hover', false); + } + + function warningClick(d) { + if (d.entity) { + context.enter(iD.modes.Select(context, [d.entity.id])); + } + } } return d3.rebind(commit, event, 'on'); diff --git a/js/id/validate.js b/js/id/validate.js index c267f8675..19bb18917 100644 --- a/js/id/validate.js +++ b/js/id/validate.js @@ -48,5 +48,5 @@ iD.validate = function(changes, graph) { } } - return warnings.length ? [warnings] : []; + return warnings; }; From 594baa18b36444e7448f44f4274b67bf47d75c78 Mon Sep 17 00:00:00 2001 From: Aaron Lidman Date: Fri, 18 Oct 2013 17:11:52 -0400 Subject: [PATCH 04/19] Update save commit dialog to use a feature list --- js/id/core/history.js | 4 ++ js/id/ui/commit.js | 44 ++++++++++---- js/id/ui/selection_list.js | 2 +- js/id/util/relevant_changes.js | 39 ++++++++---- test/spec/util/relevant_changes.js | 97 +++++++++++++++++++++++++----- 5 files changed, 146 insertions(+), 40 deletions(-) diff --git a/js/id/core/history.js b/js/id/core/history.js index 98f27a6d7..dbc5aaa2a 100644 --- a/js/id/core/history.js +++ b/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(), diff --git a/js/id/ui/commit.js b/js/id/ui/commit.js index b0b2165ea..a87eb674e 100644 --- a/js/id/ui/commit.js +++ b/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 diff --git a/js/id/ui/selection_list.js b/js/id/ui/selection_list.js index 59551cde7..054fdb7d2 100644 --- a/js/id/ui/selection_list.js +++ b/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) diff --git a/js/id/util/relevant_changes.js b/js/id/util/relevant_changes.js index c2e4bce80..d498124ab 100644 --- a/js/id/util/relevant_changes.js +++ b/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); }; diff --git a/test/spec/util/relevant_changes.js b/test/spec/util/relevant_changes.js index 2a5432bd9..2182f63dd 100644 --- a/test/spec/util/relevant_changes.js +++ b/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 + }]); }); }); From 1eb4c6ce7864f5233868513e1d8e1ee803bd1c0b Mon Sep 17 00:00:00 2001 From: Aaron Lidman Date: Mon, 21 Oct 2013 14:00:47 -0400 Subject: [PATCH 05/19] Added more distinction to changeset list items --- css/app.css | 11 +++- js/id/ui/commit.js | 123 +++++++++++++++++++++++++++++---------------- 2 files changed, 89 insertions(+), 45 deletions(-) diff --git a/css/app.css b/css/app.css index 11475c7ac..cb5adb9e8 100644 --- a/css/app.css +++ b/css/app.css @@ -2431,7 +2431,7 @@ img.wiki-image { border:1px solid #ccc; border-radius: 4px; background:#fff; - max-height: 160px; + max-height: 400px; } .mode-save .warning-section .changeset-list button { @@ -2442,6 +2442,15 @@ img.wiki-image { position: relative; border-top:1px solid #ccc; padding:5px 10px; + cursor: pointer; +} + +.mode-save .changeset-list li:hover { + background-color: #ececec; +} + +.mode-save .changeset-list .icon { + opacity: 0.5; } .changeset-list li span.count { diff --git a/js/id/ui/commit.js b/js/id/ui/commit.js index a87eb674e..25cc60cf3 100644 --- a/js/id/ui/commit.js +++ b/js/id/ui/commit.js @@ -3,13 +3,22 @@ iD.ui.Commit = function(context) { presets = context.presets(); function commit(selection) { - var changes = context.history().changes(); + var changes = context.history().changes(), + relevantChanges = iD.util.relevantChanges( + context.graph(), + changes, + context.history().base() + ); function zoomToEntity(change) { + // need to filter out verticies, they aren't/can't be highlighted? var entity = change.entity; if (change.changeType !== 'deleted') { context.map().zoomTo(entity); - context.enter(iD.modes.Select(context, [entity.id])); + // context.enter(iD.modes.Select(context, [entity.id])); + context.surface().selectAll( + iD.util.entityOrMemberSelector([entity.id], context.graph())) + .classed('hover', true); } } @@ -88,82 +97,107 @@ iD.ui.Commit = function(context) { .text(t('commit.save')); // Warnings - var warningList = body.append('div') - .attr('class', 'feature-list cf'); + var warnings = body.selectAll('div.warning-section') + .data([iD.validate(changes, context.graph())]) + .enter() + .append('div') + .attr('class', 'modal-section warning-section fillL2'); - var warning = warningList.selectAll('.feature-list-item') - .data(iD.validate(changes, context.graph())); + warnings.append('h3') + .text(t('commit.warnings')); - var warningEnter = warning.enter().append('button') - .attr('class', 'feature-list-item') + var warningLi = warnings.append('ul') + .attr('class', 'changeset-list') + .selectAll('li') + .data(function(d) { return d; }) + .enter() + .append('li') .on('mouseover', mouseover) .on('mouseout', mouseout) .on('click', warningClick); - var label = warningEnter.append('div') - .attr('class', 'label'); - - label.append('span') + warningLi.append('span') .attr('class', 'alert icon icon-pre-text'); - label.append('span') - .attr('class', 'entity-type strong') - .text(function(d) { return d.message; }); + warningLi.append('strong').text(function(d) { + return d.message; + }); - // Entities - var entityList = body.append('div') - .attr('class', 'feature-list cf'); + // icon or no icon? + // warningLi.filter(function(d) { return d.entity; }) + // .append('button') + // .attr('class', 'minor') + // .on('click', event.fix) + // .append('span') + // .attr('class', 'icon warning'); - var entity = entityList.selectAll('.feature-list-item') - .data(function() { - return iD.util.relevantChanges(context.graph(), changes, context.history().base()); - }); + var changeSection = body.selectAll('div.commit-section') + .data([0]) + .enter() + .append('div') + .attr('class', 'commit-section modal-section fillL2'); - var entityEnter = entity.enter().append('button') - .attr('class', 'feature-list-item') + changeSection.append('h3') + .text('Changes') + .append('small') + .attr('class', 'count') + .text(relevantChanges.length); + + var li = changeSection.append('ul') + .attr('class', 'changeset-list') + .selectAll('li') + .data(function(d) { + return relevantChanges; + }) + .enter() + .append('li') .on('mouseover', mouseover) .on('mouseout', mouseout) .on('click', zoomToEntity); - label = entityEnter.append('div') - .attr('class', 'label'); + // icon or no icon? + // li.append('button') + // .attr('class', 'minor') + // .append('span') + // .attr('class', 'icon warning'); - label.append('span') + li.append('span') .attr('class', function(d) { - return d.entity.geometry(context.graph()) + ' icon icon-pre-text'; - }); + return context.geometry(d.entity.id) + ' icon icon-pre-text'; + }); - label.append('span') - .attr('class', 'entity-change-type') + // we want to change this to an icon/bg color/something else + li.append('span') + .attr('class', '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') + li.append('strong') .attr('class', 'entity-type') .text(function(d) { return context.presets().match(d.entity, context.graph()).name(); }); - label.append('span') + li.append('span') .attr('class', 'entity-name') - .text(function(d) { return iD.util.displayName(d.entity) || ''; }); + .text(function(d) { + return ' ' + (iD.util.displayName(d.entity) || ''); + }); - entityEnter.style('opacity', 0) + li.style('opacity', 0) .transition() .style('opacity', 1); - warningEnter.style('opacity', 0) + li.style('opacity', 0) .transition() .style('opacity', 1); function mouseover(d) { if (d.entity) { - context.surface().selectAll(iD.util.entityOrMemberSelector([d.entity.id], context.graph())) - .classed('hover', true); + context.surface().selectAll( + iD.util.entityOrMemberSelector([d.entity.id], context.graph()) + ).classed('hover', true); } } @@ -181,7 +215,8 @@ iD.ui.Commit = function(context) { }; // TODO: -// indicate changetype -// indicate changed geo/tags +// indicate changed geometry/tags // check for and indicate if entity is a member of a multipolygon - // there's probably something somewhere for doing that + // there's probably something somewhere for doing that, parent? +// deleted changeset items majorly BORK the list + // handle deleted items better in relevant-changes From 7ead26928de055bc031442fd90789f3a9afeb560 Mon Sep 17 00:00:00 2001 From: Aaron Lidman Date: Mon, 21 Oct 2013 14:45:43 -0400 Subject: [PATCH 06/19] Fixed references to deleted items --- js/id/ui/commit.js | 6 +++--- js/id/util/relevant_changes.js | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/js/id/ui/commit.js b/js/id/ui/commit.js index 25cc60cf3..327a27331 100644 --- a/js/id/ui/commit.js +++ b/js/id/ui/commit.js @@ -163,7 +163,7 @@ iD.ui.Commit = function(context) { li.append('span') .attr('class', function(d) { - return context.geometry(d.entity.id) + ' icon icon-pre-text'; + return d.geometryType + ' icon icon-pre-text'; }); // we want to change this to an icon/bg color/something else @@ -176,13 +176,13 @@ iD.ui.Commit = function(context) { li.append('strong') .attr('class', 'entity-type') .text(function(d) { - return context.presets().match(d.entity, context.graph()).name(); + return context.presets().match(d.entity, context.history().base()).name(); }); li.append('span') .attr('class', 'entity-name') .text(function(d) { - return ' ' + (iD.util.displayName(d.entity) || ''); + return ' ' + d.name; }); li.style('opacity', 0) diff --git a/js/id/util/relevant_changes.js b/js/id/util/relevant_changes.js index d498124ab..cffc9aefd 100644 --- a/js/id/util/relevant_changes.js +++ b/js/id/util/relevant_changes.js @@ -5,8 +5,10 @@ iD.util.relevantChanges = function(graph, changes, base) { function addEntity(entity, changeType) { relevant[entity.id] = { + name: iD.util.displayName(entity) || '', entity: entity, - changeType: changeType + changeType: changeType, + geometryType: base.entity(entity.id).geometry(base) }; } From 7b97a6116c7a5a15349f109573fc26e5038babed Mon Sep 17 00:00:00 2001 From: Aaron Lidman Date: Mon, 21 Oct 2013 15:41:24 -0400 Subject: [PATCH 07/19] Actually fixed references to deleted items --- js/id/ui/commit.js | 9 +++++---- js/id/util/relevant_changes.js | 12 ++++++------ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/js/id/ui/commit.js b/js/id/ui/commit.js index 327a27331..7cf0d86cf 100644 --- a/js/id/ui/commit.js +++ b/js/id/ui/commit.js @@ -4,10 +4,11 @@ iD.ui.Commit = function(context) { function commit(selection) { var changes = context.history().changes(), + base = context.history().base(), relevantChanges = iD.util.relevantChanges( context.graph(), changes, - context.history().base() + base ); function zoomToEntity(change) { @@ -163,7 +164,7 @@ iD.ui.Commit = function(context) { li.append('span') .attr('class', function(d) { - return d.geometryType + ' icon icon-pre-text'; + return base.entity(d.entity.id).geometry(base) + ' icon icon-pre-text'; }); // we want to change this to an icon/bg color/something else @@ -176,13 +177,13 @@ iD.ui.Commit = function(context) { li.append('strong') .attr('class', 'entity-type') .text(function(d) { - return context.presets().match(d.entity, context.history().base()).name(); + return context.presets().match(d.entity, base).name(); }); li.append('span') .attr('class', 'entity-name') .text(function(d) { - return ' ' + d.name; + return ' ' + (iD.util.displayName(d.entity) || ''); }); li.style('opacity', 0) diff --git a/js/id/util/relevant_changes.js b/js/id/util/relevant_changes.js index cffc9aefd..c289d2eeb 100644 --- a/js/id/util/relevant_changes.js +++ b/js/id/util/relevant_changes.js @@ -5,15 +5,14 @@ iD.util.relevantChanges = function(graph, changes, base) { function addEntity(entity, changeType) { relevant[entity.id] = { - name: iD.util.displayName(entity) || '', entity: entity, changeType: changeType, - geometryType: base.entity(entity.id).geometry(base) }; } - function addParents(entity) { - var parents = graph.parentWays(entity); + function addParents(entity, theGraph) { + if (!theGraph) theGraph = graph; + var parents = theGraph.parentWays(entity); for (var j = parents.length - 1; j >= 0; j--) { var parent = parents[j]; if (!(parent.id in relevant)) addEntity(parent, 'modified'); @@ -22,8 +21,9 @@ iD.util.relevantChanges = function(graph, changes, base) { _.each(changes, function(entities, change) { _.each(entities, function(entity) { - if (entity.geometry(change === 'deleted' ? base : graph) === 'vertex') { - addParents(entity); + var relevantGraph = change === 'deleted' ? base : graph; + if (entity.geometry(relevantGraph) === 'vertex') { + addParents(entity, relevantGraph); if (change === 'modified' && (entity.tags !== base.entity(entity.id).tags)) { addEntity(entity, change); } From 22b4e5c6b5157e43b50de9749d8ea7f34f7eb1f7 Mon Sep 17 00:00:00 2001 From: Aaron Lidman Date: Mon, 21 Oct 2013 19:38:58 -0400 Subject: [PATCH 08/19] Remove warning and changeset zoom icons and fixed warning problems --- css/app.css | 1 - js/id/ui/commit.js | 39 ++++++++++----------------------------- 2 files changed, 10 insertions(+), 30 deletions(-) diff --git a/css/app.css b/css/app.css index cb5adb9e8..94a60fc39 100644 --- a/css/app.css +++ b/css/app.css @@ -2431,7 +2431,6 @@ img.wiki-image { border:1px solid #ccc; border-radius: 4px; background:#fff; - max-height: 400px; } .mode-save .warning-section .changeset-list button { diff --git a/js/id/ui/commit.js b/js/id/ui/commit.js index 7cf0d86cf..bae030932 100644 --- a/js/id/ui/commit.js +++ b/js/id/ui/commit.js @@ -12,11 +12,10 @@ iD.ui.Commit = function(context) { ); function zoomToEntity(change) { - // need to filter out verticies, they aren't/can't be highlighted? var entity = change.entity; - if (change.changeType !== 'deleted') { + if (change.changeType !== 'deleted' && + context.graph().entity(entity.id).geometry(context.graph()) !== 'vertex') { context.map().zoomTo(entity); - // context.enter(iD.modes.Select(context, [entity.id])); context.surface().selectAll( iD.util.entityOrMemberSelector([entity.id], context.graph())) .classed('hover', true); @@ -99,7 +98,9 @@ iD.ui.Commit = function(context) { // Warnings var warnings = body.selectAll('div.warning-section') - .data([iD.validate(changes, context.graph())]) + .data(function() { + return iD.validate(changes, context.graph()); + }) .enter() .append('div') .attr('class', 'modal-section warning-section fillL2'); @@ -110,7 +111,7 @@ iD.ui.Commit = function(context) { var warningLi = warnings.append('ul') .attr('class', 'changeset-list') .selectAll('li') - .data(function(d) { return d; }) + .data(function(d) { if (d) return [d]; }) .enter() .append('li') .on('mouseover', mouseover) @@ -124,14 +125,6 @@ iD.ui.Commit = function(context) { return d.message; }); - // icon or no icon? - // warningLi.filter(function(d) { return d.entity; }) - // .append('button') - // .attr('class', 'minor') - // .on('click', event.fix) - // .append('span') - // .attr('class', 'icon warning'); - var changeSection = body.selectAll('div.commit-section') .data([0]) .enter() @@ -156,18 +149,12 @@ iD.ui.Commit = function(context) { .on('mouseout', mouseout) .on('click', zoomToEntity); - // icon or no icon? - // li.append('button') - // .attr('class', 'minor') - // .append('span') - // .attr('class', 'icon warning'); - li.append('span') .attr('class', function(d) { - return base.entity(d.entity.id).geometry(base) + ' icon icon-pre-text'; + var graph = d.changeType === 'deleted' ? base : context.graph(); + return graph.entity(d.entity.id).geometry(graph) + ' icon icon-pre-text'; }); - // we want to change this to an icon/bg color/something else li.append('span') .attr('class', 'change-type') .text(function(d) { @@ -177,7 +164,8 @@ iD.ui.Commit = function(context) { li.append('strong') .attr('class', 'entity-type') .text(function(d) { - return context.presets().match(d.entity, base).name(); + var graph = d.changeType === 'deleted' ? base : context.graph(); + return context.presets().match(d.entity, graph).name(); }); li.append('span') @@ -214,10 +202,3 @@ iD.ui.Commit = function(context) { return d3.rebind(commit, event, 'on'); }; - -// TODO: -// indicate changed geometry/tags -// check for and indicate if entity is a member of a multipolygon - // there's probably something somewhere for doing that, parent? -// deleted changeset items majorly BORK the list - // handle deleted items better in relevant-changes From b4d8e777b5afa4d5c766908e9e93a0e631e46f9e Mon Sep 17 00:00:00 2001 From: Aaron Lidman Date: Mon, 21 Oct 2013 19:46:05 -0400 Subject: [PATCH 09/19] Fix warnings --- js/id/ui/commit.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/js/id/ui/commit.js b/js/id/ui/commit.js index bae030932..9081e1147 100644 --- a/js/id/ui/commit.js +++ b/js/id/ui/commit.js @@ -98,9 +98,7 @@ iD.ui.Commit = function(context) { // Warnings var warnings = body.selectAll('div.warning-section') - .data(function() { - return iD.validate(changes, context.graph()); - }) + .data(iD.validate(changes, context.graph())) .enter() .append('div') .attr('class', 'modal-section warning-section fillL2'); @@ -110,9 +108,6 @@ iD.ui.Commit = function(context) { var warningLi = warnings.append('ul') .attr('class', 'changeset-list') - .selectAll('li') - .data(function(d) { if (d) return [d]; }) - .enter() .append('li') .on('mouseover', mouseover) .on('mouseout', mouseout) From 95e5852cbe38947622aa9b134efcde91b26dddf1 Mon Sep 17 00:00:00 2001 From: Aaron Lidman Date: Mon, 21 Oct 2013 20:24:04 -0400 Subject: [PATCH 10/19] Fixed the warnings --- js/id/ui/commit.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/js/id/ui/commit.js b/js/id/ui/commit.js index 9081e1147..700de9e91 100644 --- a/js/id/ui/commit.js +++ b/js/id/ui/commit.js @@ -98,16 +98,20 @@ iD.ui.Commit = function(context) { // Warnings var warnings = body.selectAll('div.warning-section') - .data(iD.validate(changes, context.graph())) + .data([iD.validate(changes, context.graph())]) .enter() .append('div') - .attr('class', 'modal-section warning-section fillL2'); + .attr('class', 'modal-section warning-section fillL2') + .style('display', function(d) { return _.isEmpty(d) ? 'none' : null; }); warnings.append('h3') .text(t('commit.warnings')); var warningLi = warnings.append('ul') .attr('class', 'changeset-list') + .selectAll('li') + .data(function(d) { return d; }) + .enter() .append('li') .on('mouseover', mouseover) .on('mouseout', mouseout) From 2dc46806a56d02b2b187e847d2864ca5cfa8e18c Mon Sep 17 00:00:00 2001 From: Aaron Lidman Date: Mon, 21 Oct 2013 21:01:04 -0400 Subject: [PATCH 11/19] Styled the save button a little --- css/app.css | 6 ++++++ js/id/ui/commit.js | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/css/app.css b/css/app.css index 94a60fc39..dba906fd0 100644 --- a/css/app.css +++ b/css/app.css @@ -2406,6 +2406,8 @@ img.wiki-image { float: none; margin: auto; display: block; + color: white; + font-size: 14px; } .mode-save .user-info img { @@ -2426,6 +2428,10 @@ img.wiki-image { color:#fff; } +.mode-save .commit-info { + margin-bottom: 10px; +} + .mode-save .changeset-list { overflow: auto; border:1px solid #ccc; diff --git a/js/id/ui/commit.js b/js/id/ui/commit.js index 700de9e91..0b2c9321a 100644 --- a/js/id/ui/commit.js +++ b/js/id/ui/commit.js @@ -85,7 +85,7 @@ iD.ui.Commit = function(context) { // Confirm Button var saveButton = saveSection.append('button') - .attr('class', 'action col3 button') + .attr('class', 'action col4 button') .on('click.save', function() { event.save({ comment: commentField.node().value From dc98c6fc50790583f7bbc7c1e2cf08bf7e0d4ebd Mon Sep 17 00:00:00 2001 From: Aaron Lidman Date: Mon, 21 Oct 2013 21:31:33 -0400 Subject: [PATCH 12/19] Add zoom to warning selection --- js/id/ui/commit.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/js/id/ui/commit.js b/js/id/ui/commit.js index 0b2c9321a..a9ea7ba0f 100644 --- a/js/id/ui/commit.js +++ b/js/id/ui/commit.js @@ -195,7 +195,10 @@ iD.ui.Commit = function(context) { } function warningClick(d) { - if (d.entity) context.enter(iD.modes.Select(context, [d.entity.id])); + if (d.entity) { + context.map().zoomTo(d.entity); + context.enter(iD.modes.Select(context, [d.entity.id])); + } } } From a0d3694f2946d1e445bbcb2de57b020ce924ca0c Mon Sep 17 00:00:00 2001 From: samanpwbb Date: Tue, 22 Oct 2013 11:20:55 -0400 Subject: [PATCH 13/19] adding deleted/added feature icons --- css/app.css | 12 ++++++++++-- dist/img/sprite.svg | 22 +++++++++++++++++----- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/css/app.css b/css/app.css index dba906fd0..33c4d8d50 100644 --- a/css/app.css +++ b/css/app.css @@ -261,7 +261,7 @@ ul li { list-style: none;} border-radius: 0 0 3px 3px; } -.toggle-list label > span { +.toggle-list label > span { display: block; overflow: hidden; white-space: nowrap; @@ -569,6 +569,14 @@ button[disabled] .icon.layers { background-position: -300px -40px;} button[disabled] .icon.avatar { background-position: -320px -40px;} button[disabled] .icon.nearby { background-position: -340px -40px;} +.icon.point.deleted { background-position: -300px -80px;} +.icon.line.deleted { background-position: -320px -80px;} +.icon.area.deleted { background-position: -340px -80px;} + +.icon.point.created { background-position: -300px -100px;} +.icon.line.created { background-position: -320px -100px;} +.icon.area.created { background-position: -340px -100px;} + /* Out link is special */ .icon.out-link { height: 14px; width: 14px; background-position: -500px 0;} @@ -2704,7 +2712,7 @@ img.wiki-image { } /* Move over tooltips that are near the edge of screen */ .add-point .tooltip { - left: 33.3333% !important; + left: 33.3333% !important; } .curtain-tooltip.intro-points-add .tooltip-arrow, diff --git a/dist/img/sprite.svg b/dist/img/sprite.svg index 411a53f0c..a869d1bd8 100644 --- a/dist/img/sprite.svg +++ b/dist/img/sprite.svg @@ -13,7 +13,7 @@ width="800" height="560" id="svg12393" - inkscape:version="0.48.4 r9939" + inkscape:version="0.48.2 r9819" sodipodi:docname="sprite.svg"> + + From df6a292a44ce5c26234ad837c1847227ecea4a70 Mon Sep 17 00:00:00 2001 From: Aaron Lidman Date: Tue, 22 Oct 2013 11:48:21 -0400 Subject: [PATCH 14/19] Add colored changeset icons --- css/app.css | 4 +++- js/id/ui/commit.js | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/css/app.css b/css/app.css index 33c4d8d50..cf0383a7a 100644 --- a/css/app.css +++ b/css/app.css @@ -577,6 +577,8 @@ button[disabled] .icon.nearby { background-position: -340px -40px;} .icon.line.created { background-position: -320px -100px;} .icon.area.created { background-position: -340px -100px;} +.icon.modified { opacity: .5; } + /* Out link is special */ .icon.out-link { height: 14px; width: 14px; background-position: -500px 0;} @@ -2462,7 +2464,7 @@ img.wiki-image { background-color: #ececec; } -.mode-save .changeset-list .icon { +.mode-save .changeset-list .alert { opacity: 0.5; } diff --git a/js/id/ui/commit.js b/js/id/ui/commit.js index a9ea7ba0f..fe2406b57 100644 --- a/js/id/ui/commit.js +++ b/js/id/ui/commit.js @@ -151,7 +151,8 @@ iD.ui.Commit = function(context) { li.append('span') .attr('class', function(d) { var graph = d.changeType === 'deleted' ? base : context.graph(); - return graph.entity(d.entity.id).geometry(graph) + ' icon icon-pre-text'; + return graph.entity(d.entity.id).geometry(graph) + ' ' + d.changeType + + ' icon icon-pre-text'; }); li.append('span') @@ -170,7 +171,10 @@ iD.ui.Commit = function(context) { li.append('span') .attr('class', 'entity-name') .text(function(d) { - return ' ' + (iD.util.displayName(d.entity) || ''); + var name = iD.util.displayName(d.entity) || '', + string = ''; + if (name !== '') string += ':'; + return string += ' ' + name; }); li.style('opacity', 0) From e8e9f7a4ffa3f36068bb089cbb85cc86c89a9d3d Mon Sep 17 00:00:00 2001 From: Aaron Lidman Date: Wed, 23 Oct 2013 11:42:33 -0400 Subject: [PATCH 15/19] Remove blue circle and icon adjustments --- css/app.css | 6 ++++-- js/id/ui/commit.js | 5 +---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/css/app.css b/css/app.css index cf0383a7a..1798f5985 100644 --- a/css/app.css +++ b/css/app.css @@ -569,14 +569,16 @@ button[disabled] .icon.layers { background-position: -300px -40px;} button[disabled] .icon.avatar { background-position: -320px -40px;} button[disabled] .icon.nearby { background-position: -340px -40px;} -.icon.point.deleted { background-position: -300px -80px;} +.icon.point.deleted { background-position: -302px -80px;} .icon.line.deleted { background-position: -320px -80px;} .icon.area.deleted { background-position: -340px -80px;} -.icon.point.created { background-position: -300px -100px;} +.icon.point.created { background-position: -302px -100px;} .icon.line.created { background-position: -320px -100px;} .icon.area.created { background-position: -340px -100px;} +.icon.point.modified { background-position: -24px 0; } + .icon.modified { opacity: .5; } /* Out link is special */ diff --git a/js/id/ui/commit.js b/js/id/ui/commit.js index fe2406b57..76794e486 100644 --- a/js/id/ui/commit.js +++ b/js/id/ui/commit.js @@ -131,10 +131,7 @@ iD.ui.Commit = function(context) { .attr('class', 'commit-section modal-section fillL2'); changeSection.append('h3') - .text('Changes') - .append('small') - .attr('class', 'count') - .text(relevantChanges.length); + .text(relevantChanges.length + ' Changes'); var li = changeSection.append('ul') .attr('class', 'changeset-list') From a05b9b1be53de2e738b5ceb71f11312a238a189d Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 23 Oct 2013 12:00:07 -0700 Subject: [PATCH 16/19] Report an existing tagged vertex that is moved as modified --- js/id/util/relevant_changes.js | 20 +++++++++++++------- test/spec/util/relevant_changes.js | 15 +++++++++++++-- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/js/id/util/relevant_changes.js b/js/id/util/relevant_changes.js index c289d2eeb..869ae27c5 100644 --- a/js/id/util/relevant_changes.js +++ b/js/id/util/relevant_changes.js @@ -6,7 +6,7 @@ iD.util.relevantChanges = function(graph, changes, base) { function addEntity(entity, changeType) { relevant[entity.id] = { entity: entity, - changeType: changeType, + changeType: changeType }; } @@ -21,14 +21,20 @@ iD.util.relevantChanges = function(graph, changes, base) { _.each(changes, function(entities, change) { _.each(entities, function(entity) { - var relevantGraph = change === 'deleted' ? base : graph; - if (entity.geometry(relevantGraph) === 'vertex') { - addParents(entity, relevantGraph); - if (change === 'modified' && (entity.tags !== base.entity(entity.id).tags)) { + if (entity.geometry(change === 'deleted' ? base : graph) !== 'vertex') { + addEntity(entity, change); + + } else if (change === 'modified') { + var moved = entity.loc !== base.entity(entity.id).loc, + retagged = entity.tags !== base.entity(entity.id).tags; + + if (moved) { + addParents(entity, graph); + } + + if (retagged || (moved && entity.hasInterestingTags())) { addEntity(entity, change); } - } else { - addEntity(entity, change); } }); }); diff --git a/test/spec/util/relevant_changes.js b/test/spec/util/relevant_changes.js index 2182f63dd..23ee5f631 100644 --- a/test/spec/util/relevant_changes.js +++ b/test/spec/util/relevant_changes.js @@ -1,6 +1,6 @@ describe("iD.util.relevantChanges", function() { var base = iD.Graph({ - 'a': iD.Node({id: 'a', loc: [0, 0]}), + 'a': iD.Node({id: 'a', loc: [0, 0], tags: {crossing: 'zebra'}}), 'b': iD.Node({id: 'b', loc: [2, 0]}), 'c': iD.Node({id: 'c', loc: [2, 2]}), 'd': iD.Node({id: 'd', loc: [0, 2]}), @@ -84,7 +84,7 @@ describe("iD.util.relevantChanges", function() { }); it("reports an existing vertex with added tags as modified", function() { - var vertex = iD.Node({id: 'f', tags: {yes: 'it works'}}), + var vertex = base.entity('a').mergeTags({highway: 'traffic_signals'}), graph = base.replace(vertex), changes = { modified: [vertex] }, a = iD.util.relevantChanges(graph, changes, base); @@ -93,4 +93,15 @@ describe("iD.util.relevantChanges", function() { entity: vertex }]); }); + + it("reports an existing tagged vertex that is moved as modified", function() { + var vertex = base.entity('a').move([1, 2]), + graph = base.replace(vertex), + changes = { modified: [vertex] }, + a = iD.util.relevantChanges(graph, changes, base); + expect(a[1]).to.eql({ + changeType: 'modified', + entity: vertex + }); + }); }); From b8e88a7aaf21ff61cac268493bb183335096c9ca Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 23 Oct 2013 13:20:57 -0700 Subject: [PATCH 17/19] Convert relevantChanges to a method on Difference --- index.html | 1 - js/id/core/difference.js | 44 +++++++++- js/id/core/history.js | 4 - js/id/ui/commit.js | 23 ++--- js/id/util/relevant_changes.js | 43 --------- test/index.html | 2 - test/index_packaged.html | 1 - test/spec/core/difference.js | 135 +++++++++++++++++++++++++++++ test/spec/util/relevant_changes.js | 107 ----------------------- 9 files changed, 184 insertions(+), 176 deletions(-) delete mode 100644 js/id/util/relevant_changes.js delete mode 100644 test/spec/util/relevant_changes.js diff --git a/index.html b/index.html index dae31a0fd..c88bace38 100644 --- a/index.html +++ b/index.html @@ -37,7 +37,6 @@ - diff --git a/js/id/core/difference.js b/js/id/core/difference.js index 9c1a80ca5..8b8ecbb3e 100644 --- a/js/id/core/difference.js +++ b/js/id/core/difference.js @@ -84,7 +84,6 @@ iD.Difference = function(base, head) { }; difference.addParents = function(entities) { - for (var i in entities) { addParents(head.parentWays(entities[i]), entities); addParents(head.parentRelations(entities[i]), entities); @@ -92,6 +91,49 @@ iD.Difference = function(base, head) { return entities; }; + difference.summary = function() { + var relevant = {}; + + function addEntity(entity, graph, changeType) { + relevant[entity.id] = { + entity: entity, + graph: graph, + changeType: changeType + }; + } + + function addParents(entity) { + var parents = head.parentWays(entity); + for (var j = parents.length - 1; j >= 0; j--) { + var parent = parents[j]; + if (!(parent.id in relevant)) addEntity(parent, head, 'modified'); + } + } + + _.each(changes, function(change) { + if (change.head && change.head.geometry(head) !== 'vertex') { + addEntity(change.head, head, change.base ? 'modified' : 'created'); + + } else if (change.base && change.base.geometry(base) !== 'vertex') { + addEntity(change.base, base, 'deleted'); + + } else if (change.base && change.head) { // modified vertex + var moved = change.base.loc !== change.head.loc, + retagged = change.base.tags !== change.head.tags; + + if (moved) { + addParents(change.head); + } + + if (retagged || (moved && change.head.hasInterestingTags())) { + addEntity(change.head, head, 'modified'); + } + } + }); + + return d3.values(relevant); + }; + difference.complete = function(extent) { var result = {}, id, change; diff --git a/js/id/core/history.js b/js/id/core/history.js index dbc5aaa2a..98f27a6d7 100644 --- a/js/id/core/history.js +++ b/js/id/core/history.js @@ -41,10 +41,6 @@ iD.History = function(context) { return stack[index].graph; }, - base: function() { - return stack[0].graph; - }, - merge: function(entities, extent) { var base = stack[0].graph.base(), diff --git a/js/id/ui/commit.js b/js/id/ui/commit.js index 76794e486..6adc9511f 100644 --- a/js/id/ui/commit.js +++ b/js/id/ui/commit.js @@ -1,15 +1,9 @@ iD.ui.Commit = function(context) { - var event = d3.dispatch('cancel', 'save'), - presets = context.presets(); + var event = d3.dispatch('cancel', 'save'); function commit(selection) { var changes = context.history().changes(), - base = context.history().base(), - relevantChanges = iD.util.relevantChanges( - context.graph(), - changes, - base - ); + summary = context.history().difference().summary(); function zoomToEntity(change) { var entity = change.entity; @@ -131,14 +125,12 @@ iD.ui.Commit = function(context) { .attr('class', 'commit-section modal-section fillL2'); changeSection.append('h3') - .text(relevantChanges.length + ' Changes'); + .text(summary.length + ' Changes'); var li = changeSection.append('ul') .attr('class', 'changeset-list') .selectAll('li') - .data(function(d) { - return relevantChanges; - }) + .data(summary) .enter() .append('li') .on('mouseover', mouseover) @@ -147,9 +139,7 @@ iD.ui.Commit = function(context) { li.append('span') .attr('class', function(d) { - var graph = d.changeType === 'deleted' ? base : context.graph(); - return graph.entity(d.entity.id).geometry(graph) + ' ' + d.changeType + - ' icon icon-pre-text'; + return d.entity.geometry(d.graph) + ' ' + d.changeType + ' icon icon-pre-text'; }); li.append('span') @@ -161,8 +151,7 @@ iD.ui.Commit = function(context) { li.append('strong') .attr('class', 'entity-type') .text(function(d) { - var graph = d.changeType === 'deleted' ? base : context.graph(); - return context.presets().match(d.entity, graph).name(); + return context.presets().match(d.entity, d.graph).name(); }); li.append('span') diff --git a/js/id/util/relevant_changes.js b/js/id/util/relevant_changes.js deleted file mode 100644 index 869ae27c5..000000000 --- a/js/id/util/relevant_changes.js +++ /dev/null @@ -1,43 +0,0 @@ -// filters out verticies where the parent entity is already present -// for simpler changeset listing -iD.util.relevantChanges = function(graph, changes, base) { - var relevant = {}; - - function addEntity(entity, changeType) { - relevant[entity.id] = { - entity: entity, - changeType: changeType - }; - } - - function addParents(entity, theGraph) { - if (!theGraph) theGraph = graph; - var parents = theGraph.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') { - addEntity(entity, change); - - } else if (change === 'modified') { - var moved = entity.loc !== base.entity(entity.id).loc, - retagged = entity.tags !== base.entity(entity.id).tags; - - if (moved) { - addParents(entity, graph); - } - - if (retagged || (moved && entity.hasInterestingTags())) { - addEntity(entity, change); - } - } - }); - }); - - return d3.values(relevant); -}; diff --git a/test/index.html b/test/index.html index 170cc5ea4..14fb4087e 100644 --- a/test/index.html +++ b/test/index.html @@ -192,7 +192,6 @@ - @@ -274,7 +273,6 @@ - diff --git a/test/index_packaged.html b/test/index_packaged.html index 650a0b886..356e4b687 100644 --- a/test/index_packaged.html +++ b/test/index_packaged.html @@ -91,7 +91,6 @@ - diff --git a/test/spec/core/difference.js b/test/spec/core/difference.js index 5fb006224..5f128acce 100644 --- a/test/spec/core/difference.js +++ b/test/spec/core/difference.js @@ -126,6 +126,141 @@ describe("iD.Difference", function () { }); }); + describe("#summary", function () { + var base = iD.Graph({ + 'a': iD.Node({id: 'a', tags: {crossing: 'zebra'}}), + 'b': iD.Node({id: 'b'}), + '-': iD.Way({id: '-', nodes: ['a', 'b']}) + }); + + it("reports a created way as created", function() { + var way = iD.Way({id: '+'}), + head = base.replace(way), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'created', + entity: way, + graph: head + }]); + }); + + it("reports a deleted way as deleted", function() { + var way = base.entity('-'), + head = base.remove(way), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'deleted', + entity: way, + graph: base + }]); + }); + + it("reports a modified way as modified", function() { + var way = base.entity('-').mergeTags({highway: 'primary'}), + head = base.replace(way), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'modified', + entity: way, + graph: head + }]); + }); + + it("reports a way as modified when a member vertex is moved", function() { + var vertex = base.entity('b').move([0,3]), + head = base.replace(vertex), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'modified', + entity: head.entity('-'), + graph: head + }]); + }); + + it("reports a way as modified when a member vertex is added", function() { + var vertex = iD.Node({id: 'c'}), + way = base.entity('-').addNode('c'), + head = base.replace(vertex).replace(way), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'modified', + entity: way, + graph: head + }]); + }); + + it("reports a way as modified when a member vertex is removed", function() { + var way = base.entity('-').removeNode('b'), + head = base.replace(way), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'modified', + entity: way, + graph: head + }]); + }); + + it("reports a created way containing a moved vertex as being created", function() { + var vertex = base.entity('b').move([0,3]), + way = iD.Way({id: '+', nodes: ['b']}), + head = base.replace(way).replace(vertex), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'created', + entity: way, + graph: head + }, { + changeType: 'modified', + entity: head.entity('-'), + graph: head + }]); + }); + + it("reports a created way with a created vertex as being created", function() { + var vertex = iD.Node({id: 'c'}), + way = iD.Way({id: '+', nodes: ['c']}), + head = base.replace(vertex).replace(way), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'created', + entity: way, + graph: head + }]); + }); + + it("reports an existing vertex with added tags as modified", function() { + var vertex = base.entity('a').mergeTags({highway: 'traffic_signals'}), + head = base.replace(vertex), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'modified', + entity: vertex, + graph: head + }]); + }); + + it("reports an existing tagged vertex that is moved as modified", function() { + var vertex = base.entity('a').move([1, 2]), + head = base.replace(vertex), + diff = iD.Difference(base, head); + + expect(diff.summary()[1]).to.eql({ + changeType: 'modified', + entity: vertex, + graph: head + }); + }); + }); + describe("#complete", function () { it("includes created entities", function () { var node = iD.Node({id: 'n'}), diff --git a/test/spec/util/relevant_changes.js b/test/spec/util/relevant_changes.js deleted file mode 100644 index 23ee5f631..000000000 --- a/test/spec/util/relevant_changes.js +++ /dev/null @@ -1,107 +0,0 @@ -describe("iD.util.relevantChanges", function() { - var base = iD.Graph({ - 'a': iD.Node({id: 'a', loc: [0, 0], tags: {crossing: 'zebra'}}), - 'b': iD.Node({id: 'b', loc: [2, 0]}), - 'c': iD.Node({id: 'c', loc: [2, 2]}), - 'd': iD.Node({id: 'd', loc: [0, 2]}), - 'e': iD.Node({id: 'e', loc: [0, 2]}), - '-': iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'e', 'a']}) - }); - - it("returns a way that changed", function() { - 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("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 = base.entity('a').mergeTags({highway: 'traffic_signals'}), - graph = base.replace(vertex), - changes = { modified: [vertex] }, - a = iD.util.relevantChanges(graph, changes, base); - expect(a).to.eql([{ - changeType: 'modified', - entity: vertex - }]); - }); - - it("reports an existing tagged vertex that is moved as modified", function() { - var vertex = base.entity('a').move([1, 2]), - graph = base.replace(vertex), - changes = { modified: [vertex] }, - a = iD.util.relevantChanges(graph, changes, base); - expect(a[1]).to.eql({ - changeType: 'modified', - entity: vertex - }); - }); -}); From 697faf0265269b4d4bd38bccd3fcd44e298c9f66 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 23 Oct 2013 13:22:43 -0700 Subject: [PATCH 18/19] Use summary to calculate # in Save button --- js/id/core/history.js | 4 ---- js/id/ui/save.js | 4 ++-- test/spec/core/history.js | 16 ---------------- 3 files changed, 2 insertions(+), 22 deletions(-) diff --git a/js/id/core/history.js b/js/id/core/history.js index 98f27a6d7..4a8edb5d3 100644 --- a/js/id/core/history.js +++ b/js/id/core/history.js @@ -158,10 +158,6 @@ iD.History = function(context) { return this.difference().length() > 0; }, - numChanges: function() { - return this.difference().length(); - }, - imageryUsed: function(sources) { if (sources) { imageryUsed = sources; diff --git a/js/id/ui/save.js b/js/id/ui/save.js index 4d12e0062..926a0e60d 100644 --- a/js/id/ui/save.js +++ b/js/id/ui/save.js @@ -42,13 +42,13 @@ iD.ui.Save = function(context) { var numChanges = 0; context.history().on('change.save', function() { - var _ = history.numChanges(); + var _ = history.difference().summary().length; if (_ === numChanges) return; numChanges = _; tooltip.title(iD.ui.tooltipHtml(t(numChanges > 0 ? - 'save.help' : 'save.no_changes'), key)) + 'save.help' : 'save.no_changes'), key)); button .classed('disabled', numChanges === 0) diff --git a/test/spec/core/history.js b/test/spec/core/history.js index ad68efff4..ec584711e 100644 --- a/test/spec/core/history.js +++ b/test/spec/core/history.js @@ -211,22 +211,6 @@ describe("iD.History", function () { }); }); - 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({ n1: 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"); From 2de500a90a691b84c12f637a178988013098272b Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 23 Oct 2013 13:34:23 -0700 Subject: [PATCH 19/19] Add a few more vertex cases to summary --- js/id/core/difference.js | 6 ++++++ test/spec/core/difference.js | 42 ++++++++++++++++++++++++++++++++---- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/js/id/core/difference.js b/js/id/core/difference.js index 8b8ecbb3e..12ba48bc3 100644 --- a/js/id/core/difference.js +++ b/js/id/core/difference.js @@ -128,6 +128,12 @@ iD.Difference = function(base, head) { if (retagged || (moved && change.head.hasInterestingTags())) { addEntity(change.head, head, 'modified'); } + + } else if (change.head && change.head.hasInterestingTags()) { // created vertex + addEntity(change.head, head, 'created'); + + } else if (change.base && change.base.hasInterestingTags()) { // deleted vertex + addEntity(change.base, base, 'deleted'); } }); diff --git a/test/spec/core/difference.js b/test/spec/core/difference.js index 5f128acce..a7920e87c 100644 --- a/test/spec/core/difference.js +++ b/test/spec/core/difference.js @@ -130,6 +130,7 @@ describe("iD.Difference", function () { var base = iD.Graph({ 'a': iD.Node({id: 'a', tags: {crossing: 'zebra'}}), 'b': iD.Node({id: 'b'}), + 'v': iD.Node({id: 'v'}), '-': iD.Way({id: '-', nodes: ['a', 'b']}) }); @@ -236,7 +237,7 @@ describe("iD.Difference", function () { }]); }); - it("reports an existing vertex with added tags as modified", function() { + it("reports a vertex as modified when it has tags and they are changed", function() { var vertex = base.entity('a').mergeTags({highway: 'traffic_signals'}), head = base.replace(vertex), diff = iD.Difference(base, head); @@ -248,16 +249,49 @@ describe("iD.Difference", function () { }]); }); - it("reports an existing tagged vertex that is moved as modified", function() { + it("reports a vertex as modified when it has tags and is moved", function() { var vertex = base.entity('a').move([1, 2]), head = base.replace(vertex), diff = iD.Difference(base, head); - expect(diff.summary()[1]).to.eql({ + expect(diff.summary()).to.eql([{ + changeType: 'modified', + entity: head.entity('-'), + graph: head + }, { changeType: 'modified', entity: vertex, graph: head - }); + }]); + }); + + it("reports a vertex as deleted when it had tags", function() { + var vertex = base.entity('v'), + head = base.remove(vertex), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'deleted', + entity: vertex, + graph: base + }]); + }); + + it("reports a vertex as created when it has tags", function() { + var vertex = iD.Node({id: 'c', tags: {crossing: 'zebra'}}), + way = base.entity('-').addNode('c'), + head = base.replace(way).replace(vertex), + diff = iD.Difference(base, head); + + expect(diff.summary()).to.eql([{ + changeType: 'modified', + entity: way, + graph: head + }, { + changeType: 'created', + entity: vertex, + graph: head + }]); }); });