From 4903d495b721f302641006511e618d7a09b0dddc Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Tue, 21 Mar 2017 02:17:04 -0400 Subject: [PATCH 1/5] Draw selected items last, so halos are more visible (see #2914) --- modules/modes/select.js | 2 + modules/renderer/map.js | 2 +- modules/svg/lines.js | 117 +++++++++++++++++++++++++++++++--------- test/spec/svg/lines.js | 22 ++++---- 4 files changed, 106 insertions(+), 37 deletions(-) diff --git a/modules/modes/select.js b/modules/modes/select.js index 93c9e1036..8f216387c 100644 --- a/modules/modes/select.js +++ b/modules/modes/select.js @@ -477,6 +477,8 @@ export function modeSelect(context, selectedIDs) { var loc = extent.center(); context.map().centerEase(loc); + } else { + context.map().pan([0,0]); } timeout = window.setTimeout(function() { diff --git a/modules/renderer/map.js b/modules/renderer/map.js index b51bdd475..8c5cc16ff 100644 --- a/modules/renderer/map.js +++ b/modules/renderer/map.js @@ -42,7 +42,7 @@ export function rendererMap(context) { drawLayers = svgLayers(projection, context), drawPoints = svgPoints(projection, context), drawVertices = svgVertices(projection, context), - drawLines = svgLines(projection), + drawLines = svgLines(projection, context), drawAreas = svgAreas(projection, context), drawMidpoints = svgMidpoints(projection, context), drawLabels = svgLabels(projection, context), diff --git a/modules/svg/lines.js b/modules/svg/lines.js index f88174a1a..da8d93c24 100644 --- a/modules/svg/lines.js +++ b/modules/svg/lines.js @@ -11,7 +11,7 @@ import { osmEntity, osmSimpleMultipolygonOuterMember } from '../osm/index'; import { utilDetect } from '../util/detect'; -export function svgLines(projection) { +export function svgLines(projection, context) { var detected = utilDetect(); var highway_stack = { @@ -29,15 +29,45 @@ export function svgLines(projection) { footway: 11 }; + function waystack(a, b) { - var as = 0, bs = 0; - if (a.tags.highway) { as -= highway_stack[a.tags.highway]; } - if (b.tags.highway) { bs -= highway_stack[b.tags.highway]; } - return as - bs; + var selected = context.selectedIDs(), + scoreA = selected.indexOf(a.id) !== -1 ? 20 : 0, + scoreB = selected.indexOf(b.id) !== -1 ? 20 : 0; + + if (a.tags.highway) { scoreA -= highway_stack[a.tags.highway]; } + if (b.tags.highway) { scoreB -= highway_stack[b.tags.highway]; } + return scoreA - scoreB; } return function drawLines(selection, graph, entities, filter) { + + + function drawLineGroup(selection, klass, data) { + var lines = selection + .selectAll('path') + .filter(filter) + .data(data, osmEntity.key); + + lines.exit() + .remove(); + + // Optimization: call simple TagClasses only on enter selection. This + // works because osmEntity.key is defined to include the entity v attribute. + lines.enter() + .append('path') + .attr('class', function(d) { return 'way line ' + klass + ' ' + d.id; }) + .call(svgTagClasses()) + .merge(lines) + .sort(waystack) + .attr('d', getPath) + .call(svgTagClasses().tags(svgRelationMemberTags(graph))); + + return selection; + } + + var ways = [], pathdata = {}, onewaydata = {}, getPath = svgPath(projection, graph); @@ -77,7 +107,7 @@ export function svgLines(projection) { var linegroup = layergroup .selectAll('g.linegroup') - .data(['shadow', 'casing', 'stroke']); + .data(['shadow', 'casing', 'stroke', 'shadow2', 'casing2', 'stroke2']); linegroup = linegroup.enter() .append('g') @@ -85,27 +115,64 @@ export function svgLines(projection) { .merge(linegroup); - var lines = linegroup - .selectAll('path') - .filter(filter) - .data( - function() { return pathdata[this.parentNode.__data__] || []; }, - osmEntity.key - ); + layergroup.selectAll('g.line-shadow') + .call(drawLineGroup, 'shadow', function() { + var data = pathdata[this.parentNode.__data__] || []; + return data.filter(function(d) { return context.selectedIDs().indexOf(d.id) === -1; }); + }); + layergroup.selectAll('g.line-casing') + .call(drawLineGroup, 'casing', function() { + var data = pathdata[this.parentNode.__data__] || []; + return data.filter(function(d) { return context.selectedIDs().indexOf(d.id) === -1; }); + }); + layergroup.selectAll('g.line-stroke') + .call(drawLineGroup, 'stroke', function() { + var data = pathdata[this.parentNode.__data__] || []; + return data.filter(function(d) { return context.selectedIDs().indexOf(d.id) === -1; }); + }); + layergroup.selectAll('g.line-shadow2') + .call(drawLineGroup, 'shadow', function() { + var data = pathdata[this.parentNode.__data__] || []; + return data.filter(function(d) { return context.selectedIDs().indexOf(d.id) !== -1; }); + }); + layergroup.selectAll('g.line-casing2') + .call(drawLineGroup, 'casing', function() { + var data = pathdata[this.parentNode.__data__] || []; + return data.filter(function(d) { return context.selectedIDs().indexOf(d.id) !== -1; }); + }); + layergroup.selectAll('g.line-stroke2') + .call(drawLineGroup, 'stroke', function() { + var data = pathdata[this.parentNode.__data__] || []; + return data.filter(function(d) { return context.selectedIDs().indexOf(d.id) !== -1; }); + }); + + + // var lines = linegroup + // .selectAll('path') + // .filter(filter) + // .data( + // function() { + // var data = pathdata[this.parentNode.__data__] || []; + // return data.filter(function(d) { return context.selectedIDs().indexOf(d.id) === -1; }); + // }, + // osmEntity.key + // ); + + // lines.exit() + // .remove(); + + // // Optimization: call simple TagClasses only on enter selection. This + // // works because osmEntity.key is defined to include the entity v attribute. + // lines.enter() + // .append('path') + // .attr('class', function(d) { return 'way line ' + this.parentNode.__data__ + ' ' + d.id; }) + // .call(svgTagClasses()) + // .merge(lines) + // .sort(waystack) + // .attr('d', getPath) + // .call(svgTagClasses().tags(svgRelationMemberTags(graph))); - lines.exit() - .remove(); - // Optimization: call simple TagClasses only on enter selection. This - // works because osmEntity.key is defined to include the entity v attribute. - lines.enter() - .append('path') - .attr('class', function(d) { return 'way line ' + this.parentNode.__data__ + ' ' + d.id; }) - .call(svgTagClasses()) - .merge(lines) - .sort(waystack) - .attr('d', getPath) - .call(svgTagClasses().tags(svgRelationMemberTags(graph))); var onewaygroup = layergroup diff --git a/test/spec/svg/lines.js b/test/spec/svg/lines.js index 001e12cc6..805d5f400 100644 --- a/test/spec/svg/lines.js +++ b/test/spec/svg/lines.js @@ -22,7 +22,7 @@ describe('iD.svgLines', function () { line = iD.Way({nodes: [a.id, b.id]}), graph = iD.Graph([a, b, line]); - surface.call(iD.svgLines(projection), graph, [line], all); + surface.call(iD.svgLines(projection, context), graph, [line], all); expect(surface.select('path.way')).to.be.classed('way'); expect(surface.select('path.line')).to.be.classed('line'); @@ -34,7 +34,7 @@ describe('iD.svgLines', function () { line = iD.Way({nodes: [a.id, b.id], tags: {highway: 'residential'}}), graph = iD.Graph([a, b, line]); - surface.call(iD.svgLines(projection), graph, [line], all); + surface.call(iD.svgLines(projection, context), graph, [line], all); expect(surface.select('.line')).to.be.classed('tag-highway'); expect(surface.select('.line')).to.be.classed('tag-highway-residential'); @@ -47,7 +47,7 @@ describe('iD.svgLines', function () { relation = iD.Relation({members: [{id: line.id}], tags: {type: 'multipolygon', natural: 'wood'}}), graph = iD.Graph([a, b, line, relation]); - surface.call(iD.svgLines(projection), graph, [line], all); + surface.call(iD.svgLines(projection, context), graph, [line], all); expect(surface.select('.stroke')).to.be.classed('tag-natural-wood'); }); @@ -60,7 +60,7 @@ describe('iD.svgLines', function () { r = iD.Relation({members: [{id: w.id}], tags: {type: 'multipolygon'}}), graph = iD.Graph([a, b, c, w, r]); - surface.call(iD.svgLines(projection), graph, [w], all); + surface.call(iD.svgLines(projection, context), graph, [w], all); expect(surface.select('.stroke')).to.be.classed('tag-natural-wood'); }); @@ -74,7 +74,7 @@ describe('iD.svgLines', function () { r = iD.Relation({members: [{id: o.id, role: 'outer'}, {id: i.id, role: 'inner'}], tags: {type: 'multipolygon'}}), graph = iD.Graph([a, b, c, o, i, r]); - surface.call(iD.svgLines(projection), graph, [i], all); + surface.call(iD.svgLines(projection, context), graph, [i], all); expect(surface.select('.stroke')).to.be.classed('tag-natural-wood'); }); @@ -90,7 +90,7 @@ describe('iD.svgLines', function () { ]); it('stacks higher lines above lower ones in a single render', function () { - surface.call(iD.svgLines(projection), graph, [graph.entity('lo'), graph.entity('hi')], none); + surface.call(iD.svgLines(projection, context), graph, [graph.entity('lo'), graph.entity('hi')], none); var selection = surface.selectAll('g.line-stroke > path.line'); expect(selection.nodes()[0].__data__.id).to.eql('lo'); @@ -98,7 +98,7 @@ describe('iD.svgLines', function () { }); it('stacks higher lines above lower ones in a single render (reverse)', function () { - surface.call(iD.svgLines(projection), graph, [graph.entity('hi'), graph.entity('lo')], none); + surface.call(iD.svgLines(projection, context), graph, [graph.entity('hi'), graph.entity('lo')], none); var selection = surface.selectAll('g.line-stroke > path.line'); expect(selection.nodes()[0].__data__.id).to.eql('lo'); @@ -106,8 +106,8 @@ describe('iD.svgLines', function () { }); it('stacks higher lines above lower ones in separate renders', function () { - surface.call(iD.svgLines(projection), graph, [graph.entity('lo')], none); - surface.call(iD.svgLines(projection), graph, [graph.entity('hi')], none); + surface.call(iD.svgLines(projection, context), graph, [graph.entity('lo')], none); + surface.call(iD.svgLines(projection, context), graph, [graph.entity('hi')], none); var selection = surface.selectAll('g.line-stroke > path.line'); expect(selection.nodes()[0].__data__.id).to.eql('lo'); @@ -115,8 +115,8 @@ describe('iD.svgLines', function () { }); it('stacks higher lines above lower in separate renders (reverse)', function () { - surface.call(iD.svgLines(projection), graph, [graph.entity('hi')], none); - surface.call(iD.svgLines(projection), graph, [graph.entity('lo')], none); + surface.call(iD.svgLines(projection, context), graph, [graph.entity('hi')], none); + surface.call(iD.svgLines(projection, context), graph, [graph.entity('lo')], none); var selection = surface.selectAll('g.line-stroke > path.line'); expect(selection.nodes()[0].__data__.id).to.eql('lo'); From 062824970b5a4440fdeb845eb271d14d47e49927 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Tue, 21 Mar 2017 02:23:18 -0400 Subject: [PATCH 2/5] Wider halos around bridges --- css/50_misc.css | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/css/50_misc.css b/css/50_misc.css index faae427ad..e533a7647 100644 --- a/css/50_misc.css +++ b/css/50_misc.css @@ -55,13 +55,13 @@ path.casing.tag-bridge { } path.shadow.tag-bridge { - stroke-width: 22; + stroke-width: 24; } path.casing.tag-bridge { stroke-width: 16; } .low-zoom path.shadow.tag-bridge { - stroke-width: 14; + stroke-width: 16; } .low-zoom path.casing.tag-bridge { stroke-width: 10; @@ -78,7 +78,7 @@ path.shadow.tag-highway-steps.tag-bridge, path.shadow.tag-highway-footway.tag-bridge, path.shadow.tag-highway-cycleway.tag-bridge, path.shadow.tag-highway-bridleway.tag-bridge { - stroke-width: 17; + stroke-width: 18; } path.casing.line.tag-railway.tag-bridge, path.casing.tag-highway-living_street.tag-bridge, From 85ed3cc2d3de48203c65bcd60d110e9649e3ca64 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Tue, 21 Mar 2017 12:42:46 -0400 Subject: [PATCH 3/5] Cleanup line drawing code with selected sort --- modules/modes/select.js | 4 +- modules/svg/lines.js | 128 +++++++++++++++------------------------- 2 files changed, 51 insertions(+), 81 deletions(-) diff --git a/modules/modes/select.js b/modules/modes/select.js index 8f216387c..fda2cb8b8 100644 --- a/modules/modes/select.js +++ b/modules/modes/select.js @@ -477,8 +477,8 @@ export function modeSelect(context, selectedIDs) { var loc = extent.center(); context.map().centerEase(loc); - } else { - context.map().pan([0,0]); + } else if (singular() && singular().type === 'way') { + context.map().pan([0,0]); // full redraw, to adjust z-sorting #2914 } timeout = window.setTimeout(function() { diff --git a/modules/svg/lines.js b/modules/svg/lines.js index da8d93c24..94ca0206a 100644 --- a/modules/svg/lines.js +++ b/modules/svg/lines.js @@ -7,7 +7,7 @@ import { svgTagClasses } from './index'; -import { osmEntity, osmSimpleMultipolygonOuterMember } from '../osm/index'; +import { osmEntity, osmSimpleMultipolygonOuterMember } from '../osm'; import { utilDetect } from '../util/detect'; @@ -30,25 +30,25 @@ export function svgLines(projection, context) { }; - function waystack(a, b) { - var selected = context.selectedIDs(), - scoreA = selected.indexOf(a.id) !== -1 ? 20 : 0, - scoreB = selected.indexOf(b.id) !== -1 ? 20 : 0; - - if (a.tags.highway) { scoreA -= highway_stack[a.tags.highway]; } - if (b.tags.highway) { scoreB -= highway_stack[b.tags.highway]; } - return scoreA - scoreB; - } + function drawLines(selection, graph, entities, filter) { - return function drawLines(selection, graph, entities, filter) { + function waystack(a, b) { + var selected = context.selectedIDs(), + scoreA = selected.indexOf(a.id) !== -1 ? 20 : 0, + scoreB = selected.indexOf(b.id) !== -1 ? 20 : 0; + + if (a.tags.highway) { scoreA -= highway_stack[a.tags.highway]; } + if (b.tags.highway) { scoreB -= highway_stack[b.tags.highway]; } + return scoreA - scoreB; + } - function drawLineGroup(selection, klass, data) { + function drawLineGroup(selection, klass, isSelected) { var lines = selection .selectAll('path') .filter(filter) - .data(data, osmEntity.key); + .data(getPathData(isSelected), osmEntity.key); lines.exit() .remove(); @@ -68,8 +68,24 @@ export function svgLines(projection, context) { } - var ways = [], pathdata = {}, onewaydata = {}, - getPath = svgPath(projection, graph); + function getPathData(isSelected) { + return function() { + var layer = this.parentNode.__data__; + var data = pathdata[layer] || []; + return data.filter(function(d) { + if (isSelected) + return context.selectedIDs().indexOf(d.id) !== -1; + else + return context.selectedIDs().indexOf(d.id) === -1; + }); + }; + } + + + var getPath = svgPath(projection, graph), + ways = [], + pathdata = {}, + onewaydata = {}; for (var i = 0; i < entities.length; i++) { var entity = entities[i], @@ -82,7 +98,6 @@ export function svgLines(projection, context) { } ways = ways.filter(getPath); - pathdata = _.groupBy(ways, function(way) { return way.layer(); }); _.forOwn(pathdata, function(v, k) { @@ -93,6 +108,7 @@ export function svgLines(projection, context) { .valueOf(); }); + var layer = selection.selectAll('.layer-lines'); var layergroup = layer @@ -104,75 +120,27 @@ export function svgLines(projection, context) { .attr('class', function(d) { return 'layergroup layer' + String(d); }) .merge(layergroup); - - var linegroup = layergroup + layergroup .selectAll('g.linegroup') - .data(['shadow', 'casing', 'stroke', 'shadow2', 'casing2', 'stroke2']); - - linegroup = linegroup.enter() + .data(['shadow', 'casing', 'stroke', 'shadow-highlighted', 'casing-highlighted', 'stroke-highlighted']) + .enter() .append('g') - .attr('class', function(d) { return 'linegroup line-' + d; }) - .merge(linegroup); + .attr('class', function(d) { return 'linegroup line-' + d; }); layergroup.selectAll('g.line-shadow') - .call(drawLineGroup, 'shadow', function() { - var data = pathdata[this.parentNode.__data__] || []; - return data.filter(function(d) { return context.selectedIDs().indexOf(d.id) === -1; }); - }); + .call(drawLineGroup, 'shadow', false); layergroup.selectAll('g.line-casing') - .call(drawLineGroup, 'casing', function() { - var data = pathdata[this.parentNode.__data__] || []; - return data.filter(function(d) { return context.selectedIDs().indexOf(d.id) === -1; }); - }); + .call(drawLineGroup, 'casing', false); layergroup.selectAll('g.line-stroke') - .call(drawLineGroup, 'stroke', function() { - var data = pathdata[this.parentNode.__data__] || []; - return data.filter(function(d) { return context.selectedIDs().indexOf(d.id) === -1; }); - }); - layergroup.selectAll('g.line-shadow2') - .call(drawLineGroup, 'shadow', function() { - var data = pathdata[this.parentNode.__data__] || []; - return data.filter(function(d) { return context.selectedIDs().indexOf(d.id) !== -1; }); - }); - layergroup.selectAll('g.line-casing2') - .call(drawLineGroup, 'casing', function() { - var data = pathdata[this.parentNode.__data__] || []; - return data.filter(function(d) { return context.selectedIDs().indexOf(d.id) !== -1; }); - }); - layergroup.selectAll('g.line-stroke2') - .call(drawLineGroup, 'stroke', function() { - var data = pathdata[this.parentNode.__data__] || []; - return data.filter(function(d) { return context.selectedIDs().indexOf(d.id) !== -1; }); - }); - - - // var lines = linegroup - // .selectAll('path') - // .filter(filter) - // .data( - // function() { - // var data = pathdata[this.parentNode.__data__] || []; - // return data.filter(function(d) { return context.selectedIDs().indexOf(d.id) === -1; }); - // }, - // osmEntity.key - // ); - - // lines.exit() - // .remove(); - - // // Optimization: call simple TagClasses only on enter selection. This - // // works because osmEntity.key is defined to include the entity v attribute. - // lines.enter() - // .append('path') - // .attr('class', function(d) { return 'way line ' + this.parentNode.__data__ + ' ' + d.id; }) - // .call(svgTagClasses()) - // .merge(lines) - // .sort(waystack) - // .attr('d', getPath) - // .call(svgTagClasses().tags(svgRelationMemberTags(graph))); - + .call(drawLineGroup, 'stroke', false); + layergroup.selectAll('g.line-shadow-highlighted') + .call(drawLineGroup, 'shadow', true); + layergroup.selectAll('g.line-casing-highlighted') + .call(drawLineGroup, 'casing', true); + layergroup.selectAll('g.line-stroke-highlighted') + .call(drawLineGroup, 'stroke', true); var onewaygroup = layergroup @@ -184,7 +152,6 @@ export function svgLines(projection, context) { .attr('class', 'onewaygroup') .merge(onewaygroup); - var oneways = onewaygroup .selectAll('path') .filter(filter) @@ -206,5 +173,8 @@ export function svgLines(projection, context) { if (detected.ie) { oneways.each(function() { this.parentNode.insertBefore(this, this); }); } - }; + } + + + return drawLines; } From 0228f934b32f5c0fa1a2eac94423bae9118b1cdb Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Tue, 21 Mar 2017 14:19:57 -0400 Subject: [PATCH 4/5] Put back the `selected` class when redrawing selected lines --- modules/svg/lines.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/svg/lines.js b/modules/svg/lines.js index 94ca0206a..c788986ff 100644 --- a/modules/svg/lines.js +++ b/modules/svg/lines.js @@ -57,7 +57,9 @@ export function svgLines(projection, context) { // works because osmEntity.key is defined to include the entity v attribute. lines.enter() .append('path') - .attr('class', function(d) { return 'way line ' + klass + ' ' + d.id; }) + .attr('class', function(d) { + return 'way line ' + klass + ' ' + d.id + (isSelected ? ' selected' : ''); + }) .call(svgTagClasses()) .merge(lines) .sort(waystack) From d664f74832276af46b9ba211d51ece12f2688175 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Tue, 21 Mar 2017 14:20:25 -0400 Subject: [PATCH 5/5] When changing selection, always update the url hash --- modules/behavior/hash.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/modules/behavior/hash.js b/modules/behavior/hash.js index 6849aa6c3..5c54b69a2 100644 --- a/modules/behavior/hash.js +++ b/modules/behavior/hash.js @@ -28,20 +28,17 @@ export function behaviorHash(context) { q = _.omit(utilStringQs(window.location.hash.substring(1)), 'comment'), newParams = {}; - if (mode && mode.id === 'browse') { - delete q.id; - } else { - var selected = context.selectedIDs().filter(function(id) { - return !context.entity(id).isNew(); - }); - if (selected.length) { - newParams.id = selected.join(','); - } + delete q.id; + var selected = context.selectedIDs().filter(function(id) { + return !context.entity(id).isNew(); + }); + if (selected.length) { + newParams.id = selected.join(','); } newParams.map = zoom.toFixed(2) + - '/' + center[1].toFixed(precision) + - '/' + center[0].toFixed(precision); + '/' + center[1].toFixed(precision) + + '/' + center[0].toFixed(precision); return '#' + utilQsString(_.assign(q, newParams), true); };