From 3bc841e95b5d3f7249bee249abc4a194c0451596 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 9 Nov 2018 16:51:19 -0500 Subject: [PATCH 1/6] Adjust layer ordering to draw touch targets above data layers --- css/20_map.css | 15 +----- modules/modes/drag_node.js | 2 +- modules/renderer/map.js | 18 ++++---- modules/svg/areas.js | 11 +++-- modules/svg/index.js | 1 + modules/svg/labels.js | 22 +++++---- modules/svg/layers.js | 6 ++- modules/svg/lines.js | 10 ++-- modules/svg/midpoints.js | 76 ++++++++++++++----------------- modules/svg/osm.js | 30 +++--------- modules/svg/points.js | 19 ++++---- modules/svg/touch.js | 12 +++++ modules/svg/turns.js | 11 +---- modules/svg/vertices.js | 7 ++- modules/ui/fields/restrictions.js | 2 +- test/spec/behavior/select.js | 2 +- test/spec/svg/layers.js | 19 ++++---- test/spec/svg/osm.js | 32 +++++-------- 18 files changed, 133 insertions(+), 162 deletions(-) create mode 100644 modules/svg/touch.js diff --git a/css/20_map.css b/css/20_map.css index def29ff996..a4206260e5 100644 --- a/css/20_map.css +++ b/css/20_map.css @@ -66,7 +66,6 @@ /* points & notes */ - g.note .stroke { stroke: #222; stroke-width: 1; @@ -118,21 +117,17 @@ g.point ellipse.stroke { /* vertices and midpoints */ - g.vertex .fill { fill: #000; } - g.vertex .stroke { stroke: #666; stroke-width: 1; fill: white; } - g.vertex.shared .stroke { fill: #bbb; } - g.midpoint .fill { fill: #eee; stroke: #444; @@ -160,7 +155,6 @@ g.vertex.selected .shadow { /* lines */ - .preset-icon .icon.iD-other-line { color: #fff; fill: #777; @@ -198,7 +192,6 @@ path.line.stroke { /* Labels / Markers */ - text { font-size: 10px; color: #222; @@ -238,7 +231,7 @@ text.pointlabel { dominant-baseline: auto; } -.layer-labels-halo text { +.labels-group.halo text { opacity: 0.7; stroke: #fff; stroke-width: 5px; @@ -246,9 +239,8 @@ text.pointlabel { } text.nolabel { - opacity: 0; + opacity: 0 !important; } - text.point { font-size: 10px; } @@ -259,14 +251,12 @@ text.point { stroke-width: 2px; stroke-miterlimit: 1; } - .icon.areaicon { fill: #222; opacity: 0.8; } /* Highlighting */ - g.point.highlighted .shadow, path.shadow.highlighted { stroke-opacity: 0.95; @@ -279,7 +269,6 @@ g.vertex.highlighted .shadow { } /* Turn Restrictions */ - g.turn rect, g.turn circle { fill: none; diff --git a/modules/modes/drag_node.js b/modules/modes/drag_node.js index 518816881f..a395d98fb4 100644 --- a/modules/modes/drag_node.js +++ b/modules/modes/drag_node.js @@ -429,7 +429,7 @@ export function modeDragNode(context) { var drag = behaviorDrag() - .selector('.layer-points-targets .target') + .selector('.layer-touch.points .target') .surface(d3_select('#map').node()) .origin(origin) .on('start', start) diff --git a/modules/renderer/map.js b/modules/renderer/map.js index 2d699b8626..4dcf37e431 100644 --- a/modules/renderer/map.js +++ b/modules/renderer/map.js @@ -194,16 +194,14 @@ export function rendererMap(context) { .on('mouseover.vertices', function() { if (map.editable() && !_transformed) { var hover = d3_event.target.__data__; - surface.selectAll('.data-layer-osm') - .call(drawVertices.drawHover, context.graph(), hover, map.extent()); + surface.call(drawVertices.drawHover, context.graph(), hover, map.extent()); dispatch.call('drawn', this, { full: false }); } }) .on('mouseout.vertices', function() { if (map.editable() && !_transformed) { var hover = d3_event.relatedTarget && d3_event.relatedTarget.__data__; - surface.selectAll('.data-layer-osm') - .call(drawVertices.drawHover, context.graph(), hover, map.extent()); + surface.call(drawVertices.drawHover, context.graph(), hover, map.extent()); dispatch.call('drawn', this, { full: false }); } }); @@ -232,7 +230,7 @@ export function rendererMap(context) { data = context.features().filter(data, graph); - surface.selectAll('.data-layer-osm') + surface .call(drawVertices.drawSelected, graph, map.extent()) .call(drawLines, graph, data, filter) .call(drawAreas, graph, data, filter) @@ -327,11 +325,10 @@ export function rendererMap(context) { if (mode && mode.id === 'select') { // update selected vertices - the user might have just double-clicked a way, // creating a new vertex, triggering a partial redraw without a mode change - surface.selectAll('.data-layer-osm') - .call(drawVertices.drawSelected, graph, map.extent()); + surface.call(drawVertices.drawSelected, graph, map.extent()); } - surface.selectAll('.data-layer-osm') + surface .call(drawVertices, graph, data, filter, map.extent(), fullRedraw) .call(drawLines, graph, data, filter) .call(drawAreas, graph, data, filter) @@ -346,6 +343,7 @@ export function rendererMap(context) { function editOff() { context.features().resetStats(); surface.selectAll('.layer-osm *').remove(); + surface.selectAll('.layer-touch *').remove(); var mode = context.mode(); if (mode && mode.id !== 'save' && mode.id !== 'select-note' && mode.id !== 'select-data') { @@ -838,7 +836,7 @@ export function rendererMap(context) { map.editable = function() { - var osmLayer = surface.selectAll('.data-layer-osm'); + var osmLayer = surface.selectAll('.data-layer.osm'); if (!osmLayer.empty() && osmLayer.classed('disabled')) return false; return map.zoom() >= context.minEditableZoom(); @@ -846,7 +844,7 @@ export function rendererMap(context) { map.notesEditable = function() { - var noteLayer = surface.selectAll('.data-layer-notes'); + var noteLayer = surface.selectAll('.data-layer.notes'); if (!noteLayer.empty() && noteLayer.classed('disabled')) return false; return map.zoom() >= context.minEditableZoom(); diff --git a/modules/svg/areas.js b/modules/svg/areas.js index 546919ecdb..a334ca602c 100644 --- a/modules/svg/areas.js +++ b/modules/svg/areas.js @@ -152,9 +152,11 @@ export function svgAreas(projection, context) { .attr('d', path); - var layer = selection.selectAll('.layer-areas .layer-areas-areas'); + var drawLayer = selection.selectAll('.layer-osm.areas'); + var touchLayer = selection.selectAll('.layer-touch.areas'); - var areagroup = layer + // Draw areas.. + var areagroup = drawLayer .selectAll('g.areagroup') .data(['fill', 'shadow', 'stroke']); @@ -188,7 +190,6 @@ export function svgAreas(projection, context) { .merge(paths) .each(function(entity) { var layer = this.parentNode.__data__; - this.setAttribute('class', entity.type + ' area ' + layer + ' ' + entity.id); if (layer === 'fill') { @@ -200,8 +201,8 @@ export function svgAreas(projection, context) { .attr('d', path); - // touch targets - selection.selectAll('.layer-areas .layer-areas-targets') + // Draw touch targets.. + touchLayer .call(drawTargets, graph, data.stroke, filter); } diff --git a/modules/svg/index.js b/modules/svg/index.js index 4b1bd37a33..099d18fca1 100644 --- a/modules/svg/index.js +++ b/modules/svg/index.js @@ -21,5 +21,6 @@ export { svgRelationMemberTags } from './helpers.js'; export { svgSegmentWay } from './helpers.js'; export { svgStreetside } from './streetside.js'; export { svgTagClasses } from './tag_classes.js'; +export { svgTouch } from './touch.js'; export { svgTurns } from './turns.js'; export { svgVertices } from './vertices.js'; diff --git a/modules/svg/labels.js b/modules/svg/labels.js index 1b02951fd6..6bda16f47d 100644 --- a/modules/svg/labels.js +++ b/modules/svg/labels.js @@ -672,17 +672,23 @@ export function svgLabels(projection, context) { } - var layer = selection.selectAll('.layer-labels'); - var halo = layer.selectAll('.layer-labels-halo'); - var label = layer.selectAll('.layer-labels-label'); - var debug = layer.selectAll('.layer-labels-debug'); + var layer = selection.selectAll('.layer-osm.labels'); + layer.selectAll('.labels-group') + .data(['halo', 'label', 'debug']) + .enter() + .append('g') + .attr('class', function(d) { return 'labels-group ' + d; }); + + var halo = layer.selectAll('.labels-group.halo'); + var label = layer.selectAll('.labels-group.label'); + var debug = layer.selectAll('.labels-group.debug'); // points drawPointLabels(label, labelled.point, filter, 'pointlabel', positions.point); drawPointLabels(halo, labelled.point, filter, 'pointlabel-halo', positions.point); // lines - drawLinePaths(halo, labelled.line, filter, '', positions.line); + drawLinePaths(layer, labelled.line, filter, '', positions.line); drawLineLabels(label, labelled.line, filter, 'linelabel', positions.line); drawLineLabels(halo, labelled.line, filter, 'linelabel-halo', positions.line); @@ -701,8 +707,8 @@ export function svgLabels(projection, context) { function filterLabels(selection) { - var layers = selection - .selectAll('.layer-labels-label, .layer-labels-halo'); + var drawLayer = selection.selectAll('.layer-osm.labels'); + var layers = drawLayer.selectAll('.labels-group.halo, .labels-group.label'); layers.selectAll('.nolabel') .classed('nolabel', false); @@ -733,7 +739,7 @@ export function svgLabels(projection, context) { // draw the mouse bbox if debugging is on.. - var debug = selection.selectAll('.layer-labels-debug'); + var debug = selection.selectAll('.labels-group.debug'); var gj = []; if (context.getDebug('collision')) { gj = bbox ? [{ diff --git a/modules/svg/layers.js b/modules/svg/layers.js index 7493cc6953..220a53201b 100644 --- a/modules/svg/layers.js +++ b/modules/svg/layers.js @@ -15,6 +15,7 @@ import { svgMapillarySigns } from './mapillary_signs'; import { svgOpenstreetcamImages } from './openstreetcam_images'; import { svgOsm } from './osm'; import { svgNotes } from './notes'; +import { svgTouch } from './touch'; import { utilRebind } from '../util/rebind'; import { utilGetDimensions, utilSetDimensions } from '../util/dimensions'; @@ -30,7 +31,8 @@ export function svgLayers(projection, context) { { id: 'mapillary-images', layer: svgMapillaryImages(projection, context, dispatch) }, { id: 'mapillary-signs', layer: svgMapillarySigns(projection, context, dispatch) }, { id: 'openstreetcam-images', layer: svgOpenstreetcamImages(projection, context, dispatch) }, - { id: 'debug', layer: svgDebug(projection, context, dispatch) } + { id: 'debug', layer: svgDebug(projection, context, dispatch) }, + { id: 'touch', layer: svgTouch(projection, context, dispatch) } ]; @@ -58,7 +60,7 @@ export function svgLayers(projection, context) { groups.enter() .append('g') - .attr('class', function(d) { return 'data-layer data-layer-' + d.id; }) + .attr('class', function(d) { return 'data-layer ' + d.id; }) .merge(groups) .each(function(d) { d3_select(this).call(d.layer); }); } diff --git a/modules/svg/lines.js b/modules/svg/lines.js index 67c2cce9a5..ed6762de86 100644 --- a/modules/svg/lines.js +++ b/modules/svg/lines.js @@ -175,9 +175,11 @@ export function svgLines(projection, context) { }); - var covered = selection.selectAll('.layer-covered'); // under areas - var uncovered = selection.selectAll('.layer-lines .layer-lines-lines'); // over areas + var covered = selection.selectAll('.layer-osm.covered'); // under areas + var uncovered = selection.selectAll('.layer-osm.lines'); // over areas + var touchLayer = selection.selectAll('.layer-touch.lines'); + // Draw lines.. [covered, uncovered].forEach(function(selection) { var range = (selection === covered ? d3_range(-10,0) : d3_range(0,11)); var layergroup = selection @@ -243,8 +245,8 @@ export function svgLines(projection, context) { } }); - // touch targets - selection.selectAll('.layer-lines .layer-lines-targets') + // Draw touch targets.. + touchLayer .call(drawTargets, graph, ways, filter); } diff --git a/modules/svg/midpoints.js b/modules/svg/midpoints.js index 0f68d949c1..6ed67823fc 100644 --- a/modules/svg/midpoints.js +++ b/modules/svg/midpoints.js @@ -54,16 +54,13 @@ export function svgMidpoints(projection, context) { function drawMidpoints(selection, graph, entities, filter, extent) { - var layer = selection.selectAll('.layer-points .layer-points-midpoints'); + var drawLayer = selection.selectAll('.layer-osm.points .points-group.midpoints'); + var touchLayer = selection.selectAll('.layer-touch.points'); var mode = context.mode(); if (mode && mode.id !== 'select') { - layer.selectAll('g.midpoint') - .remove(); - - selection.selectAll('.layer-points .layer-points-targets .midpoint.target') - .remove(); - + drawLayer.selectAll('.midpoint').remove(); + touchLayer.selectAll('.midpoint.target').remove(); return; } @@ -73,51 +70,45 @@ export function svgMidpoints(projection, context) { for (var i = 0; i < entities.length; i++) { var entity = entities[i]; - if (entity.type !== 'way') - continue; - if (!filter(entity)) - continue; - if (context.selectedIDs().indexOf(entity.id) < 0) - continue; + if (entity.type !== 'way') continue; + if (!filter(entity)) continue; + if (context.selectedIDs().indexOf(entity.id) < 0) continue; var nodes = graph.childNodes(entity); for (var j = 0; j < nodes.length - 1; j++) { - var a = nodes[j]; var b = nodes[j + 1]; var id = [a.id, b.id].sort().join('-'); if (midpoints[id]) { midpoints[id].parents.push(entity); - } else { - if (geoVecLength(projection(a.loc), projection(b.loc)) > 40) { - var point = geoVecInterp(a.loc, b.loc, 0.5); - var loc = null; - - if (extent.intersects(point)) { - loc = point; - } else { - for (var k = 0; k < 4; k++) { - point = geoLineIntersection([a.loc, b.loc], [poly[k], poly[k + 1]]); - if (point && - geoVecLength(projection(a.loc), projection(point)) > 20 && - geoVecLength(projection(b.loc), projection(point)) > 20) - { - loc = point; - break; - } + } else if (geoVecLength(projection(a.loc), projection(b.loc)) > 40) { + var point = geoVecInterp(a.loc, b.loc, 0.5); + var loc = null; + + if (extent.intersects(point)) { + loc = point; + } else { + for (var k = 0; k < 4; k++) { + point = geoLineIntersection([a.loc, b.loc], [poly[k], poly[k + 1]]); + if (point && + geoVecLength(projection(a.loc), projection(point)) > 20 && + geoVecLength(projection(b.loc), projection(point)) > 20) + { + loc = point; + break; } } + } - if (loc) { - midpoints[id] = { - type: 'midpoint', - id: id, - loc: loc, - edge: [a.id, b.id], - parents: [entity] - }; - } + if (loc) { + midpoints[id] = { + type: 'midpoint', + id: id, + loc: loc, + edge: [a.id, b.id], + parents: [entity] + }; } } } @@ -138,8 +129,7 @@ export function svgMidpoints(projection, context) { } - var groups = layer - .selectAll('g.midpoint') + var groups = drawLayer.selectAll('.midpoint') .filter(midpointFilter) .data(_values(midpoints), function(d) { return d.id; }); @@ -179,7 +169,7 @@ export function svgMidpoints(projection, context) { // Draw touch targets.. - selection.selectAll('.layer-points .layer-points-targets') + touchLayer .call(drawTargets, graph, _values(midpoints), midpointFilter); } diff --git a/modules/svg/osm.js b/modules/svg/osm.js index a580e2a737..7eee64afe1 100644 --- a/modules/svg/osm.js +++ b/modules/svg/osm.js @@ -7,36 +7,18 @@ export function svgOsm(projection, context, dispatch) { .data(['covered', 'areas', 'lines', 'points', 'labels']) .enter() .append('g') - .attr('class', function(d) { return 'layer-osm layer-' + d; }); + .attr('class', function(d) { return 'layer-osm ' + d; }); - selection.selectAll('.layer-areas').selectAll('.layer-areas-group') - .data(['areas', 'targets']) + selection.selectAll('.layer-osm.points').selectAll('.points-group') + .data(['points', 'midpoints', 'vertices', 'turns']) .enter() .append('g') - .attr('class', function(d) { return 'layer-areas-group layer-areas-' + d; }); - - selection.selectAll('.layer-lines').selectAll('.layer-lines-group') - .data(['lines', 'targets']) - .enter() - .append('g') - .attr('class', function(d) { return 'layer-lines-group layer-lines-' + d; }); - - selection.selectAll('.layer-points').selectAll('.layer-points-group') - .data(['points', 'midpoints', 'vertices', 'turns', 'targets']) - .enter() - .append('g') - .attr('class', function(d) { return 'layer-points-group layer-points-' + d; }); - - selection.selectAll('.layer-labels').selectAll('.layer-labels-group') - .data(['halo', 'label', 'debug']) - .enter() - .append('g') - .attr('class', function(d) { return 'layer-labels-group layer-labels-' + d; }); + .attr('class', function(d) { return 'points-group ' + d; }); } function showLayer() { - var layer = context.surface().selectAll('.data-layer-osm'); + var layer = context.surface().selectAll('.data-layer.osm'); layer.interrupt(); layer @@ -52,7 +34,7 @@ export function svgOsm(projection, context, dispatch) { function hideLayer() { - var layer = context.surface().selectAll('.data-layer-osm'); + var layer = context.surface().selectAll('.data-layer.osm'); layer.interrupt(); layer diff --git a/modules/svg/points.js b/modules/svg/points.js index 06cec1c93a..24d185a507 100644 --- a/modules/svg/points.js +++ b/modules/svg/points.js @@ -71,21 +71,22 @@ export function svgPoints(projection, context) { var wireframe = context.surface().classed('fill-wireframe'); var zoom = geoScaleToZoom(projection.scale()); - // points with a direction will render as vertices at higher zooms + // Points with a direction will render as vertices at higher zooms.. function renderAsPoint(entity) { return entity.geometry(graph) === 'point' && !(zoom >= 18 && entity.directions(graph, projection).length); } - // all points will render as vertices in wireframe mode too + // All points will render as vertices in wireframe mode too.. var points = wireframe ? [] : entities.filter(renderAsPoint); - points.sort(sortY); - var layer = selection.selectAll('.layer-points .layer-points-points'); + var drawLayer = selection.selectAll('.layer-osm.points .points-group.points'); + var touchLayer = selection.selectAll('.layer-touch.points'); - var groups = layer.selectAll('g.point') + // Draw points.. + var groups = drawLayer.selectAll('g.point') .filter(filter) .data(points, fastEntityKey); @@ -134,17 +135,17 @@ export function svgPoints(projection, context) { var preset = context.presets().match(entity, graph); var picon = preset && preset.icon; - if (!picon) + if (!picon) { return ''; - else { + } else { var isMaki = /^maki-/.test(picon); return '#' + picon + (isMaki ? '-11' : ''); } }); - // touch targets - selection.selectAll('.layer-points .layer-points-targets') + // Draw touch targets.. + touchLayer .call(drawTargets, graph, points, filter); } diff --git a/modules/svg/touch.js b/modules/svg/touch.js new file mode 100644 index 0000000000..33290f744c --- /dev/null +++ b/modules/svg/touch.js @@ -0,0 +1,12 @@ +export function svgTouch() { + + function drawTouch(selection) { + selection.selectAll('.layer-touch') + .data(['areas', 'lines', 'points', 'notes']) + .enter() + .append('g') + .attr('class', function(d) { return 'layer-touch ' + d; }); + } + + return drawTouch; +} diff --git a/modules/svg/turns.js b/modules/svg/turns.js index 2d3ace24c9..c5aefc7bcb 100644 --- a/modules/svg/turns.js +++ b/modules/svg/turns.js @@ -12,16 +12,9 @@ export function svgTurns(projection) { return '#iD-turn-yes' + u; } - var layer = selection.selectAll('.data-layer-osm').selectAll('.layer-turns') - .data([0]); + var drawLayer = selection.selectAll('.layer-osm.points .points-group.turns'); - layer = layer.enter() - .append('g') - .attr('class', 'layer-osm layer-turns') - .merge(layer); - - - var groups = layer.selectAll('g.turn') + var groups = drawLayer.selectAll('g.turn') .data(turns, function(d) { return d.key; }); groups.exit() diff --git a/modules/svg/vertices.js b/modules/svg/vertices.js index d518e8b8e0..521a32a09c 100644 --- a/modules/svg/vertices.js +++ b/modules/svg/vertices.js @@ -326,6 +326,9 @@ export function svgVertices(projection, context) { var mode = context.mode(); var isMoving = mode && /^(add|draw|drag|move|rotate)/.test(mode.id); + var drawLayer = selection.selectAll('.layer-osm.points .points-group.vertices'); + var touchLayer = selection.selectAll('.layer-touch.points'); + if (fullRedraw) { _currPersistent = {}; _radii = {}; @@ -371,7 +374,7 @@ export function svgVertices(projection, context) { var filterRendered = function(d) { return d.id in _currPersistent || d.id in _currSelected || d.id in _currHover || filter(d); }; - selection.selectAll('.layer-points .layer-points-vertices') + drawLayer .call(draw, graph, currentVisible(all), sets, filterRendered); // Draw touch targets.. @@ -379,7 +382,7 @@ export function svgVertices(projection, context) { var filterTouch = function(d) { return isMoving ? true : filterRendered(d); }; - selection.selectAll('.layer-points .layer-points-targets') + touchLayer .call(drawTargets, graph, currentVisible(all), filterTouch); diff --git a/modules/ui/fields/restrictions.js b/modules/ui/fields/restrictions.js index e11189ca13..eb377eb10f 100644 --- a/modules/ui/fields/restrictions.js +++ b/modules/ui/fields/restrictions.js @@ -268,7 +268,7 @@ export function uiFieldRestrictions(field, context) { .translate(geoVecSubtract(c, extentCenter)) .clipExtent([[0, 0], d]); - var drawLayers = svgLayers(projection, context).only('osm').dimensions(d); + var drawLayers = svgLayers(projection, context).only(['osm','touch']).dimensions(d); var drawVertices = svgVertices(projection, context); var drawLines = svgLines(projection, context); var drawTurns = svgTurns(projection, context); diff --git a/test/spec/behavior/select.js b/test/spec/behavior/select.js index bcd8370402..7771ebd89b 100644 --- a/test/spec/behavior/select.js +++ b/test/spec/behavior/select.js @@ -14,7 +14,7 @@ describe('iD.behaviorSelect', function() { .append('div') .attr('class', 'inspector-wrap'); - context.surface().select('.data-layer-osm').selectAll('circle') + context.surface().select('.data-layer.osm').selectAll('circle') .data([a, b]) .enter().append('circle') .attr('class', function(d) { return d.id; }); diff --git a/test/spec/svg/layers.js b/test/spec/svg/layers.js index cc327edf1c..2a574c1c1c 100644 --- a/test/spec/svg/layers.js +++ b/test/spec/svg/layers.js @@ -26,15 +26,16 @@ describe('iD.svgLayers', function () { it('creates default data layers', function () { container.call(iD.svgLayers(projection, context)); var nodes = container.selectAll('svg .data-layer').nodes(); - expect(nodes.length).to.eql(8); - expect(d3.select(nodes[0]).classed('data-layer-osm')).to.be.true; - expect(d3.select(nodes[1]).classed('data-layer-notes')).to.be.true; - expect(d3.select(nodes[2]).classed('data-layer-data')).to.be.true; - expect(d3.select(nodes[3]).classed('data-layer-streetside')).to.be.true; - expect(d3.select(nodes[4]).classed('data-layer-mapillary-images')).to.be.true; - expect(d3.select(nodes[5]).classed('data-layer-mapillary-signs')).to.be.true; - expect(d3.select(nodes[6]).classed('data-layer-openstreetcam-images')).to.be.true; - expect(d3.select(nodes[7]).classed('data-layer-debug')).to.be.true; + expect(nodes.length).to.eql(9); + expect(d3.select(nodes[0]).classed('osm')).to.be.true; + expect(d3.select(nodes[1]).classed('notes')).to.be.true; + expect(d3.select(nodes[2]).classed('data')).to.be.true; + expect(d3.select(nodes[3]).classed('streetside')).to.be.true; + expect(d3.select(nodes[4]).classed('mapillary-images')).to.be.true; + expect(d3.select(nodes[5]).classed('mapillary-signs')).to.be.true; + expect(d3.select(nodes[6]).classed('openstreetcam-images')).to.be.true; + expect(d3.select(nodes[7]).classed('debug')).to.be.true; + expect(d3.select(nodes[8]).classed('touch')).to.be.true; }); }); diff --git a/test/spec/svg/osm.js b/test/spec/svg/osm.js index 84761c96f4..84a77346c5 100644 --- a/test/spec/svg/osm.js +++ b/test/spec/svg/osm.js @@ -9,31 +9,21 @@ describe('iD.svgOsm', function () { container.call(iD.svgOsm()); var layers = container.selectAll('g.layer-osm').nodes(); expect(layers.length).to.eql(5); - expect(d3.select(layers[0]).classed('layer-covered')).to.be.true; - expect(d3.select(layers[1]).classed('layer-areas')).to.be.true; - expect(d3.select(layers[2]).classed('layer-lines')).to.be.true; - expect(d3.select(layers[3]).classed('layer-points')).to.be.true; - expect(d3.select(layers[4]).classed('layer-labels')).to.be.true; + expect(d3.select(layers[0]).classed('covered')).to.be.true; + expect(d3.select(layers[1]).classed('areas')).to.be.true; + expect(d3.select(layers[2]).classed('lines')).to.be.true; + expect(d3.select(layers[3]).classed('points')).to.be.true; + expect(d3.select(layers[4]).classed('labels')).to.be.true; }); it('creates default osm point layers', function () { container.call(iD.svgOsm()); - var layers = container.selectAll('g.layer-points g.layer-points-group').nodes(); - expect(layers.length).to.eql(5); - expect(d3.select(layers[0]).classed('layer-points-points')).to.be.true; - expect(d3.select(layers[1]).classed('layer-points-midpoints')).to.be.true; - expect(d3.select(layers[2]).classed('layer-points-vertices')).to.be.true; - expect(d3.select(layers[3]).classed('layer-points-turns')).to.be.true; - expect(d3.select(layers[4]).classed('layer-points-targets')).to.be.true; - }); - - it('creates default osm label layers', function () { - container.call(iD.svgOsm()); - var layers = container.selectAll('g.layer-labels g.layer-labels-group').nodes(); - expect(layers.length).to.eql(3); - expect(d3.select(layers[0]).classed('layer-labels-halo')).to.be.true; - expect(d3.select(layers[1]).classed('layer-labels-label')).to.be.true; - expect(d3.select(layers[2]).classed('layer-labels-debug')).to.be.true; + var layers = container.selectAll('g.points-group').nodes(); + expect(layers.length).to.eql(4); + expect(d3.select(layers[0]).classed('points')).to.be.true; + expect(d3.select(layers[1]).classed('midpoints')).to.be.true; + expect(d3.select(layers[2]).classed('vertices')).to.be.true; + expect(d3.select(layers[3]).classed('turns')).to.be.true; }); }); From cc71f924f347bc6adac5bf3c457f492277b37b3b Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 9 Nov 2018 22:54:07 -0500 Subject: [PATCH 2/6] Fix stacking order in turn restriction editor --- css/20_map.css | 14 ++--- modules/svg/touch.js | 2 +- modules/svg/turns.js | 135 ++++++++++++++++++++++++++++--------------- 3 files changed, 95 insertions(+), 56 deletions(-) diff --git a/css/20_map.css b/css/20_map.css index a4206260e5..a36d9237ba 100644 --- a/css/20_map.css +++ b/css/20_map.css @@ -33,7 +33,8 @@ /* `.target` objects are interactive */ /* They can be picked up, clicked, hovered, or things can connect to them */ -.node.target { +.node.target, +.turn .target { pointer-events: fill; fill-opacity: 0.8; fill: currentColor; @@ -50,6 +51,7 @@ stroke-linejoin: round; } + /* `.target-nope` objects are explicitly forbidden to join to */ .surface:not(.nope-disabled) .node.target.target-nope, .surface:not(.nope-disabled) .way.target.target-nope { @@ -269,15 +271,9 @@ g.vertex.highlighted .shadow { } /* Turn Restrictions */ -g.turn rect, -g.turn circle { +.points-group.turns g.turn rect, +.points-group.turns g.turn circle { fill: none; - pointer-events: all; -} - -.form-field-restrictions .vertex { - cursor: auto !important; - pointer-events: none; } /* Turn restriction paths and vertices */ diff --git a/modules/svg/touch.js b/modules/svg/touch.js index 33290f744c..96bb1c8710 100644 --- a/modules/svg/touch.js +++ b/modules/svg/touch.js @@ -2,7 +2,7 @@ export function svgTouch() { function drawTouch(selection) { selection.selectAll('.layer-touch') - .data(['areas', 'lines', 'points', 'notes']) + .data(['areas', 'lines', 'points', 'turns', 'notes']) .enter() .append('g') .attr('class', function(d) { return 'layer-touch ' + d; }); diff --git a/modules/svg/turns.js b/modules/svg/turns.js index c5aefc7bcb..a0bc7e173e 100644 --- a/modules/svg/turns.js +++ b/modules/svg/turns.js @@ -1,45 +1,69 @@ import { geoAngle, geoPathLength } from '../geo'; -export function svgTurns(projection) { - - return function drawTurns(selection, graph, turns) { - - function icon(turn) { - var u = turn.u ? '-u' : ''; - if (turn.no) return '#iD-turn-no' + u; - if (turn.only) return '#iD-turn-only' + u; - return '#iD-turn-yes' + u; +export function svgTurns(projection, context) { + + function icon(turn) { + var u = turn.u ? '-u' : ''; + if (turn.no) return '#iD-turn-no' + u; + if (turn.only) return '#iD-turn-only' + u; + return '#iD-turn-yes' + u; + } + + function drawTurns(selection, graph, turns) { + + function turnTransform(d) { + var pxRadius = 50; + var toWay = graph.entity(d.to.way); + var toPoints = graph.childNodes(toWay) + .map(function (n) { return n.loc; }) + .map(projection); + var toLength = geoPathLength(toPoints); + var mid = toLength / 2; // midpoint of destination way + + var toNode = graph.entity(d.to.node); + var toVertex = graph.entity(d.to.vertex); + var a = geoAngle(toVertex, toNode, projection); + var o = projection(toVertex.loc); + var r = d.u ? 0 // u-turn: no radius + : !toWay.__via ? pxRadius // leaf way: put marker at pxRadius + : Math.min(mid, pxRadius); // via way: prefer pxRadius, fallback to mid for very short ways + + return 'translate(' + (r * Math.cos(a) + o[0]) + ',' + (r * Math.sin(a) + o[1]) + ') ' + + 'rotate(' + a * 180 / Math.PI + ')'; } + var drawLayer = selection.selectAll('.layer-osm.points .points-group.turns'); + var touchLayer = selection.selectAll('.layer-touch.turns'); + // Draw turns.. var groups = drawLayer.selectAll('g.turn') .data(turns, function(d) { return d.key; }); + // exit groups.exit() .remove(); - - var enter = groups.enter() + // enter + var groupsEnter = groups.enter() .append('g') .attr('class', function(d) { return 'turn ' + d.key; }); - var nEnter = enter + var turnsEnter = groupsEnter .filter(function(d) { return !d.u; }); - nEnter.append('rect') + turnsEnter.append('rect') .attr('transform', 'translate(-22, -12)') .attr('width', '44') .attr('height', '24'); - nEnter.append('use') + turnsEnter.append('use') .attr('transform', 'translate(-22, -12)') .attr('width', '44') .attr('height', '24'); - - var uEnter = enter + var uEnter = groupsEnter .filter(function(d) { return d.u; }); uEnter.append('circle') @@ -50,41 +74,60 @@ export function svgTurns(projection) { .attr('width', '32') .attr('height', '32'); - + // update groups = groups - .merge(enter); - - groups - .attr('opacity', function(d) { - return d.direct === false ? '0.7' : null; - }) - .attr('transform', function(d) { - var pxRadius = 50; - var toWay = graph.entity(d.to.way); - var toPoints = graph.childNodes(toWay) - .map(function (n) { return n.loc; }) - .map(projection); - var toLength = geoPathLength(toPoints); - var mid = toLength / 2; // midpoint of destination way - - var toNode = graph.entity(d.to.node); - var toVertex = graph.entity(d.to.vertex); - var a = geoAngle(toVertex, toNode, projection); - var o = projection(toVertex.loc); - var r = d.u ? 0 // u-turn: no radius - : !toWay.__via ? pxRadius // leaf way: put marker at pxRadius - : Math.min(mid, pxRadius); // via way: prefer pxRadius, fallback to mid for very short ways - - return 'translate(' + (r * Math.cos(a) + o[0]) + ',' + (r * Math.sin(a) + o[1]) + ') ' + - 'rotate(' + a * 180 / Math.PI + ')'; - }); + .merge(groupsEnter) + .attr('opacity', function(d) { return d.direct === false ? '0.7' : null; }) + .attr('transform', turnTransform); groups.select('use') .attr('xlink:href', icon); - groups.select('rect'); - groups.select('circle'); + groups.select('rect'); // propagate bound data + groups.select('circle'); // propagate bound data + + + // Draw touch targets.. + var fillClass = context.getDebug('target') ? 'pink ' : 'nocolor '; + groups = touchLayer.selectAll('g.turn') + .data(turns, function(d) { return d.key; }); + + // exit + groups.exit() + .remove(); + + // enter + groupsEnter = groups.enter() + .append('g') + .attr('class', function(d) { return 'turn ' + d.key; }); + + turnsEnter = groupsEnter + .filter(function(d) { return !d.u; }); + + turnsEnter.append('rect') + .attr('class', 'target ' + fillClass) + .attr('transform', 'translate(-22, -12)') + .attr('width', '44') + .attr('height', '24'); + + uEnter = groupsEnter + .filter(function(d) { return d.u; }); + + uEnter.append('circle') + .attr('class', 'target ' + fillClass) + .attr('r', '16'); + + // update + groups = groups + .merge(groupsEnter) + .attr('transform', turnTransform); + + groups.select('rect'); // propagate bound data + groups.select('circle'); // propagate bound data + return this; - }; + } + + return drawTurns; } From 558dc031852a0d609788505912ea9871d8ee4761 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 9 Nov 2018 23:24:40 -0500 Subject: [PATCH 3/6] Show active vertex while dragging in wireframe mode --- css/70_fills.css | 1 - 1 file changed, 1 deletion(-) diff --git a/css/70_fills.css b/css/70_fills.css index fa6e58cf8a..4845d8e7e7 100644 --- a/css/70_fills.css +++ b/css/70_fills.css @@ -27,7 +27,6 @@ } .fill-wireframe .point, -.fill-wireframe .vertex.active, .fill-wireframe .areaicon, .fill-wireframe .areaicon-halo, .fill-wireframe path.casing, From 05a5563a413bc9209714c26f14f6ff2f9365977a Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sat, 10 Nov 2018 00:05:53 -0500 Subject: [PATCH 4/6] Use a transient to memoize preset.match This slightly speeds up some things, including label rendering --- modules/presets/index.js | 56 +++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/modules/presets/index.js b/modules/presets/index.js index c4c9150977..70038eef78 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -35,42 +35,44 @@ export function presetIndex() { }; all.match = function(entity, resolver) { - var geometry = entity.geometry(resolver); - var address; + return resolver.transient(entity, 'presetMatch', function() { + var geometry = entity.geometry(resolver); + var address; - // Treat entities on addr:interpolation lines as points, not vertices - #3241 - if (geometry === 'vertex' && entity.isOnAddressLine(resolver)) { - geometry = 'point'; - } + // Treat entities on addr:interpolation lines as points, not vertices - #3241 + if (geometry === 'vertex' && entity.isOnAddressLine(resolver)) { + geometry = 'point'; + } - var geometryMatches = _index[geometry]; - var best = -1; - var match; + var geometryMatches = _index[geometry]; + var best = -1; + var match; - for (var k in entity.tags) { - // If any part of an address is present, - // allow fallback to "Address" preset - #4353 - if (k.match(/^addr:/) !== null && geometryMatches['addr:*']) { - address = geometryMatches['addr:*'][0]; - } + for (var k in entity.tags) { + // If any part of an address is present, + // allow fallback to "Address" preset - #4353 + if (k.match(/^addr:/) !== null && geometryMatches['addr:*']) { + address = geometryMatches['addr:*'][0]; + } - var keyMatches = geometryMatches[k]; - if (!keyMatches) continue; + var keyMatches = geometryMatches[k]; + if (!keyMatches) continue; - for (var i = 0; i < keyMatches.length; i++) { - var score = keyMatches[i].matchScore(entity); - if (score > best) { - best = score; - match = keyMatches[i]; + for (var i = 0; i < keyMatches.length; i++) { + var score = keyMatches[i].matchScore(entity); + if (score > best) { + best = score; + match = keyMatches[i]; + } } } - } - if (address && (!match || match.isFallback())) { - match = address; - } + if (address && (!match || match.isFallback())) { + match = address; + } - return match || all.item(geometry); + return match || all.item(geometry); + }); }; From 4a3d5e231640bc42cff09c42cbdfd12900a981ec Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sat, 10 Nov 2018 00:40:45 -0500 Subject: [PATCH 5/6] Avoid reflow caused by restriction editor checking its dimensions --- modules/ui/fields/restrictions.js | 9 ++++++++- modules/ui/init.js | 8 ++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/modules/ui/fields/restrictions.js b/modules/ui/fields/restrictions.js index eb377eb10f..c0104a4d88 100644 --- a/modules/ui/fields/restrictions.js +++ b/modules/ui/fields/restrictions.js @@ -235,7 +235,14 @@ export function uiFieldRestrictions(field, context) { var filter = utilFunctor(true); var projection = geoRawMercator(); - var d = utilGetDimensions(selection); + // Reflow warning: `utilGetDimensions` calls `getBoundingClientRect` + // Instead of asking the restriction-container for its dimensions, + // we can ask the #sidebar, which can have its dimensions cached. + // width: calc as sidebar - padding + // height: hardcoded (from `80_app.css`) + // var d = utilGetDimensions(selection); + var sdims = utilGetDimensions(d3_select('#sidebar')); + var d = [ sdims[0] - 50, 370 ]; var c = geoVecScale(d, 0.5); var z = 22; diff --git a/modules/ui/init.js b/modules/ui/init.js index 48977559f0..497201b0a5 100644 --- a/modules/ui/init.js +++ b/modules/ui/init.js @@ -390,8 +390,12 @@ export function uiInit(context) { ui.onResize = function(withPan) { var map = context.map(); - var content = d3_select('#content'); - var mapDimensions = utilGetDimensions(content, true); + + // Recalc dimensions of map and sidebar.. (`true` = force recalc) + // This will call `getBoundingClientRect` and trigger reflow, + // but the values will be cached for later use. + var mapDimensions = utilGetDimensions(d3_select('#content'), true); + utilGetDimensions(d3_select('#sidebar'), true); if (withPan !== undefined) { map.redrawEnable(false); From 9cd40f22be53bed307b2c280654fee3c56082c40 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 12 Nov 2018 12:17:12 -0500 Subject: [PATCH 6/6] When there is no activeID, we can memoize svgSegmentWay --- modules/svg/helpers.js | 124 ++++++++++++++++++++++------------------- 1 file changed, 66 insertions(+), 58 deletions(-) diff --git a/modules/svg/helpers.js b/modules/svg/helpers.js index 6c9bd344a7..7449a65189 100644 --- a/modules/svg/helpers.js +++ b/modules/svg/helpers.js @@ -214,67 +214,75 @@ export function svgRelationMemberTags(graph) { export function svgSegmentWay(way, graph, activeID) { - var isActiveWay = (way.nodes.indexOf(activeID) !== -1); - var features = { passive: [], active: [] }; - var start = {}; - var end = {}; - var node, type; - - for (var i = 0; i < way.nodes.length; i++) { - node = graph.entity(way.nodes[i]); - type = svgPassiveVertex(node, graph, activeID); - end = { node: node, type: type }; - - if (start.type !== undefined) { - if (start.node.id === activeID || end.node.id === activeID) { - // push nothing - } else if (isActiveWay && (start.type === 2 || end.type === 2)) { // one adjacent vertex - pushActive(start, end, i); - } else if (start.type === 0 && end.type === 0) { // both active vertices - pushActive(start, end, i); - } else { - pushPassive(start, end, i); - } - } - - start = end; + // When there is no activeID, we can memoize this expensive computation + if (activeID === undefined) { + return graph.transient(way, 'waySegments', getWaySegments); + } else { + return getWaySegments(); } - return features; - - - function pushActive(start, end, index) { - features.active.push({ - type: 'Feature', - id: way.id + '-' + index + '-nope', - properties: { - nope: true, - target: true, - entity: way, - nodes: [start.node, end.node], - index: index - }, - geometry: { - type: 'LineString', - coordinates: [start.node.loc, end.node.loc] + function getWaySegments() { + var isActiveWay = (way.nodes.indexOf(activeID) !== -1); + var features = { passive: [], active: [] }; + var start = {}; + var end = {}; + var node, type; + + for (var i = 0; i < way.nodes.length; i++) { + node = graph.entity(way.nodes[i]); + type = svgPassiveVertex(node, graph, activeID); + end = { node: node, type: type }; + + if (start.type !== undefined) { + if (start.node.id === activeID || end.node.id === activeID) { + // push nothing + } else if (isActiveWay && (start.type === 2 || end.type === 2)) { // one adjacent vertex + pushActive(start, end, i); + } else if (start.type === 0 && end.type === 0) { // both active vertices + pushActive(start, end, i); + } else { + pushPassive(start, end, i); + } } - }); - } - function pushPassive(start, end, index) { - features.passive.push({ - type: 'Feature', - id: way.id + '-' + index, - properties: { - target: true, - entity: way, - nodes: [start.node, end.node], - index: index - }, - geometry: { - type: 'LineString', - coordinates: [start.node.loc, end.node.loc] - } - }); + start = end; + } + + return features; + + function pushActive(start, end, index) { + features.active.push({ + type: 'Feature', + id: way.id + '-' + index + '-nope', + properties: { + nope: true, + target: true, + entity: way, + nodes: [start.node, end.node], + index: index + }, + geometry: { + type: 'LineString', + coordinates: [start.node.loc, end.node.loc] + } + }); + } + + function pushPassive(start, end, index) { + features.passive.push({ + type: 'Feature', + id: way.id + '-' + index, + properties: { + target: true, + entity: way, + nodes: [start.node, end.node], + index: index + }, + geometry: { + type: 'LineString', + coordinates: [start.node.loc, end.node.loc] + } + }); + } } }