From 4d8981072134a41bb224c3c8f423c1d785396d73 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Tue, 15 Jan 2019 13:49:50 +0000 Subject: [PATCH 01/38] Add ImproveOSM service Only processing one-way errors for now --- css/20_map.css | 9 +- css/55_cursors.css | 2 + css/65_data.css | 17 ++- data/core.yaml | 3 + dist/locales/en.json | 4 + modules/osm/improveOSM.js | 50 +++++++ modules/osm/index.js | 1 + modules/services/improveOSM.js | 222 +++++++++++++++++++++++++++++ modules/services/index.js | 3 + modules/svg/improveOSM.js | 245 +++++++++++++++++++++++++++++++++ modules/svg/layers.js | 2 + modules/ui/map_data.js | 2 +- 12 files changed, 557 insertions(+), 3 deletions(-) create mode 100644 modules/osm/improveOSM.js create mode 100644 modules/services/improveOSM.js create mode 100644 modules/svg/improveOSM.js diff --git a/css/20_map.css b/css/20_map.css index 6c9ed5c4b..72d604a7e 100644 --- a/css/20_map.css +++ b/css/20_map.css @@ -33,7 +33,8 @@ /* No interactivity except what we specifically allow */ .data-layer.osm *, .data-layer.notes *, -.data-layer.keepRight * { +.data-layer.keepRight *, +.data-layer.improveOSM * { pointer-events: none; } @@ -44,6 +45,7 @@ /* `.target` objects are interactive */ /* They can be picked up, clicked, hovered, or things can connect to them */ +.iOSM_error.target, .kr_error.target, .note.target, .node.target, @@ -83,6 +85,7 @@ /* points, notes & QA */ /* points, notes, markers */ +g.iOSM_error .stroke, g.kr_error .stroke, g.note .stroke { stroke: #222; @@ -91,6 +94,7 @@ g.note .stroke { opacity: 0.6; } +g.iOSM_error.active .stroke, g.kr_error.active .stroke, g.note.active .stroke { stroke: #222; @@ -105,6 +109,7 @@ g.point .stroke { fill: #fff; } +g.iOSM_error .shadow, g.kr_error .shadow, g.point .shadow, g.note .shadow { @@ -114,6 +119,7 @@ g.note .shadow { stroke-opacity: 0; } +g.iOSM_error.hover:not(.selected) .shadow, g.kr_error.hover:not(.selected) .shadow, g.note.hover:not(.selected) .shadow, g.point.related:not(.selected) .shadow, @@ -121,6 +127,7 @@ g.point.hover:not(.selected) .shadow { stroke-opacity: 0.5; } +g.iOSM_error.selected .shadow, g.kr_error.selected .shadow, g.note.selected .shadow, g.point.selected .shadow { diff --git a/css/55_cursors.css b/css/55_cursors.css index f473301c4..efc293266 100644 --- a/css/55_cursors.css +++ b/css/55_cursors.css @@ -98,8 +98,10 @@ .mode-browse .note, .mode-browse .kr_error, +.mode-browse .iOSM_error, .mode-select .note, .mode-select .kr_error, +.mode-select .iOSM_error, .turn rect, .turn circle { cursor: pointer; diff --git a/css/65_data.css b/css/65_data.css index 71aafab9c..5aebce603 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -2,7 +2,8 @@ /* OSM Notes and KeepRight Layers */ .kr_error-header-icon .kr_error-fill, -.layer-keepRight .kr_error .kr_error-fill { +.layer-keepRight .kr_error .kr_error-fill, +.layer-improveOSM .iOSM_error .iOSM_error-fill { stroke: #333; stroke-width: 1.3px; /* NOTE: likely a better way to scale the icon stroke */ } @@ -115,6 +116,20 @@ color: #c35; } +/* ImproveOSM Errors +------------------------------------------------------- */ + +.iOSM_error_type_ow { /* missing one way */ + color: #EE7600; +} + +.iOSM_error_type_mr { /* missing road */ + color: #B0171F; +} + +.iOSM_error_type_tr { /* missing turn restriction */ + color: #1E90FF; +} /* Custom Map Data (geojson, gpx, kml, vector tile) */ .layer-mapdata { diff --git a/data/core.yaml b/data/core.yaml index c28e04da6..0e9fd2fc8 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -491,6 +491,9 @@ en: keepRight: tooltip: Automatically detected map issues from keepright.at title: KeepRight Issues + improveOSM: + tooltip: Missing data automatically detected by improveosm.org + title: ImproveOSM Issues custom: tooltip: "Drag and drop a data file onto the page, or click the button to setup" title: Custom Map Data diff --git a/dist/locales/en.json b/dist/locales/en.json index 020477d53..e8667b2f6 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -596,6 +596,10 @@ "tooltip": "Automatically detected map issues from keepright.at", "title": "KeepRight Issues" }, + "improveOSM": { + "tooltip": "Missing data automatically detected by improveosm.org", + "title": "ImproveOSM Issues" + }, "custom": { "tooltip": "Drag and drop a data file onto the page, or click the button to setup", "title": "Custom Map Data", diff --git a/modules/osm/improveOSM.js b/modules/osm/improveOSM.js new file mode 100644 index 000000000..b337fcfb6 --- /dev/null +++ b/modules/osm/improveOSM.js @@ -0,0 +1,50 @@ +import _extend from 'lodash-es/extend'; + + +export function impOsmError() { + if (!(this instanceof impOsmError)) { + return (new impOsmError()).initialize(arguments); + } else if (arguments.length) { + this.initialize(arguments); + } +} + +// ImproveOSM has no error IDs unfortunately +// So no way to explicitly refer to each error in their DB +impOsmError.id = function() { + return impOsmError.id.next--; +}; + + +impOsmError.id.next = -1; + + +_extend(impOsmError.prototype, { + + type: 'impOsmError', + + initialize: function(sources) { + for (var i = 0; i < sources.length; ++i) { + var source = sources[i]; + for (var prop in source) { + if (Object.prototype.hasOwnProperty.call(source, prop)) { + if (source[prop] === undefined) { + delete this[prop]; + } else { + this[prop] = source[prop]; + } + } + } + } + + if (!this.id) { + this.id = impOsmError.id() + ''; // as string + } + + return this; + }, + + update: function(attrs) { + return impOsmError(this, attrs); // {v: 1 + (this.v || 0)} + } +}); diff --git a/modules/osm/index.js b/modules/osm/index.js index f8d7addc1..72f7b3ac4 100644 --- a/modules/osm/index.js +++ b/modules/osm/index.js @@ -1,6 +1,7 @@ export { osmChangeset } from './changeset'; export { osmEntity } from './entity'; export { krError } from './keepRight'; +export { impOsmError } from './improveOSM'; export { osmNode } from './node'; export { osmNote } from './note'; export { osmRelation } from './relation'; diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js new file mode 100644 index 000000000..f4695766a --- /dev/null +++ b/modules/services/improveOSM.js @@ -0,0 +1,222 @@ +import _extend from 'lodash-es/extend'; +import _find from 'lodash-es/find'; +import _forEach from 'lodash-es/forEach'; + +import rbush from 'rbush'; + +import { dispatch as d3_dispatch } from 'd3-dispatch'; +import { json as d3_json } from 'd3-request'; +import { request as d3_request } from 'd3-request'; + +import { geoExtent } from '../geo'; +import { impOsmError } from '../osm'; +import { t } from '../util/locale'; +import { utilRebind, utilTiler, utilQsString } from '../util'; + + +var tiler = utilTiler(); +var dispatch = d3_dispatch('loaded'); + +var _erCache; +var _erZoom = 14; + +var _impOsmUrls = { + ow: 'http://directionofflow.skobbler.net/directionOfFlowService', + mr: 'http://missingroads.skobbler.net/missingGeoService', + tr: 'http://turnrestrictionservice.skobbler.net/turnRestrictionService' +}; + +var _missingTypes = { + PARKING: 'Unmapped parking', + ROAD: 'Unmapped road(s)', + BOTH: 'Unmapped road(s) and parking', + PATH: 'Unmapped path(s)', + WATER: 'Unmapped water feature' // ? +}; + +function abortRequest(i) { + _forEach(i, function(v) { + if (v) { + v.abort(); + } + }); +} + +function abortUnwantedRequests(cache, tiles) { + _forEach(cache.inflight, function(v, k) { + var wanted = _find(tiles, function(tile) { + return k === tile.id; + }); + if (!wanted) { + abortRequest(v); + delete cache.inflight[k]; + } + }); +} + + +function encodeErrorRtree(d) { + return { minX: d.loc[0], minY: d.loc[1], maxX: d.loc[0], maxY: d.loc[1], data: d }; +} + + +// replace or remove error from rtree +function updateRtree(item, replace) { + _erCache.rtree.remove(item, function isEql(a, b) { + return a.data.id === b.data.id; + }); + + if (replace) { + _erCache.rtree.insert(item); + } +} + +export default { + init: function() { + if (!_erCache) { + this.reset(); + } + + this.event = utilRebind(this, dispatch, 'on'); + }, + + reset: function() { + if (_erCache) { + _forEach(_erCache.inflight, abortRequest); + } + _erCache = { + data: {}, + loaded: {}, + inflight: {}, + closed: {}, + rtree: rbush() + }; + }, + + loadErrors: function(projection) { + var options = { + client: 'iD', + confidenceLevel: 'C1', // most confident cases only for now + status: 'OPEN', + type: 'PARKING,ROAD,BOTH,PATH', // exclude WATER + zoom: '19' + }; + + // determine the needed tiles to cover the view + var tiles = tiler + .zoomExtent([_erZoom, _erZoom]) + .getTiles(projection); + + // abort inflight requests that are no longer needed + abortUnwantedRequests(_erCache, tiles); + + // issue new requests.. + tiles.forEach(function(tile) { + if (_erCache.loaded[tile.id] || _erCache.inflight[tile.id]) return; + + var rect = tile.extent.rectangle(); + var params = _extend({}, options, { east: rect[0], south: rect[3], west: rect[2], north: rect[1] }); + + // 3 separate requests to store for each tile + var requests = {}; + + // TODO: Just implement TRs and One-ways for now, much more simple + _forEach(_impOsmUrls, function(v, k) { + var url = v + '/search?' + utilQsString(params); + + if (k == 'mr' || k == 'tr') return + + requests[k] = d3_json(url, + function(err, data) { + delete _erCache.inflight[tile.id]; + + if (err) return; + _erCache.loaded[tile.id] = true; + + // Clusters are returned at low zoom + // if (data.clusters) { + // data.clusters.forEach(function(feature) { + // var loc = feature.point; + // var size = feature.size; + // }); + // } + + // Road segments at high zoom == oneways + if (data.roadSegments) { + data.roadSegments.forEach(function(feature) { + var loc = feature.points[0]; + + var d = new impOsmError({ + loc: [loc.lon, loc.lat], + comment: null, + description: '', + error_type: feature.type, + parent_error_type: k, + title: 'Missing One-way' + }); + + _erCache.data[d.id] = d; + _erCache.rtree.insert(encodeErrorRtree(d)); + }) + } + + // Tiles at high zoom == missing roads + // if (data.tiles) { + // data.tiles.forEach(function(feature) { + // // Get description based on type + // var desc = _missingTypes[feature.type]; + + + // var d = new impOsmError({ + // loc: [feature.x, feature.y], + // comment: null, + // description: desc || '', + // error_type: feature.type, + // parent_error_type: k, + // title: 'Missing Roads' + // }); + + // _erCache.data[d.id] = d; + // _erCache.rtree.insert(encodeErrorRtree(d)); + // }) + // } + + + // if (data.entities) { + // data.entities.forEach(function(feature) { + // var loc = feature.point; + + // var d = new impOsmError({ + // loc: [loc.lat, loc.lon], + // comment: null, + // description: desc || '', + // error_type: feature.turnType, + // parent_error_type: k, + // title: 'Missing Turn Restriction' + // }); + + // _erCache.data[d.id] = d; + // _erCache.rtree.insert(encodeErrorRtree(d)); + // }) + // } + } + ); + }); + + _erCache.inflight[tile.id] = requests; + dispatch.call('loaded'); + }) + }, + + // get all cached errors covering the viewport + getErrors: function(projection) { + var viewport = projection.clipExtent(); + var min = [viewport[0][0], viewport[1][1]]; + var max = [viewport[1][0], viewport[0][1]]; + var bbox = geoExtent(projection.invert(min), projection.invert(max)).bbox(); + + return _erCache.rtree.search(bbox).map(function(d) { + return d.data; + }); + } +}; diff --git a/modules/services/index.js b/modules/services/index.js index 8b75d7418..838ed435f 100644 --- a/modules/services/index.js +++ b/modules/services/index.js @@ -1,4 +1,5 @@ import serviceKeepRight from './keepRight'; +import serviceImproveOSM from './improveOSM'; import serviceMapillary from './mapillary'; import serviceMapRules from './maprules'; import serviceNominatim from './nominatim'; @@ -15,6 +16,7 @@ import serviceWikipedia from './wikipedia'; export var services = { geocoder: serviceNominatim, keepRight: serviceKeepRight, + improveOSM: serviceImproveOSM, mapillary: serviceMapillary, openstreetcam: serviceOpenstreetcam, osm: serviceOsm, @@ -29,6 +31,7 @@ export var services = { export { serviceKeepRight, + serviceImproveOSM, serviceMapillary, serviceMapRules, serviceNominatim, diff --git a/modules/svg/improveOSM.js b/modules/svg/improveOSM.js new file mode 100644 index 000000000..738b63533 --- /dev/null +++ b/modules/svg/improveOSM.js @@ -0,0 +1,245 @@ +import _throttle from 'lodash-es/throttle'; +import { select as d3_select } from 'd3-selection'; + +import { modeBrowse } from '../modes'; +import { svgPointTransform } from './index'; +import { services } from '../services'; + +var _improveOsmEnabled = false; +var _errorService; + + +export function svgImproveOSM(projection, context, dispatch) { + var throttledRedraw = _throttle(function () { dispatch.call('change'); }, 1000); + var minZoom = 12; + var touchLayer = d3_select(null); + var drawLayer = d3_select(null); + var _improveOsmVisible = false; + + + function markerPath(selection, klass) { + selection + .attr('class', klass) + .attr('transform', 'translate(-4, -24)') + .attr('d', 'M11.6,6.2H7.1l1.4-5.1C8.6,0.6,8.1,0,7.5,0H2.2C1.7,0,1.3,0.3,1.3,0.8L0,10.2c-0.1,0.6,0.4,1.1,0.9,1.1h4.6l-1.8,7.6C3.6,19.4,4.1,20,4.7,20c0.3,0,0.6-0.2,0.8-0.5l6.9-11.9C12.7,7,12.3,6.2,11.6,6.2z'); + } + + + // Loosely-coupled improveOSM service for fetching errors. + function getService() { + if (services.improveOSM && !_errorService) { + _errorService = services.improveOSM; + _errorService.on('loaded', throttledRedraw); + } else if (!services.improveOSM && _errorService) { + _errorService = null; + } + + return _errorService; + } + + + // Show the errors + function editOn() { + if (!_improveOsmVisible) { + _improveOsmVisible = true; + drawLayer + .style('display', 'block'); + } + } + + + // Immediately remove the errors and their touch targets + function editOff() { + if (_improveOsmVisible) { + _improveOsmVisible = false; + drawLayer + .style('display', 'none'); + drawLayer.selectAll('.iOSM_error') + .remove(); + touchLayer.selectAll('.iOSM_error') + .remove(); + } + } + + + // Enable the layer. This shows the errors and transitions them to visible. + function layerOn() { + editOn(); + + drawLayer + .style('opacity', 0) + .transition() + .duration(250) + .style('opacity', 1) + .on('end interrupt', function () { + dispatch.call('change'); + }); + } + + + // Disable the layer. This transitions the layer invisible and then hides the errors. + function layerOff() { + throttledRedraw.cancel(); + drawLayer.interrupt(); + touchLayer.selectAll('.iOSM_error') + .remove(); + + drawLayer + .transition() + .duration(250) + .style('opacity', 0) + .on('end interrupt', function () { + editOff(); + dispatch.call('change'); + }); + } + + + // Update the error markers + function updateMarkers() { + if (!_improveOsmVisible || !_improveOsmEnabled) return; + + var service = getService(); + var selectedID = context.selectedErrorID(); + var data = (service ? service.getErrors(projection) : []); + var getTransform = svgPointTransform(projection); + + // Draw markers.. + var markers = drawLayer.selectAll('.iOSM_error') + .data(data, function(d) { return d.id; }); + + // exit + markers.exit() + .remove(); + + // enter + var markersEnter = markers.enter() + .append('g') + .attr('class', function(d) { + return 'iOSM_error iOSM_error-' + d.id + ' iOSM_error_type_' + d.parent_error_type; } + ); + + markersEnter + .append('ellipse') + .attr('cx', 0.5) + .attr('cy', 1) + .attr('rx', 6.5) + .attr('ry', 3) + .attr('class', 'stroke'); + + markersEnter + .append('path') + .call(markerPath, 'shadow'); + + markersEnter + .append('use') + .attr('class', 'iOSM_error-fill') + .attr('width', '20px') + .attr('height', '20px') + .attr('x', '-8px') + .attr('y', '-22px') + .attr('xlink:href', '#iD-icon-bolt'); + + // update + markers + .merge(markersEnter) + .sort(sortY) + .classed('selected', function(d) { return d.id === selectedID; }) + .attr('transform', getTransform); + + + // Draw targets.. + if (touchLayer.empty()) return; + var fillClass = context.getDebug('target') ? 'pink ' : 'nocolor '; + + var targets = touchLayer.selectAll('.iOSM_error') + .data(data, function(d) { return d.id; }); + + // exit + targets.exit() + .remove(); + + // enter/update + targets.enter() + .append('rect') + .attr('width', '20px') + .attr('height', '20px') + .attr('x', '-8px') + .attr('y', '-22px') + .merge(targets) + .sort(sortY) + .attr('class', function(d) { + return 'iOSM_error target iOSM_error-' + d.id + ' ' + fillClass; + }) + .attr('transform', getTransform); + + + function sortY(a, b) { + return (a.id === selectedID) ? 1 + : (b.id === selectedID) ? -1 + : (a.severity === 'error' && b.severity !== 'error') ? 1 + : (b.severity === 'error' && a.severity !== 'error') ? -1 + : b.loc[1] - a.loc[1]; + } + } + + + // Draw the ImproveOSM layer and schedule loading errors and updating markers. + function drawImproveOSM(selection) { + var service = getService(); + + var surface = context.surface(); + if (surface && !surface.empty()) { + touchLayer = surface.selectAll('.data-layer.touch .layer-touch.markers'); + } + + drawLayer = selection.selectAll('.layer-improveOSM') + .data(service ? [0] : []); + + drawLayer.exit() + .remove(); + + drawLayer = drawLayer.enter() + .append('g') + .attr('class', 'layer-improveOSM') + .style('display', _improveOsmEnabled ? 'block' : 'none') + .merge(drawLayer); + + if (_improveOsmEnabled) { + if (service && ~~context.map().zoom() >= minZoom) { + editOn(); + service.loadErrors(projection); + updateMarkers(); + } else { + editOff(); + } + } + } + + + // Toggles the layer on and off + drawImproveOSM.enabled = function(val) { + if (!arguments.length) return _improveOsmEnabled; + + _improveOsmEnabled = val; + if (_improveOsmEnabled) { + layerOn(); + } else { + layerOff(); + if (context.selectedErrorID()) { + context.enter(modeBrowse(context)); + } + } + + dispatch.call('change'); + return this; + }; + + + drawImproveOSM.supported = function() { + return !!getService(); + }; + + + return drawImproveOSM; +} diff --git a/modules/svg/layers.js b/modules/svg/layers.js index 59c9ab22c..ffe380c89 100644 --- a/modules/svg/layers.js +++ b/modules/svg/layers.js @@ -11,6 +11,7 @@ import { svgData } from './data'; import { svgDebug } from './debug'; import { svgGeolocate } from './geolocate'; import { svgKeepRight } from './keepRight'; +import { svgImproveOSM } from './improveOSM'; import { svgStreetside } from './streetside'; import { svgMapillaryImages } from './mapillary_images'; import { svgMapillarySigns } from './mapillary_signs'; @@ -30,6 +31,7 @@ export function svgLayers(projection, context) { { id: 'notes', layer: svgNotes(projection, context, dispatch) }, { id: 'data', layer: svgData(projection, context, dispatch) }, { id: 'keepRight', layer: svgKeepRight(projection, context, dispatch) }, + { id: 'improveOSM', layer: svgImproveOSM(projection, context, dispatch) }, { id: 'streetside', layer: svgStreetside(projection, context, dispatch)}, { id: 'mapillary-images', layer: svgMapillaryImages(projection, context, dispatch) }, { id: 'mapillary-signs', layer: svgMapillarySigns(projection, context, dispatch) }, diff --git a/modules/ui/map_data.js b/modules/ui/map_data.js index 2f115a9f3..d32b5d530 100644 --- a/modules/ui/map_data.js +++ b/modules/ui/map_data.js @@ -227,7 +227,7 @@ export function uiMapData(context) { function drawQAItems(selection) { - var qaKeys = ['keepRight']; + var qaKeys = ['keepRight', 'improveOSM']; var qaLayers = layers.all().filter(function(obj) { return qaKeys.indexOf(obj.id) !== -1; }); var ul = selection From 4f2646d56731aa4316c08933aba4e240a84eac7d Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sun, 20 Jan 2019 13:21:26 +0000 Subject: [PATCH 02/38] Add basic title + description UI --- data/core.yaml | 12 +++ dist/locales/en.json | 17 +++++ modules/behavior/hover.js | 6 +- modules/behavior/select.js | 7 +- modules/modes/select_error.js | 50 ++++++++---- modules/services/improveOSM.js | 22 ++++++ modules/ui/improveOSM_details.js | 127 +++++++++++++++++++++++++++++++ modules/ui/improveOSM_editor.js | 90 ++++++++++++++++++++++ modules/ui/improveOSM_header.js | 66 ++++++++++++++++ modules/ui/index.js | 3 + 10 files changed, 382 insertions(+), 18 deletions(-) create mode 100644 modules/ui/improveOSM_details.js create mode 100644 modules/ui/improveOSM_editor.js create mode 100644 modules/ui/improveOSM_header.js diff --git a/data/core.yaml b/data/core.yaml index 0e9fd2fc8..6a4195511 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -661,6 +661,18 @@ en: cannot_zoom: "Cannot zoom out further in current mode." full_screen: Toggle Full Screen QA: + improveOSM: + title: ImproveOSM Error + error_types: + ow: + title: Missing One-way + description: Aggregate data suggests {var1} may be missing a "oneway" tag. Please confirm via imagery or survey before mapping. + mr: + title: Missing Road + description: Aggregate data suggests there may be an unmapped road here. Please confirm via imagery or survey before mapping. + tr: + title: Missing Turn Restriction + description: Aggregate data suggests this junction may be missing a turn restriction. Please confirm via imagery or survey before mapping. keepRight: title: KeepRight Error detail_title: Error diff --git a/dist/locales/en.json b/dist/locales/en.json index e8667b2f6..bea513f1d 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -803,6 +803,23 @@ "cannot_zoom": "Cannot zoom out further in current mode.", "full_screen": "Toggle Full Screen", "QA": { + "improveOSM": { + "title": "ImproveOSM Error", + "error_types": { + "ow": { + "title": "Missing One-way", + "description": "Aggregate data suggests {var1} may be missing a \"oneway\" tag. Please confirm via imagery or survey before mapping." + }, + "mr": { + "title": "Missing Road", + "description": "Aggregate data suggests there may be an unmapped road here. Please confirm via imagery or survey before mapping." + }, + "tr": { + "title": "Missing Turn Restriction", + "description": "Aggregate data suggests this junction may be missing a turn restriction. Please confirm via imagery or survey before mapping." + } + } + }, "keepRight": { "title": "KeepRight Error", "detail_title": "Error", diff --git a/modules/behavior/hover.js b/modules/behavior/hover.js index 58b9187a9..627fba543 100644 --- a/modules/behavior/hover.js +++ b/modules/behavior/hover.js @@ -5,7 +5,7 @@ import { select as d3_select } from 'd3-selection'; -import { osmEntity, osmNote, krError } from '../osm'; +import { osmEntity, osmNote, krError, impOsmError } from '../osm'; import { utilKeybinding, utilRebind } from '../util'; @@ -112,6 +112,10 @@ export function behaviorHover(context) { entity = datum; selector = '.data' + datum.__featurehash__; + } else if (datum instanceof impOsmError) { + entity = datum; + selector = '.iOSM_error-' + datum.id; + } else if (datum instanceof krError) { entity = datum; selector = '.kr_error-' + datum.id; diff --git a/modules/behavior/select.js b/modules/behavior/select.js index 8c445930d..427d6b3b9 100644 --- a/modules/behavior/select.js +++ b/modules/behavior/select.js @@ -19,6 +19,7 @@ import { import { osmEntity, osmNote, + impOsmError, krError } from '../osm'; @@ -170,10 +171,14 @@ export function behaviorSelect(context) { context .selectedNoteID(datum.id) .enter(modeSelectNote(context, datum.id)); + } else if (datum instanceof impOsmError & !isMultiselect) { // clicked an improveOSM error + context + .selectedErrorID(datum.id) + .enter(modeSelectError(context, datum.id, 'ImproveOSM')); } else if (datum instanceof krError & !isMultiselect) { // clicked a krError error context .selectedErrorID(datum.id) - .enter(modeSelectError(context, datum.id)); + .enter(modeSelectError(context, datum.id, 'KeepRight')); } else { // clicked nothing.. context.selectedNoteID(null); context.selectedErrorID(null); diff --git a/modules/modes/select_error.js b/modules/modes/select_error.js index dbdae8aff..4472a7da5 100644 --- a/modules/modes/select_error.js +++ b/modules/modes/select_error.js @@ -13,26 +13,44 @@ import { import { t } from '../util/locale'; import { services } from '../services'; import { modeBrowse, modeDragNode, modeDragNote } from '../modes'; -import { uiKeepRightEditor } from '../ui'; +import { uiImproveOsmEditor, uiKeepRightEditor } from '../ui'; import { utilKeybinding } from '../util'; -export function modeSelectError(context, selectedErrorID) { +export function modeSelectError(context, selectedErrorID, selectedErrorSource) { var mode = { id: 'select-error', button: 'browse' }; - var keepRight = services.keepRight; var keybinding = utilKeybinding('select-error'); - var keepRightEditor = uiKeepRightEditor(context) - .on('change', function() { - context.map().pan([0,0]); // trigger a redraw - var error = checkSelectedID(); - if (!error) return; - context.ui().sidebar - .show(keepRightEditor.error(error)); - }); + + var errorService, errorEditor; + switch (selectedErrorSource) { + case 'ImproveOSM': + errorService = services.improveOSM; + errorEditor = uiImproveOsmEditor(context) + .on('change', function() { + context.map().pan([0,0]); // trigger a redraw + var error = checkSelectedID(); + if (!error) return; + context.ui().sidebar + .show(errorEditor.error(error)); + }); + break; + case 'KeepRight': + errorService = services.keepRight; + errorEditor = uiKeepRightEditor(context) + .on('change', function() { + context.map().pan([0,0]); // trigger a redraw + var error = checkSelectedID(); + if (!error) return; + context.ui().sidebar + .show(errorEditor.error(error)); + }); + break; + } + var behaviors = [ behaviorBreathe(context), @@ -45,8 +63,8 @@ export function modeSelectError(context, selectedErrorID) { function checkSelectedID() { - if (!keepRight) return; - var error = keepRight.getError(selectedErrorID); + if (!errorService) return; + var error = errorService.getError(selectedErrorID); if (!error) { context.enter(modeBrowse(context)); } @@ -55,8 +73,8 @@ export function modeSelectError(context, selectedErrorID) { mode.zoomToSelected = function() { - if (!keepRight) return; - var error = keepRight.getError(selectedErrorID); + if (!errorService) return; + var error = errorService.getError(selectedErrorID); if (error) { context.map().centerZoomEase(error.loc, 20); } @@ -78,7 +96,7 @@ export function modeSelectError(context, selectedErrorID) { selectError(); var sidebar = context.ui().sidebar; - sidebar.show(keepRightEditor.error(error)); + sidebar.show(errorEditor.error(error)); context.map() .on('drawn.select-error', selectError); diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index f4695766a..849749632 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -218,5 +218,27 @@ export default { return _erCache.rtree.search(bbox).map(function(d) { return d.data; }); + }, + + // get a single error from the cache + getError: function(id) { + return _erCache.data[id]; + }, + + // replace a single error in the cache + replaceError: function(error) { + if (!(error instanceof impOsmError) || !error.id) return; + + _erCache.data[error.id] = error; + updateRtree(encodeErrorRtree(error), true); // true = replace + return error; + }, + + // remove a single error from the cache + removeError: function(error) { + if (!(error instanceof impOsmError) || !error.id) return; + + delete _erCache.data[error.id]; + updateRtree(encodeErrorRtree(error), false); // false = remove } }; diff --git a/modules/ui/improveOSM_details.js b/modules/ui/improveOSM_details.js new file mode 100644 index 000000000..227cef8b9 --- /dev/null +++ b/modules/ui/improveOSM_details.js @@ -0,0 +1,127 @@ +import { + event as d3_event, + select as d3_select +} from 'd3-selection'; + +import { dataEn } from '../../data'; +import { modeSelect } from '../modes'; +import { t } from '../util/locale'; +import { utilDisplayName, utilEntityOrMemberSelector, utilEntityRoot } from '../util'; + + +export function uiImproveOsmDetails(context) { + var _error; + + + function errorDetail(d) { + var unknown = t('inspector.unknown'); + + if (!d) return unknown; + var errorType = d.parent_error_type; + var et = dataEn.QA.improveOSM.error_types[errorType]; + + var detail; + if (et && et.description) { + detail = t('QA.improveOSM.error_types.' + errorType + '.description'); + } else { + detail = unknown; + } + + return detail; + } + + + function improveOsmDetails(selection) { + var details = selection.selectAll('.kr_error-details') + .data( + (_error ? [_error] : []), + function(d) { return d.id + '-' + (d.status || 0); } + ); + + details.exit() + .remove(); + + var detailsEnter = details.enter() + .append('div') + .attr('class', 'kr_error-details kr_error-details-container'); + + + // description + var descriptionEnter = detailsEnter + .append('div') + .attr('class', 'kr_error-details-description'); + + descriptionEnter + .append('h4') + .text(function() { return t('QA.keepRight.detail_description'); }); + + descriptionEnter + .append('div') + .attr('class', 'kr_error-details-description-text') + .html(errorDetail); + + // If there are entity links in the error message.. + descriptionEnter.selectAll('.kr_error_entity_link, .kr_error_object_link') + .each(function() { + var link = d3_select(this); + var isObjectLink = link.classed('kr_error_object_link'); + var entityID = isObjectLink ? + (utilEntityRoot(_error.object_type) + _error.object_id) + : this.textContent; + var entity = context.hasEntity(entityID); + + // Add click handler + link + .on('mouseover', function() { + context.surface().selectAll(utilEntityOrMemberSelector([entityID], context.graph())) + .classed('hover', true); + }) + .on('mouseout', function() { + context.surface().selectAll('.hover') + .classed('hover', false); + }) + .on('click', function() { + d3_event.preventDefault(); + var osmlayer = context.layers().layer('osm'); + if (!osmlayer.enabled()) { + osmlayer.enabled(true); + } + + context.map().centerZoom(_error.loc, 20); + + if (entity) { + context.enter(modeSelect(context, [entityID])); + } else { + context.loadEntity(entityID, function() { + context.enter(modeSelect(context, [entityID])); + }); + } + }); + + // Replace with friendly name if possible + // (The entity may not yet be loaded into the graph) + if (entity) { + var name = utilDisplayName(entity); // try to use common name + + if (!name && !isObjectLink) { + var preset = context.presets().match(entity, context.graph()); + name = preset && !preset.isFallback() && preset.name(); // fallback to preset name + } + + if (name) { + this.innerText = name; + } + } + }); + } + + + improveOsmDetails.error = function(val) { + if (!arguments.length) return _error; + _error = val; + return improveOsmDetails; + }; + + + return improveOsmDetails; +} diff --git a/modules/ui/improveOSM_editor.js b/modules/ui/improveOSM_editor.js new file mode 100644 index 000000000..ed1d5065c --- /dev/null +++ b/modules/ui/improveOSM_editor.js @@ -0,0 +1,90 @@ +import { dispatch as d3_dispatch } from 'd3-dispatch'; +import { select as d3_select } from 'd3-selection'; + +import { t } from '../util/locale'; +import { services } from '../services'; +import { modeBrowse } from '../modes'; +import { svgIcon } from '../svg'; + +import { + uiImproveOsmDetails, + uiImproveOsmHeader, + uiQuickLinks, + uiTooltipHtml +} from './index'; + +import { utilNoAuto, utilRebind } from '../util'; + + +export function uiImproveOsmEditor(context) { + var dispatch = d3_dispatch('change'); + var errorDetails = uiImproveOsmDetails(context); + var errorHeader = uiImproveOsmHeader(context); + var quickLinks = uiQuickLinks(); + + var _error; + + + function improveOsmEditor(selection) { + // quick links + var choices = [{ + id: 'zoom_to', + label: 'inspector.zoom_to.title', + tooltip: function() { + return uiTooltipHtml(t('inspector.zoom_to.tooltip_issue'), t('inspector.zoom_to.key')); + }, + click: function zoomTo() { + context.mode().zoomToSelected(); + } + }]; + + + var header = selection.selectAll('.header') + .data([0]); + + var headerEnter = header.enter() + .append('div') + .attr('class', 'header fillL'); + + headerEnter + .append('button') + .attr('class', 'fr keepRight-editor-close') + .on('click', function() { + context.enter(modeBrowse(context)); + }) + .call(svgIcon('#iD-icon-close')); + + headerEnter + .append('h3') + .text(t('QA.improveOSM.title')); + + + var body = selection.selectAll('.body') + .data([0]); + + body = body.enter() + .append('div') + .attr('class', 'body') + .merge(body); + + var editor = body.selectAll('.keepRight-editor') + .data([0]); + + editor.enter() + .append('div') + .attr('class', 'modal-section keepRight-editor') + .merge(editor) + .call(errorHeader.error(_error)) + .call(quickLinks.choices(choices)) + .call(errorDetails.error(_error)); + } + + improveOsmEditor.error = function(val) { + if (!arguments.length) return _error; + _error = val; + return improveOsmEditor; + }; + + + return utilRebind(improveOsmEditor, dispatch, 'on'); +} diff --git a/modules/ui/improveOSM_header.js b/modules/ui/improveOSM_header.js new file mode 100644 index 000000000..3a2f19eee --- /dev/null +++ b/modules/ui/improveOSM_header.js @@ -0,0 +1,66 @@ +import { dataEn } from '../../data'; +import { svgIcon } from '../svg'; +import { t } from '../util/locale'; + + +export function uiImproveOsmHeader() { + var _error; + + + function errorTitle(d) { + var unknown = t('inspector.unknown'); + + if (!d) return unknown; + var errorType = d.parent_error_type; + var et = dataEn.QA.improveOSM.error_types[errorType]; + + if (et && et.title) { + return t('QA.improveOSM.error_types.' + errorType + '.title'); + } else { + return unknown; + } + } + + + function improveOsmHeader(selection) { + var header = selection.selectAll('.kr_error-header') + .data( + (_error ? [_error] : []), + function(d) { return d.id + '-' + (d.status || 0); } + ); + + header.exit() + .remove(); + + var headerEnter = header.enter() + .append('div') + .attr('class', 'kr_error-header'); + + var iconEnter = headerEnter + .append('div') + .attr('class', 'kr_error-header-icon') + .classed('new', function(d) { return d.id < 0; }); + + iconEnter + .append('div') + .attr('class', function(d) { + return 'preset-icon-28 iOSM_error iOSM_error-' + d.id + ' iOSM_error_type_' + d.parent_error_type; + }) + .call(svgIcon('#iD-icon-bolt', 'iOSM_error-fill')); + + headerEnter + .append('div') + .attr('class', 'kr_error-header-label') + .text(errorTitle); + } + + + improveOsmHeader.error = function(val) { + if (!arguments.length) return _error; + _error = val; + return improveOsmHeader; + }; + + + return improveOsmHeader; +} diff --git a/modules/ui/index.js b/modules/ui/index.js index de4ff07c7..c1361064e 100644 --- a/modules/ui/index.js +++ b/modules/ui/index.js @@ -28,6 +28,9 @@ export { uiFormFields } from './form_fields'; export { uiFullScreen } from './full_screen'; export { uiGeolocate } from './geolocate'; export { uiHelp } from './help'; +export { uiImproveOsmDetails } from './improveOSM_details'; +export { uiImproveOsmEditor } from './improveOSM_editor'; +export { uiImproveOsmHeader } from './improveOSM_header'; export { uiInfo } from './info'; export { uiInspector } from './inspector'; export { uiKeepRightDetails } from './keepRight_details'; From 23a4d71410b78c697301921450c2db10c632ec48 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sun, 20 Jan 2019 13:31:13 +0000 Subject: [PATCH 03/38] Handle turn restriction errors --- modules/services/improveOSM.js | 46 ++++++++++++++------------------ modules/svg/improveOSM.js | 2 +- modules/ui/improveOSM_details.js | 2 +- modules/ui/improveOSM_header.js | 4 +-- 4 files changed, 24 insertions(+), 30 deletions(-) diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index 849749632..faa18044d 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -124,7 +124,7 @@ export default { _forEach(_impOsmUrls, function(v, k) { var url = v + '/search?' + utilQsString(params); - if (k == 'mr' || k == 'tr') return + if (k == 'mr') return requests[k] = d3_json(url, function(err, data) { @@ -148,11 +148,9 @@ export default { var d = new impOsmError({ loc: [loc.lon, loc.lat], - comment: null, - description: '', - error_type: feature.type, - parent_error_type: k, - title: 'Missing One-way' + comments: null, + error_type: k, + road_type: feature.type }); _erCache.data[d.id] = d; @@ -170,10 +168,8 @@ export default { // var d = new impOsmError({ // loc: [feature.x, feature.y], // comment: null, - // description: desc || '', - // error_type: feature.type, - // parent_error_type: k, - // title: 'Missing Roads' + // error_type: k, + // geometry_type: feature.type // }); // _erCache.data[d.id] = d; @@ -181,24 +177,22 @@ export default { // }) // } + // Entities at high zoom == turn restrictions + if (data.entities) { + data.entities.forEach(function(feature) { + var loc = feature.point; - // if (data.entities) { - // data.entities.forEach(function(feature) { - // var loc = feature.point; + var d = new impOsmError({ + loc: [loc.lon, loc.lat], + comments: null, + error_type: k, + turn_type: feature.turnType + }); - // var d = new impOsmError({ - // loc: [loc.lat, loc.lon], - // comment: null, - // description: desc || '', - // error_type: feature.turnType, - // parent_error_type: k, - // title: 'Missing Turn Restriction' - // }); - - // _erCache.data[d.id] = d; - // _erCache.rtree.insert(encodeErrorRtree(d)); - // }) - // } + _erCache.data[d.id] = d; + _erCache.rtree.insert(encodeErrorRtree(d)); + }) + } } ); }); diff --git a/modules/svg/improveOSM.js b/modules/svg/improveOSM.js index 738b63533..6255bcba3 100644 --- a/modules/svg/improveOSM.js +++ b/modules/svg/improveOSM.js @@ -116,7 +116,7 @@ export function svgImproveOSM(projection, context, dispatch) { var markersEnter = markers.enter() .append('g') .attr('class', function(d) { - return 'iOSM_error iOSM_error-' + d.id + ' iOSM_error_type_' + d.parent_error_type; } + return 'iOSM_error iOSM_error-' + d.id + ' iOSM_error_type_' + d.error_type; } ); markersEnter diff --git a/modules/ui/improveOSM_details.js b/modules/ui/improveOSM_details.js index 227cef8b9..cab443ac2 100644 --- a/modules/ui/improveOSM_details.js +++ b/modules/ui/improveOSM_details.js @@ -17,7 +17,7 @@ export function uiImproveOsmDetails(context) { var unknown = t('inspector.unknown'); if (!d) return unknown; - var errorType = d.parent_error_type; + var errorType = d.error_type; var et = dataEn.QA.improveOSM.error_types[errorType]; var detail; diff --git a/modules/ui/improveOSM_header.js b/modules/ui/improveOSM_header.js index 3a2f19eee..070c230be 100644 --- a/modules/ui/improveOSM_header.js +++ b/modules/ui/improveOSM_header.js @@ -11,7 +11,7 @@ export function uiImproveOsmHeader() { var unknown = t('inspector.unknown'); if (!d) return unknown; - var errorType = d.parent_error_type; + var errorType = d.error_type; var et = dataEn.QA.improveOSM.error_types[errorType]; if (et && et.title) { @@ -44,7 +44,7 @@ export function uiImproveOsmHeader() { iconEnter .append('div') .attr('class', function(d) { - return 'preset-icon-28 iOSM_error iOSM_error-' + d.id + ' iOSM_error_type_' + d.parent_error_type; + return 'preset-icon-28 iOSM_error iOSM_error-' + d.id + ' iOSM_error_type_' + d.error_type; }) .call(svgIcon('#iD-icon-bolt', 'iOSM_error-fill')); From ca290b510100dcc24286d5349c53b51e2fee27b4 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sun, 20 Jan 2019 15:15:45 +0000 Subject: [PATCH 04/38] Add details to error messages --- data/core.yaml | 12 +++++++---- dist/locales/en.json | 13 ++++++++---- modules/services/improveOSM.js | 35 ++++++++++++++++++++++++++++++++ modules/ui/improveOSM_details.js | 2 +- 4 files changed, 53 insertions(+), 9 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index 6a4195511..a1bd674e7 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -663,16 +663,20 @@ en: QA: improveOSM: title: ImproveOSM Error + error_parts: + path: path + parking_area: parking area + road: road error_types: ow: title: Missing One-way - description: Aggregate data suggests {var1} may be missing a "oneway" tag. Please confirm via imagery or survey before mapping. + description: 'Along this section of {highway}, {percentage}% of {num_trips} recorded trips travel in the same direction. There may be missing a "oneway" tag.' mr: - title: Missing Road - description: Aggregate data suggests there may be an unmapped road here. Please confirm via imagery or survey before mapping. + title: Missing Geometry + description: Aggregate data suggests there may be an unmapped {var1} here. tr: title: Missing Turn Restriction - description: Aggregate data suggests this junction may be missing a turn restriction. Please confirm via imagery or survey before mapping. + description: '{num_passed} of {num_trips} recorded trips make a turn from {from_way} to {to_way}. There may be a missing "{turn_restriction}" restriction.' keepRight: title: KeepRight Error detail_title: Error diff --git a/dist/locales/en.json b/dist/locales/en.json index bea513f1d..8bc3da440 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -805,18 +805,23 @@ "QA": { "improveOSM": { "title": "ImproveOSM Error", + "error_parts": { + "path": "path", + "parking_area": "parking area", + "road": "road" + }, "error_types": { "ow": { "title": "Missing One-way", - "description": "Aggregate data suggests {var1} may be missing a \"oneway\" tag. Please confirm via imagery or survey before mapping." + "description": "Along this section of {highway}, {percentage}% of {num_trips} recorded trips travel in the same direction. There may be missing a \"oneway\" tag." }, "mr": { - "title": "Missing Road", - "description": "Aggregate data suggests there may be an unmapped road here. Please confirm via imagery or survey before mapping." + "title": "Missing Geometry", + "description": "Aggregate data suggests there may be an unmapped {var1} here." }, "tr": { "title": "Missing Turn Restriction", - "description": "Aggregate data suggests this junction may be missing a turn restriction. Please confirm via imagery or survey before mapping." + "description": "{num_passed} of {num_trips} recorded trips make a turn from {from_way} to {to_way}. There may be a missing \"{turn_restriction}\" restriction." } } }, diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index faa18044d..8f239ef37 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -71,6 +71,14 @@ function updateRtree(item, replace) { } } +function linkErrorObject(d) { + return '' + d + ''; +} + +function linkEntity(d) { + return '' + d + ''; +} + export default { init: function() { if (!_erCache) { @@ -144,15 +152,25 @@ export default { // Road segments at high zoom == oneways if (data.roadSegments) { data.roadSegments.forEach(function(feature) { + // todo: make this take midpoint? var loc = feature.points[0]; var d = new impOsmError({ loc: [loc.lon, loc.lat], comments: null, error_type: k, + object_id: feature.wayId, + object_type: 'way', road_type: feature.type }); + // Variables used in the description + d.replacements = { + percentage: feature.percentOfTrips, + num_trips: feature.numberOfTrips, + highway: linkErrorObject(t('QA.keepRight.error_parts.highway')) + }; + _erCache.data[d.id] = d; _erCache.rtree.insert(encodeErrorRtree(d)); }) @@ -182,13 +200,30 @@ export default { data.entities.forEach(function(feature) { var loc = feature.point; + // Elements are presented in a strange way + var ids = feature.id.split(','); + var from_way = ids[0]; + var via_node = ids[3]; + var to_way = ids[2].split(':')[1]; + var d = new impOsmError({ loc: [loc.lon, loc.lat], comments: null, error_type: k, + object_id: via_node, + object_type: 'node', turn_type: feature.turnType }); + // Variables used in the description + d.replacements = { + num_passed: feature.numberOfPasses, + num_trips: feature.segments[0].numberOfTrips, + turn_restriction: feature.turnType.toLowerCase(), + from_way: linkEntity('w' + from_way), + to_way: linkEntity('w' + to_way) + }; + _erCache.data[d.id] = d; _erCache.rtree.insert(encodeErrorRtree(d)); }) diff --git a/modules/ui/improveOSM_details.js b/modules/ui/improveOSM_details.js index cab443ac2..be513ccb0 100644 --- a/modules/ui/improveOSM_details.js +++ b/modules/ui/improveOSM_details.js @@ -22,7 +22,7 @@ export function uiImproveOsmDetails(context) { var detail; if (et && et.description) { - detail = t('QA.improveOSM.error_types.' + errorType + '.description'); + detail = t('QA.improveOSM.error_types.' + errorType + '.description', d.replacements); } else { detail = unknown; } From bea6f4dcfd7e271bda996b9ed911a2329fb92dbc Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sun, 20 Jan 2019 16:56:23 +0000 Subject: [PATCH 05/38] Use point averages for locations --- data/core.yaml | 11 +++--- dist/locales/en.json | 11 +++--- modules/services/improveOSM.js | 62 ++++++++++++++++++---------------- 3 files changed, 45 insertions(+), 39 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index a1bd674e7..eee91225b 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -663,17 +663,18 @@ en: QA: improveOSM: title: ImproveOSM Error - error_parts: - path: path - parking_area: parking area - road: road + geometry_types: + path: paths + parking: parking areas + road: roads + both: roads and parking error_types: ow: title: Missing One-way description: 'Along this section of {highway}, {percentage}% of {num_trips} recorded trips travel in the same direction. There may be missing a "oneway" tag.' mr: title: Missing Geometry - description: Aggregate data suggests there may be an unmapped {var1} here. + description: '{num_trips} recorded trips in this area suggest there may be unmapped {geometry_type} here.' tr: title: Missing Turn Restriction description: '{num_passed} of {num_trips} recorded trips make a turn from {from_way} to {to_way}. There may be a missing "{turn_restriction}" restriction.' diff --git a/dist/locales/en.json b/dist/locales/en.json index 8bc3da440..b904101d1 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -805,10 +805,11 @@ "QA": { "improveOSM": { "title": "ImproveOSM Error", - "error_parts": { - "path": "path", - "parking_area": "parking area", - "road": "road" + "geometry_types": { + "path": "paths", + "parking": "parking areas", + "road": "roads", + "both": "roads and parking" }, "error_types": { "ow": { @@ -817,7 +818,7 @@ }, "mr": { "title": "Missing Geometry", - "description": "Aggregate data suggests there may be an unmapped {var1} here." + "description": "{num_trips} recorded trips in this area suggest there may be unmapped {geometry_type} here." }, "tr": { "title": "Missing Turn Restriction", diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index 8f239ef37..34c997f2e 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -26,14 +26,6 @@ var _impOsmUrls = { tr: 'http://turnrestrictionservice.skobbler.net/turnRestrictionService' }; -var _missingTypes = { - PARKING: 'Unmapped parking', - ROAD: 'Unmapped road(s)', - BOTH: 'Unmapped road(s) and parking', - PATH: 'Unmapped path(s)', - WATER: 'Unmapped water feature' // ? -}; - function abortRequest(i) { _forEach(i, function(v) { if (v) { @@ -79,6 +71,21 @@ function linkEntity(d) { return '' + d + ''; } +function pointAverage(points) { + var x = 0; + var y = 0; + + _forEach(points, function(v) { + x += v.lon; + y += v.lat; + }); + + x /= points.length; + y /= points.length; + + return [x, y] +} + export default { init: function() { if (!_erCache) { @@ -132,8 +139,6 @@ export default { _forEach(_impOsmUrls, function(v, k) { var url = v + '/search?' + utilQsString(params); - if (k == 'mr') return - requests[k] = d3_json(url, function(err, data) { delete _erCache.inflight[tile.id]; @@ -152,11 +157,8 @@ export default { // Road segments at high zoom == oneways if (data.roadSegments) { data.roadSegments.forEach(function(feature) { - // todo: make this take midpoint? - var loc = feature.points[0]; - var d = new impOsmError({ - loc: [loc.lon, loc.lat], + loc: pointAverage(feature.points), // TODO: This isn't great for curved roads, would be better to find actual midpoint of segment comments: null, error_type: k, object_id: feature.wayId, @@ -177,23 +179,24 @@ export default { } // Tiles at high zoom == missing roads - // if (data.tiles) { - // data.tiles.forEach(function(feature) { - // // Get description based on type - // var desc = _missingTypes[feature.type]; + if (data.tiles) { + data.tiles.forEach(function(feature) { + var d = new impOsmError({ + loc: pointAverage(feature.points), + comments: null, + error_type: k, + geometry_type: feature.type + }); + d.replacements = { + num_trips: feature.numberOfTrips, + geometry_type: t('QA.improveOSM.geometry_types.' + feature.type.toLowerCase()) + }; - // var d = new impOsmError({ - // loc: [feature.x, feature.y], - // comment: null, - // error_type: k, - // geometry_type: feature.type - // }); - - // _erCache.data[d.id] = d; - // _erCache.rtree.insert(encodeErrorRtree(d)); - // }) - // } + _erCache.data[d.id] = d; + _erCache.rtree.insert(encodeErrorRtree(d)); + }) + } // Entities at high zoom == turn restrictions if (data.entities) { @@ -216,6 +219,7 @@ export default { }); // Variables used in the description + //TODO: Add direction of travel d.replacements = { num_passed: feature.numberOfPasses, num_trips: feature.segments[0].numberOfTrips, From 9600d67fc63bb90b8e88d2c69653f3553cc03f44 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sun, 20 Jan 2019 17:20:50 +0000 Subject: [PATCH 06/38] Differentiate missing geometry errors by colour --- css/65_data.css | 11 ++++++++++- modules/services/improveOSM.js | 19 ++++++++++++------- modules/svg/improveOSM.js | 2 +- modules/ui/improveOSM_header.js | 2 +- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/css/65_data.css b/css/65_data.css index 5aebce603..2ecad6116 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -123,9 +123,18 @@ color: #EE7600; } -.iOSM_error_type_mr { /* missing road */ +.iOSM_error_type_mr_road { /* missing road */ color: #B0171F; } +.iOSM_error_type_mr_path { /* missing path */ + color: #A0522D; +} +.iOSM_error_type_mr_parking { /* missing parking */ + color: #EEEE00; +} +.iOSM_error_type_mr_both { /* missing road+parking */ + color: #FFA500; +} .iOSM_error_type_tr { /* missing turn restriction */ color: #1E90FF; diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index 34c997f2e..e36734ff3 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -160,12 +160,15 @@ export default { var d = new impOsmError({ loc: pointAverage(feature.points), // TODO: This isn't great for curved roads, would be better to find actual midpoint of segment comments: null, + error_subtype: '', error_type: k, object_id: feature.wayId, - object_type: 'way', - road_type: feature.type + object_type: 'way' }); + //TODO include road type in description? + // feature.type + // Variables used in the description d.replacements = { percentage: feature.percentOfTrips, @@ -181,16 +184,18 @@ export default { // Tiles at high zoom == missing roads if (data.tiles) { data.tiles.forEach(function(feature) { + var geoType = feature.type.toLowerCase(); + var d = new impOsmError({ loc: pointAverage(feature.points), comments: null, - error_type: k, - geometry_type: feature.type + error_subtype: '_' + geoType, + error_type: k }); d.replacements = { num_trips: feature.numberOfTrips, - geometry_type: t('QA.improveOSM.geometry_types.' + feature.type.toLowerCase()) + geometry_type: t('QA.improveOSM.geometry_types.' + geoType) }; _erCache.data[d.id] = d; @@ -212,10 +217,10 @@ export default { var d = new impOsmError({ loc: [loc.lon, loc.lat], comments: null, + error_subtype: '', error_type: k, object_id: via_node, - object_type: 'node', - turn_type: feature.turnType + object_type: 'node' }); // Variables used in the description diff --git a/modules/svg/improveOSM.js b/modules/svg/improveOSM.js index 6255bcba3..d701a8706 100644 --- a/modules/svg/improveOSM.js +++ b/modules/svg/improveOSM.js @@ -116,7 +116,7 @@ export function svgImproveOSM(projection, context, dispatch) { var markersEnter = markers.enter() .append('g') .attr('class', function(d) { - return 'iOSM_error iOSM_error-' + d.id + ' iOSM_error_type_' + d.error_type; } + return 'iOSM_error iOSM_error-' + d.id + ' iOSM_error_type_' + d.error_type + d.error_subtype; } ); markersEnter diff --git a/modules/ui/improveOSM_header.js b/modules/ui/improveOSM_header.js index 070c230be..102f5a57a 100644 --- a/modules/ui/improveOSM_header.js +++ b/modules/ui/improveOSM_header.js @@ -44,7 +44,7 @@ export function uiImproveOsmHeader() { iconEnter .append('div') .attr('class', function(d) { - return 'preset-icon-28 iOSM_error iOSM_error-' + d.id + ' iOSM_error_type_' + d.error_type; + return 'preset-icon-28 iOSM_error iOSM_error-' + d.id + ' iOSM_error_type_' + d.error_type + d.error_subtype; }) .call(svgIcon('#iD-icon-bolt', 'iOSM_error-fill')); From 39880e559b24eeff7d55d79a05a1d8f6888084c6 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sun, 20 Jan 2019 22:07:07 +0000 Subject: [PATCH 07/38] Add direction of travel to turn restriction errors --- data/core.yaml | 11 +++++++++- dist/locales/en.json | 12 ++++++++++- modules/services/improveOSM.js | 38 +++++++++++++++++++++++++++++++--- 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index eee91225b..a19cc3300 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -668,6 +668,15 @@ en: parking: parking areas road: roads both: roads and parking + directions: + east: east + north: north + northeast: northeast + northwest: northwest + south: south + southeast: southeast + southwest: southwest + west: west error_types: ow: title: Missing One-way @@ -677,7 +686,7 @@ en: description: '{num_trips} recorded trips in this area suggest there may be unmapped {geometry_type} here.' tr: title: Missing Turn Restriction - description: '{num_passed} of {num_trips} recorded trips make a turn from {from_way} to {to_way}. There may be a missing "{turn_restriction}" restriction.' + description: '{num_passed} of {num_trips} recorded trips (travelling {travel_direction}) make a turn from {from_way} to {to_way}. There may be a missing "{turn_restriction}" restriction.' keepRight: title: KeepRight Error detail_title: Error diff --git a/dist/locales/en.json b/dist/locales/en.json index b904101d1..7f674fd0e 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -811,6 +811,16 @@ "road": "roads", "both": "roads and parking" }, + "directions": { + "east": "east", + "north": "north", + "northeast": "northeast", + "northwest": "northwest", + "south": "south", + "southeast": "southeast", + "southwest": "southwest", + "west": "west" + }, "error_types": { "ow": { "title": "Missing One-way", @@ -822,7 +832,7 @@ }, "tr": { "title": "Missing Turn Restriction", - "description": "{num_passed} of {num_trips} recorded trips make a turn from {from_way} to {to_way}. There may be a missing \"{turn_restriction}\" restriction." + "description": "{num_passed} of {num_trips} recorded trips (travelling {travel_direction}) make a turn from {from_way} to {to_way}. There may be a missing \"{turn_restriction}\" restriction." } } }, diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index e36734ff3..a203dace1 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -83,7 +83,34 @@ function pointAverage(points) { x /= points.length; y /= points.length; - return [x, y] + return [x, y]; +} + +function relativeBearing(p1, p2) { + var angle = Math.atan2(p2.lon - p1.lon, p2.lat - p1.lat); + if (angle < 0) { + angle += 2 * Math.PI; + } + + // Return degrees + return angle * 180 / Math.PI; +} + +// Assuming range [0,360) +function cardinalDirection(bearing) { + var dir = 45 * Math.round(bearing / 45); + var compass = { + 0: 'north', + 45: 'northeast', + 90: 'east', + 135: 'southeast', + 180: 'south', + 225: 'southwest', + 270: 'west', + 315: 'northwest' + }; + + return t('QA.improveOSM.directions.' + compass[dir]); } export default { @@ -214,6 +241,11 @@ export default { var via_node = ids[3]; var to_way = ids[2].split(':')[1]; + var p1 = feature.segments[0].points[0]; + var p2 = feature.segments[0].points[1]; + + var dir_of_travel = cardinalDirection(relativeBearing(p1, p2)); + var d = new impOsmError({ loc: [loc.lon, loc.lat], comments: null, @@ -224,13 +256,13 @@ export default { }); // Variables used in the description - //TODO: Add direction of travel d.replacements = { num_passed: feature.numberOfPasses, num_trips: feature.segments[0].numberOfTrips, turn_restriction: feature.turnType.toLowerCase(), from_way: linkEntity('w' + from_way), - to_way: linkEntity('w' + to_way) + to_way: linkEntity('w' + to_way), + travel_direction: dir_of_travel }; _erCache.data[d.id] = d; From 214cf41019a57ad22a52e2e3fbc87417ea15cefb Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sun, 20 Jan 2019 23:27:49 +0000 Subject: [PATCH 08/38] Use more generic classes for errors --- css/20_map.css | 19 +++----- css/55_cursors.css | 6 +-- css/65_data.css | 82 ++++++++++++++++----------------- data/core.yaml | 2 +- dist/locales/en.json | 2 +- modules/behavior/hover.js | 11 ++--- modules/behavior/select.js | 4 +- modules/modes/select_error.js | 8 ++-- modules/osm/improveOSM.js | 1 + modules/osm/keepRight.js | 3 +- modules/services/improveOSM.js | 2 +- modules/svg/improveOSM.js | 23 +++++---- modules/svg/keepRight.js | 23 +++++---- modules/ui/improveOSM_header.js | 4 +- modules/ui/keepRight_header.js | 4 +- 15 files changed, 98 insertions(+), 96 deletions(-) diff --git a/css/20_map.css b/css/20_map.css index 72d604a7e..559b3ae72 100644 --- a/css/20_map.css +++ b/css/20_map.css @@ -45,8 +45,7 @@ /* `.target` objects are interactive */ /* They can be picked up, clicked, hovered, or things can connect to them */ -.iOSM_error.target, -.kr_error.target, +.qa_error.target, .note.target, .node.target, .turn .target { @@ -81,12 +80,10 @@ pointer-events: none !important; } -/* NOTE: when more QA layers are added, replace kr_error with generic QA layer selector */ /* points, notes & QA */ /* points, notes, markers */ -g.iOSM_error .stroke, -g.kr_error .stroke, +g.qa_error .stroke, g.note .stroke { stroke: #222; stroke-width: 1; @@ -94,8 +91,7 @@ g.note .stroke { opacity: 0.6; } -g.iOSM_error.active .stroke, -g.kr_error.active .stroke, +g.qa_error.active .stroke, g.note.active .stroke { stroke: #222; stroke-width: 1; @@ -109,8 +105,7 @@ g.point .stroke { fill: #fff; } -g.iOSM_error .shadow, -g.kr_error .shadow, +g.qa_error .shadow, g.point .shadow, g.note .shadow { fill: none; @@ -119,16 +114,14 @@ g.note .shadow { stroke-opacity: 0; } -g.iOSM_error.hover:not(.selected) .shadow, -g.kr_error.hover:not(.selected) .shadow, +g.qa_error.hover:not(.selected) .shadow, g.note.hover:not(.selected) .shadow, g.point.related:not(.selected) .shadow, g.point.hover:not(.selected) .shadow { stroke-opacity: 0.5; } -g.iOSM_error.selected .shadow, -g.kr_error.selected .shadow, +g.qa_error.selected .shadow, g.note.selected .shadow, g.point.selected .shadow { stroke-opacity: 0.7; diff --git a/css/55_cursors.css b/css/55_cursors.css index efc293266..1a4c8cf7f 100644 --- a/css/55_cursors.css +++ b/css/55_cursors.css @@ -97,11 +97,9 @@ } .mode-browse .note, -.mode-browse .kr_error, -.mode-browse .iOSM_error, +.mode-browse .qa_error, .mode-select .note, -.mode-select .kr_error, -.mode-select .iOSM_error, +.mode-select .qa_error, .turn rect, .turn circle { cursor: pointer; diff --git a/css/65_data.css b/css/65_data.css index 2ecad6116..8585b70b0 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -1,9 +1,9 @@ /* OSM Notes and KeepRight Layers */ -.kr_error-header-icon .kr_error-fill, -.layer-keepRight .kr_error .kr_error-fill, -.layer-improveOSM .iOSM_error .iOSM_error-fill { +.kr_error-header-icon .qa_error-fill, +.layer-keepRight .qa_error .qa_error-fill, +.layer-improveOSM .qa_error .qa_error-fill { stroke: #333; stroke-width: 1.3px; /* NOTE: likely a better way to scale the icon stroke */ } @@ -45,98 +45,98 @@ /* Keep Right Errors ------------------------------------------------------- */ -.kr_error_type_20, /* multiple nodes on same spot */ -.kr_error_type_40, /* impossible oneways */ -.kr_error_type_210, /* self intersecting ways */ -.kr_error_type_270, /* unusual motorway connection */ -.kr_error_type_310, /* roundabout issues */ -.kr_error_type_320, /* improper _link */ -.kr_error_type_350 { /* improper bridge tag */ +.kr.error_type-20, /* multiple nodes on same spot */ +.kr.error_type-40, /* impossible oneways */ +.kr.error_type-210, /* self intersecting ways */ +.kr.error_type-270, /* unusual motorway connection */ +.kr.error_type-310, /* roundabout issues */ +.kr.error_type-320, /* improper _link */ +.kr.error_type-350 { /* improper bridge tag */ color: #ff9; } -.kr_error_type_50 { /* almost junctions */ +.kr.error_type-50 { /* almost junctions */ color: #88f; } -.kr_error_type_60, /* deprecated tags */ -.kr_error_type_70, /* tagging issues */ -.kr_error_type_90, /* motorway without ref */ -.kr_error_type_100, /* place of worship without religion */ -.kr_error_type_110, /* poi without name */ -.kr_error_type_150, /* railway crossing without tag */ -.kr_error_type_220, /* misspelled tag */ -.kr_error_type_380 { /* non-physical sport tag */ +.kr.error_type-60, /* deprecated tags */ +.kr.error_type-70, /* tagging issues */ +.kr.error_type-90, /* motorway without ref */ +.kr.error_type-100, /* place of worship without religion */ +.kr.error_type-110, /* poi without name */ +.kr.error_type-150, /* railway crossing without tag */ +.kr.error_type-220, /* misspelled tag */ +.kr.error_type-380 { /* non-physical sport tag */ color: #5d0; } -.kr_error_type_130 { /* disconnected ways */ +.kr.error_type-130 { /* disconnected ways */ color: #fa3; } -.kr_error_type_170 { /* FIXME tag */ +.kr.error_type-170 { /* FIXME tag */ color: #ff0; } -.kr_error_type_190 { /* intersection without junction */ +.kr.error_type-190 { /* intersection without junction */ color: #f33; } -.kr_error_type_200 { /* overlapping ways */ +.kr.error_type-200 { /* overlapping ways */ color: #fdbf6f; } -.kr_error_type_160, /* railway layer conflict */ -.kr_error_type_230 { /* layer conflict */ +.kr.error_type-160, /* railway layer conflict */ +.kr.error_type-230 { /* layer conflict */ color: #b60; } -.kr_error_type_280 { /* boundary issues */ +.kr.error_type-280 { /* boundary issues */ color: #5f47a0; } -.kr_error_type_180, /* relation without type */ -.kr_error_type_290 { /* turn restriction issues */ +.kr.error_type-180, /* relation without type */ +.kr.error_type-290 { /* turn restriction issues */ color: #ace; } -.kr_error_type_300, /* missing maxspeed */ -.kr_error_type_390 { /* missing tracktype */ +.kr.error_type-300, /* missing maxspeed */ +.kr.error_type-390 { /* missing tracktype */ color: #090; } -.kr_error_type_360, /* language unknown */ -.kr_error_type_370, /* doubled places */ -.kr_error_type_410 { /* website issues */ +.kr.error_type-360, /* language unknown */ +.kr.error_type-370, /* doubled places */ +.kr.error_type-410 { /* website issues */ color: #f9b; } -.kr_error_type_120, /* way without nodes */ -.kr_error_type_400 { /* geometry / turn angles */ +.kr.error_type-120, /* way without nodes */ +.kr.error_type-400 { /* geometry / turn angles */ color: #c35; } /* ImproveOSM Errors ------------------------------------------------------- */ -.iOSM_error_type_ow { /* missing one way */ +.iOSM.error_type-ow- { /* missing one way */ color: #EE7600; } -.iOSM_error_type_mr_road { /* missing road */ +.iOSM.error_type-mr-road { /* missing road */ color: #B0171F; } -.iOSM_error_type_mr_path { /* missing path */ +.iOSM.error_type-mr-path { /* missing path */ color: #A0522D; } -.iOSM_error_type_mr_parking { /* missing parking */ +.iOSM.error_type-mr-parking { /* missing parking */ color: #EEEE00; } -.iOSM_error_type_mr_both { /* missing road+parking */ +.iOSM.error_type-mr-both { /* missing road+parking */ color: #FFA500; } -.iOSM_error_type_tr { /* missing turn restriction */ +.iOSM.error_type-tr- { /* missing turn restriction */ color: #1E90FF; } diff --git a/data/core.yaml b/data/core.yaml index a19cc3300..b64e5cec9 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -665,7 +665,7 @@ en: title: ImproveOSM Error geometry_types: path: paths - parking: parking areas + parking: parking road: roads both: roads and parking directions: diff --git a/dist/locales/en.json b/dist/locales/en.json index 7f674fd0e..73bdc3eb0 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -807,7 +807,7 @@ "title": "ImproveOSM Error", "geometry_types": { "path": "paths", - "parking": "parking areas", + "parking": "parking", "road": "roads", "both": "roads and parking" }, diff --git a/modules/behavior/hover.js b/modules/behavior/hover.js index 627fba543..338541253 100644 --- a/modules/behavior/hover.js +++ b/modules/behavior/hover.js @@ -112,13 +112,12 @@ export function behaviorHover(context) { entity = datum; selector = '.data' + datum.__featurehash__; - } else if (datum instanceof impOsmError) { + } else if ( + datum instanceof impOsmError || + datum instanceof krError + ) { entity = datum; - selector = '.iOSM_error-' + datum.id; - - } else if (datum instanceof krError) { - entity = datum; - selector = '.kr_error-' + datum.id; + selector = '.' + datum.source + '.error_id-' + datum.id; } else if (datum instanceof osmNote) { entity = datum; diff --git a/modules/behavior/select.js b/modules/behavior/select.js index 427d6b3b9..9a780bec0 100644 --- a/modules/behavior/select.js +++ b/modules/behavior/select.js @@ -174,11 +174,11 @@ export function behaviorSelect(context) { } else if (datum instanceof impOsmError & !isMultiselect) { // clicked an improveOSM error context .selectedErrorID(datum.id) - .enter(modeSelectError(context, datum.id, 'ImproveOSM')); + .enter(modeSelectError(context, datum.id, datum.source)); } else if (datum instanceof krError & !isMultiselect) { // clicked a krError error context .selectedErrorID(datum.id) - .enter(modeSelectError(context, datum.id, 'KeepRight')); + .enter(modeSelectError(context, datum.id, datum.source)); } else { // clicked nothing.. context.selectedNoteID(null); context.selectedErrorID(null); diff --git a/modules/modes/select_error.js b/modules/modes/select_error.js index 4472a7da5..4f7645644 100644 --- a/modules/modes/select_error.js +++ b/modules/modes/select_error.js @@ -27,7 +27,7 @@ export function modeSelectError(context, selectedErrorID, selectedErrorSource) { var errorService, errorEditor; switch (selectedErrorSource) { - case 'ImproveOSM': + case 'iOSM': errorService = services.improveOSM; errorEditor = uiImproveOsmEditor(context) .on('change', function() { @@ -38,7 +38,7 @@ export function modeSelectError(context, selectedErrorID, selectedErrorSource) { .show(errorEditor.error(error)); }); break; - case 'KeepRight': + case 'kr': errorService = services.keepRight; errorEditor = uiKeepRightEditor(context) .on('change', function() { @@ -107,7 +107,7 @@ export function modeSelectError(context, selectedErrorID, selectedErrorSource) { if (!checkSelectedID()) return; var selection = context.surface() - .selectAll('.kr_error-' + selectedErrorID); + .selectAll('.error_id-' + selectedErrorID + '.' + selectedErrorSource); if (selection.empty()) { // Return to browse mode if selected DOM elements have @@ -139,7 +139,7 @@ export function modeSelectError(context, selectedErrorID, selectedErrorSource) { .call(keybinding.unbind); context.surface() - .selectAll('.kr_error.selected') + .selectAll('.qa_error.selected') .classed('selected hover', false); context.map() diff --git a/modules/osm/improveOSM.js b/modules/osm/improveOSM.js index b337fcfb6..353e1ece5 100644 --- a/modules/osm/improveOSM.js +++ b/modules/osm/improveOSM.js @@ -22,6 +22,7 @@ impOsmError.id.next = -1; _extend(impOsmError.prototype, { type: 'impOsmError', + source: 'iOSM', initialize: function(sources) { for (var i = 0; i < sources.length; ++i) { diff --git a/modules/osm/keepRight.js b/modules/osm/keepRight.js index 1ea2d4b25..838382e65 100644 --- a/modules/osm/keepRight.js +++ b/modules/osm/keepRight.js @@ -21,6 +21,7 @@ krError.id.next = -1; _extend(krError.prototype, { type: 'krError', + source: 'kr', initialize: function(sources) { for (var i = 0; i < sources.length; ++i) { @@ -46,4 +47,4 @@ _extend(krError.prototype, { update: function(attrs) { return krError(this, attrs); // {v: 1 + (this.v || 0)} } -}); \ No newline at end of file +}); diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index a203dace1..5f2b408ea 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -216,7 +216,7 @@ export default { var d = new impOsmError({ loc: pointAverage(feature.points), comments: null, - error_subtype: '_' + geoType, + error_subtype: geoType, error_type: k }); diff --git a/modules/svg/improveOSM.js b/modules/svg/improveOSM.js index d701a8706..b5941a579 100644 --- a/modules/svg/improveOSM.js +++ b/modules/svg/improveOSM.js @@ -54,9 +54,9 @@ export function svgImproveOSM(projection, context, dispatch) { _improveOsmVisible = false; drawLayer .style('display', 'none'); - drawLayer.selectAll('.iOSM_error') + drawLayer.selectAll('.qa_error.iOSM') .remove(); - touchLayer.selectAll('.iOSM_error') + touchLayer.selectAll('.qa_error.iOSM') .remove(); } } @@ -81,7 +81,7 @@ export function svgImproveOSM(projection, context, dispatch) { function layerOff() { throttledRedraw.cancel(); drawLayer.interrupt(); - touchLayer.selectAll('.iOSM_error') + touchLayer.selectAll('.qa_error.iOSM') .remove(); drawLayer @@ -105,7 +105,7 @@ export function svgImproveOSM(projection, context, dispatch) { var getTransform = svgPointTransform(projection); // Draw markers.. - var markers = drawLayer.selectAll('.iOSM_error') + var markers = drawLayer.selectAll('.qa_error.iOSM') .data(data, function(d) { return d.id; }); // exit @@ -116,8 +116,13 @@ export function svgImproveOSM(projection, context, dispatch) { var markersEnter = markers.enter() .append('g') .attr('class', function(d) { - return 'iOSM_error iOSM_error-' + d.id + ' iOSM_error_type_' + d.error_type + d.error_subtype; } - ); + return [ + 'qa_error', + d.source, + 'error_id-' + d.id, + 'error_type-' + d.error_type + '-' + d.error_subtype + ].join(' '); + }); markersEnter .append('ellipse') @@ -133,7 +138,7 @@ export function svgImproveOSM(projection, context, dispatch) { markersEnter .append('use') - .attr('class', 'iOSM_error-fill') + .attr('class', 'qa_error-fill') .attr('width', '20px') .attr('height', '20px') .attr('x', '-8px') @@ -152,7 +157,7 @@ export function svgImproveOSM(projection, context, dispatch) { if (touchLayer.empty()) return; var fillClass = context.getDebug('target') ? 'pink ' : 'nocolor '; - var targets = touchLayer.selectAll('.iOSM_error') + var targets = touchLayer.selectAll('.qa_error.iOSM') .data(data, function(d) { return d.id; }); // exit @@ -169,7 +174,7 @@ export function svgImproveOSM(projection, context, dispatch) { .merge(targets) .sort(sortY) .attr('class', function(d) { - return 'iOSM_error target iOSM_error-' + d.id + ' ' + fillClass; + return 'qa_error ' + d.source + ' target error_id-' + d.id + ' ' + fillClass; }) .attr('transform', getTransform); diff --git a/modules/svg/keepRight.js b/modules/svg/keepRight.js index c4b8807ab..126ffd320 100644 --- a/modules/svg/keepRight.js +++ b/modules/svg/keepRight.js @@ -54,9 +54,9 @@ export function svgKeepRight(projection, context, dispatch) { _keepRightVisible = false; drawLayer .style('display', 'none'); - drawLayer.selectAll('.kr_error') + drawLayer.selectAll('.qa_error.kr') .remove(); - touchLayer.selectAll('.kr_error') + touchLayer.selectAll('.qa_error.kr') .remove(); } } @@ -81,7 +81,7 @@ export function svgKeepRight(projection, context, dispatch) { function layerOff() { throttledRedraw.cancel(); drawLayer.interrupt(); - touchLayer.selectAll('.kr_error') + touchLayer.selectAll('.qa_error.kr') .remove(); drawLayer @@ -105,7 +105,7 @@ export function svgKeepRight(projection, context, dispatch) { var getTransform = svgPointTransform(projection); // Draw markers.. - var markers = drawLayer.selectAll('.kr_error') + var markers = drawLayer.selectAll('.qa_error.kr') .data(data, function(d) { return d.id; }); // exit @@ -116,8 +116,13 @@ export function svgKeepRight(projection, context, dispatch) { var markersEnter = markers.enter() .append('g') .attr('class', function(d) { - return 'kr_error kr_error-' + d.id + ' kr_error_type_' + d.parent_error_type; } - ); + return [ + 'qa_error', + d.source, + 'error_id-' + d.id, + 'error_type-' + d.parent_error_type + ].join(' '); + }); markersEnter .append('ellipse') @@ -133,7 +138,7 @@ export function svgKeepRight(projection, context, dispatch) { markersEnter .append('use') - .attr('class', 'kr_error-fill') + .attr('class', 'qa_error-fill') .attr('width', '20px') .attr('height', '20px') .attr('x', '-8px') @@ -152,7 +157,7 @@ export function svgKeepRight(projection, context, dispatch) { if (touchLayer.empty()) return; var fillClass = context.getDebug('target') ? 'pink ' : 'nocolor '; - var targets = touchLayer.selectAll('.kr_error') + var targets = touchLayer.selectAll('.qa_error.kr') .data(data, function(d) { return d.id; }); // exit @@ -169,7 +174,7 @@ export function svgKeepRight(projection, context, dispatch) { .merge(targets) .sort(sortY) .attr('class', function(d) { - return 'kr_error target kr_error-' + d.id + ' ' + fillClass; + return 'qa_error ' + d.source + ' target error_id-' + d.id + ' ' + fillClass; }) .attr('transform', getTransform); diff --git a/modules/ui/improveOSM_header.js b/modules/ui/improveOSM_header.js index 102f5a57a..6cff86a93 100644 --- a/modules/ui/improveOSM_header.js +++ b/modules/ui/improveOSM_header.js @@ -44,9 +44,9 @@ export function uiImproveOsmHeader() { iconEnter .append('div') .attr('class', function(d) { - return 'preset-icon-28 iOSM_error iOSM_error-' + d.id + ' iOSM_error_type_' + d.error_type + d.error_subtype; + return 'preset-icon-28 qa_error ' + d.source + ' error_id-' + d.id + ' error_type-' + d.error_type + '-' + d.error_subtype; }) - .call(svgIcon('#iD-icon-bolt', 'iOSM_error-fill')); + .call(svgIcon('#iD-icon-bolt', 'qa_error-fill')); headerEnter .append('div') diff --git a/modules/ui/keepRight_header.js b/modules/ui/keepRight_header.js index 1c8d9533c..534ecaf73 100644 --- a/modules/ui/keepRight_header.js +++ b/modules/ui/keepRight_header.js @@ -49,9 +49,9 @@ export function uiKeepRightHeader() { iconEnter .append('div') .attr('class', function(d) { - return 'preset-icon-28 kr_error kr_error-' + d.id + ' kr_error_type_' + d.parent_error_type; + return 'preset-icon-28 qa_error ' + d.source + ' error_id-' + d.id + ' error_type-' + d.parent_error_type; }) - .call(svgIcon('#iD-icon-bolt', 'kr_error-fill')); + .call(svgIcon('#iD-icon-bolt', 'qa_error-fill')); headerEnter .append('div') From ac4cec14292f718684eb3b36d7cd6f8d811a6439 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Sun, 20 Jan 2019 23:47:23 +0000 Subject: [PATCH 09/38] Add sidebar preview for ImproveOSM errors --- modules/ui/sidebar.js | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/modules/ui/sidebar.js b/modules/ui/sidebar.js index 0085b0f0c..7b4b2bdea 100644 --- a/modules/ui/sidebar.js +++ b/modules/ui/sidebar.js @@ -9,9 +9,16 @@ import { selectAll as d3_selectAll } from 'd3-selection'; -import { osmEntity, osmNote, krError } from '../osm'; +import { osmEntity, osmNote, impOsmError, krError } from '../osm'; import { services } from '../services'; -import { uiDataEditor, uiFeatureList, uiInspector, uiNoteEditor, uiKeepRightEditor } from './index'; +import { + uiDataEditor, + uiFeatureList, + uiInspector, + uiNoteEditor, + uiImproveOsmEditor, + uiKeepRightEditor +} from './index'; import { textDirection } from '../util/locale'; @@ -19,10 +26,12 @@ export function uiSidebar(context) { var inspector = uiInspector(context); var dataEditor = uiDataEditor(context); var noteEditor = uiNoteEditor(context); + var improveOsmEditor = uiImproveOsmEditor(context); var keepRightEditor = uiKeepRightEditor(context); var _current; var _wasData = false; var _wasNote = false; + var _wasIOsmError = false; var _wasKRError = false; @@ -131,6 +140,23 @@ export function uiSidebar(context) { selection.selectAll('.sidebar-component') .classed('inspector-hover', true); + } else if (datum instanceof impOsmError) { + _wasIOsmError = true; + + var improveOSM = services.improveOSM; + if (improveOSM) { + datum = improveOSM.getError(datum.id); + } + + d3_selectAll('.iOSM.qa_error') + .classed('hover', function(d) { return d.id === datum.id; }); + + sidebar + .show(improveOsmEditor.error(datum)); + + selection.selectAll('.sidebar-component') + .classed('inspector-hover', true); + } else if (datum instanceof krError) { _wasKRError = true; @@ -139,7 +165,7 @@ export function uiSidebar(context) { datum = keepRight.getError(datum.id); // marker may contain stale data - get latest } - d3_selectAll('.kr_error') + d3_selectAll('.kr.qa_error') .classed('hover', function(d) { return d.id === datum.id; }); sidebar @@ -173,9 +199,10 @@ export function uiSidebar(context) { inspector .state('hide'); - } else if (_wasData || _wasNote || _wasKRError) { + } else if (_wasData || _wasNote || _wasIOsmError || _wasKRError) { _wasNote = false; _wasData = false; + _wasIOsmError = false; _wasKRError = false; d3_selectAll('.note').classed('hover', false); d3_selectAll('.kr_error').classed('hover', false); From 9d2792836f1260cf039f89a7c4e74ee41ec28621 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 21 Jan 2019 16:24:53 +0000 Subject: [PATCH 10/38] Add ImproveOSM error resolution --- modules/services/improveOSM.js | 87 ++++++++++++++++++++++++- modules/ui/improveOSM_editor.js | 108 +++++++++++++++++++++++++++++++- 2 files changed, 191 insertions(+), 4 deletions(-) diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index 5f2b408ea..126c6aa34 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -10,6 +10,7 @@ import { request as d3_request } from 'd3-request'; import { geoExtent } from '../geo'; import { impOsmError } from '../osm'; +import { services } from './index'; import { t } from '../util/locale'; import { utilRebind, utilTiler, utilQsString } from '../util'; @@ -189,8 +190,14 @@ export default { comments: null, error_subtype: '', error_type: k, + identifier: { // this is used to post changes to the error + wayId: feature.wayId, + fromNodeId: feature.fromNodeId, + toNodeId: feature.toNodeId + }, object_id: feature.wayId, - object_type: 'way' + object_type: 'way', + status: feature.status }); //TODO include road type in description? @@ -217,7 +224,12 @@ export default { loc: pointAverage(feature.points), comments: null, error_subtype: geoType, - error_type: k + error_type: k, + identifier: { + x: feature.x, + y: feature.y + }, + status: feature.status }); d.replacements = { @@ -251,8 +263,10 @@ export default { comments: null, error_subtype: '', error_type: k, + identifier: feature.id, object_id: via_node, - object_type: 'node' + object_type: 'node', + status: feature.status }); // Variables used in the description @@ -278,6 +292,73 @@ export default { }) }, + postUpdate: function(d, callback) { + if (!services.osm.authenticated()) { // Username required in payload + return callback({ message: 'Not Authenticated', status: -3}, d); + } + if (_erCache.inflight[d.id]) { + return callback({ message: 'Error update already inflight', status: -2 }, d); + } + + var username = services.osm.userDetails(function(err, user) { + if (err) return ''; + + return user.display_name; + }); + + var that = this; + var type = d.error_type; + var payload = {}; + + payload.username = username; + + // Each error type has different data for identification + if (type === 'ow') { + payload.roadSegments = [ d.identifier ]; + } else if (type === 'mr') { + payload.tiles = [ d.identifier ]; + } else if (type === 'tr') { + payload.targetIds = [ d.identifier ]; + } + + // Separate requests required to comment and change status + var url = _impOsmUrls[type] + '/comment'; + + // Comments don't currently work + // if (d.newComment !== undefined) { + // payload.text = d.newComment; + + // _krCache.inflight[d.id] = d3_request(url) + // .header('Content-Type', 'application/json') + // .post(payload, function(back) { + // console.log(back); + // }); + // } + + if (d.newStatus !== d.status) { + payload.status = d.newStatus; + payload.text = 'status changed'; + + _erCache.inflight[d.id] = [d3_request(url) + .header('Content-Type', 'application/json') + .post(JSON.stringify(payload), function(err) { + delete _erCache.inflight[d.id]; + + if (d.newStatus === 'INVALID') { + that.removeError(d); + } else if (d.newStatus === 'SOLVED') { + that.removeError(d); + + //TODO the identifiers are ugly and can't be used frontend, use error position instead? + // or perhaps don't track this at all? + //_erCache.closed[d.error_type + ':' + d.identifier] = true; + } + + return callback(err, d); + })]; + } + }, + // get all cached errors covering the viewport getErrors: function(projection) { var viewport = projection.clipExtent(); diff --git a/modules/ui/improveOSM_editor.js b/modules/ui/improveOSM_editor.js index ed1d5065c..d235b2101 100644 --- a/modules/ui/improveOSM_editor.js +++ b/modules/ui/improveOSM_editor.js @@ -76,7 +76,113 @@ export function uiImproveOsmEditor(context) { .merge(editor) .call(errorHeader.error(_error)) .call(quickLinks.choices(choices)) - .call(errorDetails.error(_error)); + .call(errorDetails.error(_error)) + .call(improveOsmSaveSection); + } + + function improveOsmSaveSection(selection) { + var isSelected = (_error && _error.id === context.selectedErrorID()); + var isShown = (_error && (isSelected || _error.newComment || _error.comment)); + var saveSection = selection.selectAll('.error-save') + .data( + (isShown ? [_error] : []), + function(d) { return d.id + '-' + (d.status || 0); } + ); + + // exit + saveSection.exit() + .remove(); + + // enter + var saveSectionEnter = saveSection.enter() + .append('div') + .attr('class', 'keepRight-save save-section cf'); + + // update + saveSection = saveSectionEnter + .merge(saveSection) + .call(errorSaveButtons); + } + + function errorSaveButtons(selection) { + var isSelected = (_error && _error.id === context.selectedErrorID()); + var buttonSection = selection.selectAll('.buttons') + .data((isSelected ? [_error] : []), function(d) { return d.status + d.id; }); + + // exit + buttonSection.exit() + .remove(); + + // enter + var buttonEnter = buttonSection.enter() + .append('div') + .attr('class', 'buttons'); + + // Comments don't currently work + // buttonEnter + // .append('button') + // .attr('class', 'button comment-button action') + // .text(t('QA.keepRight.save_comment')); + + buttonEnter + .append('button') + .attr('class', 'button close-button action'); + + buttonEnter + .append('button') + .attr('class', 'button ignore-button action'); + + + // update + buttonSection = buttonSection + .merge(buttonEnter); + + // Comments don't currently work + // buttonSection.select('.comment-button') + // .attr('disabled', function(d) { + // return d.newComment === undefined ? true : null; + // }) + // .on('click.comment', function(d) { + // this.blur(); // avoid keeping focus on the button - #4641 + // var errorService = services.improveOSM; + // if (errorService) { + // errorService.postUpdate(d, function(err, error) { + // dispatch.call('change', error); + // }); + // } + // }); + + buttonSection.select('.close-button') + .text(function(d) { + var andComment = (d.newComment !== undefined ? '_comment' : ''); + return t('QA.keepRight.close' + andComment); + }) + .on('click.close', function(d) { + this.blur(); // avoid keeping focus on the button - #4641 + var errorService = services.improveOSM; + if (errorService) { + d.newStatus = 'SOLVED'; + errorService.postUpdate(d, function(err, error) { + dispatch.call('change', error); + }); + } + }); + + buttonSection.select('.ignore-button') + .text(function(d) { + var andComment = (d.newComment !== undefined ? '_comment' : ''); + return t('QA.keepRight.ignore' + andComment); + }) + .on('click.ignore', function(d) { + this.blur(); // avoid keeping focus on the button - #4641 + var errorService = services.improveOSM; + if (errorService) { + d.newStatus = 'INVALID'; + errorService.postUpdate(d, function(err, error) { + dispatch.call('change', error); + }); + } + }); } improveOsmEditor.error = function(val) { From 3174db76b7382b063f3475a7f236d7d4dfaf1cb7 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 21 Jan 2019 16:25:29 +0000 Subject: [PATCH 11/38] Fix north direction in some error descriptions --- modules/services/improveOSM.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index 126c6aa34..5fd2145c2 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -108,7 +108,8 @@ function cardinalDirection(bearing) { 180: 'south', 225: 'southwest', 270: 'west', - 315: 'northwest' + 315: 'northwest', + 360: 'north' }; return t('QA.improveOSM.directions.' + compass[dir]); @@ -163,7 +164,6 @@ export default { // 3 separate requests to store for each tile var requests = {}; - // TODO: Just implement TRs and One-ways for now, much more simple _forEach(_impOsmUrls, function(v, k) { var url = v + '/search?' + utilQsString(params); From 6bdd0cfa894126605fe60ac04a1c1e60f5164284 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 21 Jan 2019 16:34:18 +0000 Subject: [PATCH 12/38] Fix missing semicolons --- modules/services/improveOSM.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index 5fd2145c2..6f6e17573 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -212,7 +212,7 @@ export default { _erCache.data[d.id] = d; _erCache.rtree.insert(encodeErrorRtree(d)); - }) + }); } // Tiles at high zoom == missing roads @@ -225,10 +225,7 @@ export default { comments: null, error_subtype: geoType, error_type: k, - identifier: { - x: feature.x, - y: feature.y - }, + identifier: { x: feature.x, y: feature.y }, status: feature.status }); @@ -239,7 +236,7 @@ export default { _erCache.data[d.id] = d; _erCache.rtree.insert(encodeErrorRtree(d)); - }) + }); } // Entities at high zoom == turn restrictions @@ -281,7 +278,7 @@ export default { _erCache.data[d.id] = d; _erCache.rtree.insert(encodeErrorRtree(d)); - }) + }); } } ); @@ -289,7 +286,7 @@ export default { _erCache.inflight[tile.id] = requests; dispatch.call('loaded'); - }) + }); }, postUpdate: function(d, callback) { From 31091f115918dc5d74b3432f638a48521aebcbe3 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 21 Jan 2019 18:13:14 +0000 Subject: [PATCH 13/38] Add direction of travel to oneway errors --- data/core.yaml | 2 +- dist/locales/en.json | 2 +- modules/services/improveOSM.js | 8 +++++++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index b64e5cec9..ea30294c4 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -680,7 +680,7 @@ en: error_types: ow: title: Missing One-way - description: 'Along this section of {highway}, {percentage}% of {num_trips} recorded trips travel in the same direction. There may be missing a "oneway" tag.' + description: 'Along this section of {highway}, {percentage}% of {num_trips} recorded trips travel {travel_direction}. There may be missing a "oneway" tag.' mr: title: Missing Geometry description: '{num_trips} recorded trips in this area suggest there may be unmapped {geometry_type} here.' diff --git a/dist/locales/en.json b/dist/locales/en.json index 73bdc3eb0..39ddf7426 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -824,7 +824,7 @@ "error_types": { "ow": { "title": "Missing One-way", - "description": "Along this section of {highway}, {percentage}% of {num_trips} recorded trips travel in the same direction. There may be missing a \"oneway\" tag." + "description": "Along this section of {highway}, {percentage}% of {num_trips} recorded trips travel {travel_direction}. There may be missing a \"oneway\" tag." }, "mr": { "title": "Missing Geometry", diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index 6f6e17573..b50f0e48a 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -185,6 +185,11 @@ export default { // Road segments at high zoom == oneways if (data.roadSegments) { data.roadSegments.forEach(function(feature) { + var p1 = feature.points[0]; + var p2 = feature.points[1]; + + var dir_of_travel = cardinalDirection(relativeBearing(p1, p2)); + var d = new impOsmError({ loc: pointAverage(feature.points), // TODO: This isn't great for curved roads, would be better to find actual midpoint of segment comments: null, @@ -207,7 +212,8 @@ export default { d.replacements = { percentage: feature.percentOfTrips, num_trips: feature.numberOfTrips, - highway: linkErrorObject(t('QA.keepRight.error_parts.highway')) + highway: linkErrorObject(t('QA.keepRight.error_parts.highway')), + travel_direction: dir_of_travel }; _erCache.data[d.id] = d; From 3334abf776a1dff67b5c35856c4f7b40f3d5f0bc Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Mon, 21 Jan 2019 18:23:19 +0000 Subject: [PATCH 14/38] Remove some unnecessary code --- modules/services/improveOSM.js | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index b50f0e48a..b8d9206f4 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -140,10 +140,10 @@ export default { loadErrors: function(projection) { var options = { client: 'iD', - confidenceLevel: 'C1', // most confident cases only for now + confidenceLevel: 'C1', // most confident only, still have false positives status: 'OPEN', - type: 'PARKING,ROAD,BOTH,PATH', // exclude WATER - zoom: '19' + type: 'PARKING,ROAD,BOTH,PATH', // exclude WATER as it doesn't seem useful + zoom: '19' // Use a high zoom so that clusters aren't returned }; // determine the needed tiles to cover the view @@ -174,14 +174,6 @@ export default { if (err) return; _erCache.loaded[tile.id] = true; - // Clusters are returned at low zoom - // if (data.clusters) { - // data.clusters.forEach(function(feature) { - // var loc = feature.point; - // var size = feature.size; - // }); - // } - // Road segments at high zoom == oneways if (data.roadSegments) { data.roadSegments.forEach(function(feature) { @@ -205,9 +197,6 @@ export default { status: feature.status }); - //TODO include road type in description? - // feature.type - // Variables used in the description d.replacements = { percentage: feature.percentOfTrips, @@ -303,7 +292,7 @@ export default { return callback({ message: 'Error update already inflight', status: -2 }, d); } - var username = services.osm.userDetails(function(err, user) { + var osmUsername = services.osm.userDetails(function(err, user) { if (err) return ''; return user.display_name; @@ -311,9 +300,9 @@ export default { var that = this; var type = d.error_type; - var payload = {}; - - payload.username = username; + var payload = { + username: osmUsername + }; // Each error type has different data for identification if (type === 'ow') { From 5030346257e5904d333000edde17a6d8d93bdc3b Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Tue, 22 Jan 2019 12:22:12 +0000 Subject: [PATCH 15/38] Update data layers test --- test/spec/svg/layers.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/test/spec/svg/layers.js b/test/spec/svg/layers.js index 5a40332d6..9009dcc2d 100644 --- a/test/spec/svg/layers.js +++ b/test/spec/svg/layers.js @@ -26,18 +26,19 @@ 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(11); + expect(nodes.length).to.eql(12); 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('keepRight')).to.be.true; - expect(d3.select(nodes[4]).classed('streetside')).to.be.true; - expect(d3.select(nodes[5]).classed('mapillary-images')).to.be.true; - expect(d3.select(nodes[6]).classed('mapillary-signs')).to.be.true; - expect(d3.select(nodes[7]).classed('openstreetcam-images')).to.be.true; - expect(d3.select(nodes[8]).classed('debug')).to.be.true; - expect(d3.select(nodes[9]).classed('geolocate')).to.be.true; - expect(d3.select(nodes[10]).classed('touch')).to.be.true; + expect(d3.select(nodes[4]).classed('improveOSM')).to.be.true; + expect(d3.select(nodes[5]).classed('streetside')).to.be.true; + expect(d3.select(nodes[6]).classed('mapillary-images')).to.be.true; + expect(d3.select(nodes[7]).classed('mapillary-signs')).to.be.true; + expect(d3.select(nodes[8]).classed('openstreetcam-images')).to.be.true; + expect(d3.select(nodes[9]).classed('debug')).to.be.true; + expect(d3.select(nodes[10]).classed('geolocate')).to.be.true; + expect(d3.select(nodes[11]).classed('touch')).to.be.true; }); }); From fe73b3df5a038cbed3167d25dbc1ada11ecc5cf0 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Tue, 22 Jan 2019 16:26:06 +0000 Subject: [PATCH 16/38] Improve error icon position and appearance --- modules/svg/improveOSM.js | 21 +++++++++++---------- svg/iD-sprite/icons/icon-bolt.svg | 3 +-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/modules/svg/improveOSM.js b/modules/svg/improveOSM.js index b5941a579..57e4b2b77 100644 --- a/modules/svg/improveOSM.js +++ b/modules/svg/improveOSM.js @@ -17,11 +17,12 @@ export function svgImproveOSM(projection, context, dispatch) { var _improveOsmVisible = false; + // TODO this is just reusing the path from the svg, probably some way to make that automatic (and therefore general) function markerPath(selection, klass) { selection .attr('class', klass) - .attr('transform', 'translate(-4, -24)') - .attr('d', 'M11.6,6.2H7.1l1.4-5.1C8.6,0.6,8.1,0,7.5,0H2.2C1.7,0,1.3,0.3,1.3,0.8L0,10.2c-0.1,0.6,0.4,1.1,0.9,1.1h4.6l-1.8,7.6C3.6,19.4,4.1,20,4.7,20c0.3,0,0.6-0.2,0.8-0.5l6.9-11.9C12.7,7,12.3,6.2,11.6,6.2z'); + .attr('transform', 'translate(-9, -20)') + .attr('d', 'M15,6.5h-4.2l1.3-4.7c0.1-0.5-0.4-1-0.9-1H6.2C5.8,0.7,5.4,1,5.4,1.5l-1.2,8.7c-0.1,0.6,0.4,1,0.8,1h4.3l-1.7,7.1c-0.1,0.5,0.4,1,0.9,1c0.3,0,0.6-0.2,0.7-0.5l6.4-11C16,7.2,15.6,6.5,15,6.5z'); } @@ -126,10 +127,10 @@ export function svgImproveOSM(projection, context, dispatch) { markersEnter .append('ellipse') - .attr('cx', 0.5) - .attr('cy', 1) - .attr('rx', 6.5) - .attr('ry', 3) + .attr('cx', 0) + .attr('cy', 0) + .attr('rx', 5) + .attr('ry', 2) .attr('class', 'stroke'); markersEnter @@ -141,8 +142,8 @@ export function svgImproveOSM(projection, context, dispatch) { .attr('class', 'qa_error-fill') .attr('width', '20px') .attr('height', '20px') - .attr('x', '-8px') - .attr('y', '-22px') + .attr('x', '-9px') // point of bolt is not perfectly centered + .attr('y', '-20px') .attr('xlink:href', '#iD-icon-bolt'); // update @@ -169,8 +170,8 @@ export function svgImproveOSM(projection, context, dispatch) { .append('rect') .attr('width', '20px') .attr('height', '20px') - .attr('x', '-8px') - .attr('y', '-22px') + .attr('x', '-9px') + .attr('y', '-20px') .merge(targets) .sort(sortY) .attr('class', function(d) { diff --git a/svg/iD-sprite/icons/icon-bolt.svg b/svg/iD-sprite/icons/icon-bolt.svg index 8078987b2..aaa264a45 100644 --- a/svg/iD-sprite/icons/icon-bolt.svg +++ b/svg/iD-sprite/icons/icon-bolt.svg @@ -1,8 +1,7 @@ - + From 6483c54ca47a88eec7e2dc45a8824ee7a109dcc5 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Tue, 22 Jan 2019 18:15:40 +0000 Subject: [PATCH 17/38] Update ImproveOSM error colouring Fix similar orange colours by using the same zoomed in colours as ImproveOSM for missing geometry (i.e. pink for roads). This frees up red for turn restrictions (as they are typically signed in red) and therefore blue for one-ways (as they tend to be signed blue or black). --- css/65_data.css | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/css/65_data.css b/css/65_data.css index 8585b70b0..bc3b1c584 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -120,11 +120,11 @@ ------------------------------------------------------- */ .iOSM.error_type-ow- { /* missing one way */ - color: #EE7600; + color: #1E90FF; } .iOSM.error_type-mr-road { /* missing road */ - color: #B0171F; + color: #B452CD; } .iOSM.error_type-mr-path { /* missing path */ color: #A0522D; @@ -137,7 +137,7 @@ } .iOSM.error_type-tr- { /* missing turn restriction */ - color: #1E90FF; + color: #B0171F; } /* Custom Map Data (geojson, gpx, kml, vector tile) */ From f6746a28ff392224a3b5ed4bde98351519a626f4 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Tue, 22 Jan 2019 18:32:26 +0000 Subject: [PATCH 18/38] Match error red to road sign red --- css/65_data.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/css/65_data.css b/css/65_data.css index bc3b1c584..61073966f 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -137,7 +137,7 @@ } .iOSM.error_type-tr- { /* missing turn restriction */ - color: #B0171F; + color: #EC1C24; } /* Custom Map Data (geojson, gpx, kml, vector tile) */ From 88a59cb15f2e809a8657c74911ae1527ffa124d4 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Tue, 22 Jan 2019 18:38:17 +0000 Subject: [PATCH 19/38] Stop hijacking the inflight cache This could result in error updates getting aborted when new errors are loaded as their individual ids are never going to match tile IDs (see `abortUnwantedRequests` function). --- modules/services/improveOSM.js | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index b8d9206f4..a988ef0d0 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -132,6 +132,7 @@ export default { data: {}, loaded: {}, inflight: {}, + outflight: {}, closed: {}, rtree: rbush() }; @@ -288,7 +289,7 @@ export default { if (!services.osm.authenticated()) { // Username required in payload return callback({ message: 'Not Authenticated', status: -3}, d); } - if (_erCache.inflight[d.id]) { + if (_erCache.outflight[d.id]) { return callback({ message: 'Error update already inflight', status: -2 }, d); } @@ -319,22 +320,16 @@ export default { // Comments don't currently work // if (d.newComment !== undefined) { // payload.text = d.newComment; - - // _krCache.inflight[d.id] = d3_request(url) - // .header('Content-Type', 'application/json') - // .post(payload, function(back) { - // console.log(back); - // }); // } if (d.newStatus !== d.status) { payload.status = d.newStatus; payload.text = 'status changed'; - _erCache.inflight[d.id] = [d3_request(url) + _erCache.outflight[d.id] = d3_request(url) .header('Content-Type', 'application/json') .post(JSON.stringify(payload), function(err) { - delete _erCache.inflight[d.id]; + delete _erCache.outflight[d.id]; if (d.newStatus === 'INVALID') { that.removeError(d); @@ -347,7 +342,7 @@ export default { } return callback(err, d); - })]; + }); } }, From 4a166d7a5b270a44f59894a9000d69af10b4476c Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Wed, 23 Jan 2019 13:55:14 +0000 Subject: [PATCH 20/38] Use more descriptive key names --- modules/services/improveOSM.js | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index a988ef0d0..7e577fc67 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -36,13 +36,13 @@ function abortRequest(i) { } function abortUnwantedRequests(cache, tiles) { - _forEach(cache.inflight, function(v, k) { + _forEach(cache.inflightTile, function(v, k) { var wanted = _find(tiles, function(tile) { return k === tile.id; }); if (!wanted) { abortRequest(v); - delete cache.inflight[k]; + delete cache.inflightTile[k]; } }); } @@ -126,13 +126,13 @@ export default { reset: function() { if (_erCache) { - _forEach(_erCache.inflight, abortRequest); + _forEach(_erCache.inflightTile, abortRequest); } _erCache = { data: {}, - loaded: {}, - inflight: {}, - outflight: {}, + loadedTile: {}, + inflightTile: {}, + inflightPost: {}, closed: {}, rtree: rbush() }; @@ -157,7 +157,7 @@ export default { // issue new requests.. tiles.forEach(function(tile) { - if (_erCache.loaded[tile.id] || _erCache.inflight[tile.id]) return; + if (_erCache.loadedTile[tile.id] || _erCache.inflightTile[tile.id]) return; var rect = tile.extent.rectangle(); var params = _extend({}, options, { east: rect[0], south: rect[3], west: rect[2], north: rect[1] }); @@ -170,10 +170,10 @@ export default { requests[k] = d3_json(url, function(err, data) { - delete _erCache.inflight[tile.id]; + delete _erCache.inflightTile[tile.id]; if (err) return; - _erCache.loaded[tile.id] = true; + _erCache.loadedTile[tile.id] = true; // Road segments at high zoom == oneways if (data.roadSegments) { @@ -280,7 +280,7 @@ export default { ); }); - _erCache.inflight[tile.id] = requests; + _erCache.inflightTile[tile.id] = requests; dispatch.call('loaded'); }); }, @@ -289,7 +289,7 @@ export default { if (!services.osm.authenticated()) { // Username required in payload return callback({ message: 'Not Authenticated', status: -3}, d); } - if (_erCache.outflight[d.id]) { + if (_erCache.inflightPost[d.id]) { return callback({ message: 'Error update already inflight', status: -2 }, d); } @@ -326,10 +326,10 @@ export default { payload.status = d.newStatus; payload.text = 'status changed'; - _erCache.outflight[d.id] = d3_request(url) + _erCache.inflightPost[d.id] = d3_request(url) .header('Content-Type', 'application/json') .post(JSON.stringify(payload), function(err) { - delete _erCache.outflight[d.id]; + delete _erCache.inflightPost[d.id]; if (d.newStatus === 'INVALID') { that.removeError(d); From 5d907c1e16ae0794518eda70bdaa4b68df0c939b Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Wed, 23 Jan 2019 16:27:56 +0000 Subject: [PATCH 21/38] Render errors like points with icons This allows more diverse representation of the error subject at a glance than relying on colour alone. However, it would be good to have a generic error icon that can contain icons which is differentiated from the point icon for clarity. Sidebar header currently still uses the bolt icon until I figure out how to deal with that. Also the font awesome icons don't seem to work, perhaps there's additional code needed for those that I've missed. --- modules/services/improveOSM.js | 9 +++++++ modules/svg/improveOSM.js | 45 +++++++++++++++++++++------------- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index 7e577fc67..dcb600f86 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -188,6 +188,7 @@ export default { comments: null, error_subtype: '', error_type: k, + icon: 'fas-arrow-circle-up', //TODO: Change arrow based on direction? identifier: { // this is used to post changes to the error wayId: feature.wayId, fromNodeId: feature.fromNodeId, @@ -215,12 +216,19 @@ export default { if (data.tiles) { data.tiles.forEach(function(feature) { var geoType = feature.type.toLowerCase(); + var geoIcons = { + road: 'maki-car', + parking: 'maki-parking', + both: 'maki-car', + path: 'maki-shoe' + }; var d = new impOsmError({ loc: pointAverage(feature.points), comments: null, error_subtype: geoType, error_type: k, + icon: geoIcons[geoType], identifier: { x: feature.x, y: feature.y }, status: feature.status }); @@ -256,6 +264,7 @@ export default { comments: null, error_subtype: '', error_type: k, + icon: 'temaki-junction', identifier: feature.id, object_id: via_node, object_type: 'node', diff --git a/modules/svg/improveOSM.js b/modules/svg/improveOSM.js index 57e4b2b77..7f603462b 100644 --- a/modules/svg/improveOSM.js +++ b/modules/svg/improveOSM.js @@ -16,13 +16,11 @@ export function svgImproveOSM(projection, context, dispatch) { var drawLayer = d3_select(null); var _improveOsmVisible = false; - - // TODO this is just reusing the path from the svg, probably some way to make that automatic (and therefore general) function markerPath(selection, klass) { selection .attr('class', klass) - .attr('transform', 'translate(-9, -20)') - .attr('d', 'M15,6.5h-4.2l1.3-4.7c0.1-0.5-0.4-1-0.9-1H6.2C5.8,0.7,5.4,1,5.4,1.5l-1.2,8.7c-0.1,0.6,0.4,1,0.8,1h4.3l-1.7,7.1c-0.1,0.5,0.4,1,0.9,1c0.3,0,0.6-0.2,0.7-0.5l6.4-11C16,7.2,15.6,6.5,15,6.5z'); + .attr('transform', 'translate(-8, -23)') + .attr('d', 'M 17,8 C 17,13 11,21 8.5,23.5 C 6,21 0,13 0,8 C 0,4 4,-0.5 8.5,-0.5 C 13,-0.5 17,4 17,8 z'); } @@ -125,26 +123,39 @@ export function svgImproveOSM(projection, context, dispatch) { ].join(' '); }); + markersEnter + .append('path') + .call(markerPath, 'shadow'); + markersEnter .append('ellipse') - .attr('cx', 0) - .attr('cy', 0) - .attr('rx', 5) + .attr('cx', 0.5) + .attr('cy', 1) + .attr('rx', 6.5) .attr('ry', 2) .attr('class', 'stroke'); markersEnter .append('path') - .call(markerPath, 'shadow'); + .attr('fill', 'currentColor') + .call(markerPath, 'qa_error-fill'); markersEnter .append('use') - .attr('class', 'qa_error-fill') - .attr('width', '20px') - .attr('height', '20px') - .attr('x', '-9px') // point of bolt is not perfectly centered - .attr('y', '-20px') - .attr('xlink:href', '#iD-icon-bolt'); + .attr('transform', 'translate(-5, -19)') + .attr('class', 'icon') + .attr('width', '11px') + .attr('height', '11px') + .attr('xlink:href', function(d) { + var picon = d.icon; + + if (!picon) { + return ''; + } else { + var isMaki = /^maki-/.test(picon); + return '#' + picon + (isMaki ? '-11' : ''); + } + }); // update markers @@ -169,9 +180,9 @@ export function svgImproveOSM(projection, context, dispatch) { targets.enter() .append('rect') .attr('width', '20px') - .attr('height', '20px') - .attr('x', '-9px') - .attr('y', '-20px') + .attr('height', '30px') + .attr('x', '-10px') + .attr('y', '-26px') .merge(targets) .sort(sortY) .attr('class', function(d) { From 54e02ec28de0a619cc8117d8a68c7309a98a8c45 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Wed, 23 Jan 2019 21:22:04 +0000 Subject: [PATCH 22/38] Introduce new container icon for errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As per my last commit, this icon differentiates errors from points and still allows us to specifiy icons for errors to differentiate them amongst eachother. I learned quite a bit about SVGs and using them in HTML while implementing this 😝Had some issues getting the icon to center in the header so resorted to using a flexbox instead of absolute positioning being used elsewhere. --- css/65_data.css | 12 ++++++++++++ modules/svg/improveOSM.js | 18 +++++++++--------- modules/ui/improveOSM_header.js | 32 ++++++++++++++++++++++++++++---- 3 files changed, 49 insertions(+), 13 deletions(-) diff --git a/css/65_data.css b/css/65_data.css index 61073966f..2686d940d 100644 --- a/css/65_data.css +++ b/css/65_data.css @@ -42,6 +42,18 @@ height: 15px; } +/* adjustment for error icon */ + +.kr_error-header-icon .preset-icon-28 { + top: auto; + left: auto; +} + +.kr_error-header-icon { + display: flex; + align-items: center; + justify-content: center; +} /* Keep Right Errors ------------------------------------------------------- */ diff --git a/modules/svg/improveOSM.js b/modules/svg/improveOSM.js index 7f603462b..b9881ff29 100644 --- a/modules/svg/improveOSM.js +++ b/modules/svg/improveOSM.js @@ -19,8 +19,8 @@ export function svgImproveOSM(projection, context, dispatch) { function markerPath(selection, klass) { selection .attr('class', klass) - .attr('transform', 'translate(-8, -23)') - .attr('d', 'M 17,8 C 17,13 11,21 8.5,23.5 C 6,21 0,13 0,8 C 0,4 4,-0.5 8.5,-0.5 C 13,-0.5 17,4 17,8 z'); + .attr('transform', 'translate(-10, -28)') + .attr('points', '16,3 4,3 1,6 1,17 4,20 7,20 10,27 13,20 16,20 19,17.033 19,6'); } @@ -124,25 +124,25 @@ export function svgImproveOSM(projection, context, dispatch) { }); markersEnter - .append('path') + .append('polygon') .call(markerPath, 'shadow'); markersEnter .append('ellipse') - .attr('cx', 0.5) - .attr('cy', 1) - .attr('rx', 6.5) + .attr('cx', 0) + .attr('cy', 0) + .attr('rx', 4.5) .attr('ry', 2) .attr('class', 'stroke'); markersEnter - .append('path') + .append('polygon') .attr('fill', 'currentColor') .call(markerPath, 'qa_error-fill'); markersEnter .append('use') - .attr('transform', 'translate(-5, -19)') + .attr('transform', 'translate(-5.5, -21)') .attr('class', 'icon') .attr('width', '11px') .attr('height', '11px') @@ -182,7 +182,7 @@ export function svgImproveOSM(projection, context, dispatch) { .attr('width', '20px') .attr('height', '30px') .attr('x', '-10px') - .attr('y', '-26px') + .attr('y', '-28px') .merge(targets) .sort(sortY) .attr('class', function(d) { diff --git a/modules/ui/improveOSM_header.js b/modules/ui/improveOSM_header.js index 6cff86a93..d33835e23 100644 --- a/modules/ui/improveOSM_header.js +++ b/modules/ui/improveOSM_header.js @@ -41,12 +41,36 @@ export function uiImproveOsmHeader() { .attr('class', 'kr_error-header-icon') .classed('new', function(d) { return d.id < 0; }); - iconEnter - .append('div') + var svgEnter = iconEnter + .append('svg') + .attr('width', '20px') + .attr('height', '30px') + .attr('viewbox', '0 0 20 30') .attr('class', function(d) { return 'preset-icon-28 qa_error ' + d.source + ' error_id-' + d.id + ' error_type-' + d.error_type + '-' + d.error_subtype; - }) - .call(svgIcon('#iD-icon-bolt', 'qa_error-fill')); + }); + + svgEnter + .append('polygon') + .attr('fill', 'currentColor') + .attr('class', 'qa_error-fill') + .attr('points', '16,3 4,3 1,6 1,17 4,20 7,20 10,27 13,20 16,20 19,17.033 19,6'); + + svgEnter + .append('use') + .attr('width', '11px') + .attr('height', '11px') + .attr('transform', 'translate(4.5, 7)') + .attr('xlink:href', function(d) { + var picon = d.icon; + + if (!picon) { + return ''; + } else { + var isMaki = /^maki-/.test(picon); + return '#' + picon + (isMaki ? '-11' : ''); + } + }); headerEnter .append('div') From ee5f9f3bf256de0afdb830cd1a1a877d6afc907d Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Wed, 30 Jan 2019 17:43:05 +0000 Subject: [PATCH 23/38] Use consistent short service name --- modules/behavior/hover.js | 4 ++-- modules/behavior/select.js | 4 ++-- modules/osm/improveOSM.js | 20 ++++++++++---------- modules/osm/index.js | 2 +- modules/services/improveOSM.js | 12 ++++++------ modules/ui/sidebar.js | 4 ++-- 6 files changed, 23 insertions(+), 23 deletions(-) diff --git a/modules/behavior/hover.js b/modules/behavior/hover.js index 338541253..9645b83e2 100644 --- a/modules/behavior/hover.js +++ b/modules/behavior/hover.js @@ -5,7 +5,7 @@ import { select as d3_select } from 'd3-selection'; -import { osmEntity, osmNote, krError, impOsmError } from '../osm'; +import { osmEntity, osmNote, krError, iOsmError } from '../osm'; import { utilKeybinding, utilRebind } from '../util'; @@ -113,7 +113,7 @@ export function behaviorHover(context) { selector = '.data' + datum.__featurehash__; } else if ( - datum instanceof impOsmError || + datum instanceof iOsmError || datum instanceof krError ) { entity = datum; diff --git a/modules/behavior/select.js b/modules/behavior/select.js index 9a780bec0..8043648d3 100644 --- a/modules/behavior/select.js +++ b/modules/behavior/select.js @@ -19,7 +19,7 @@ import { import { osmEntity, osmNote, - impOsmError, + iOsmError, krError } from '../osm'; @@ -171,7 +171,7 @@ export function behaviorSelect(context) { context .selectedNoteID(datum.id) .enter(modeSelectNote(context, datum.id)); - } else if (datum instanceof impOsmError & !isMultiselect) { // clicked an improveOSM error + } else if (datum instanceof iOsmError & !isMultiselect) { // clicked an improveOSM error context .selectedErrorID(datum.id) .enter(modeSelectError(context, datum.id, datum.source)); diff --git a/modules/osm/improveOSM.js b/modules/osm/improveOSM.js index 353e1ece5..fc438ee53 100644 --- a/modules/osm/improveOSM.js +++ b/modules/osm/improveOSM.js @@ -1,9 +1,9 @@ import _extend from 'lodash-es/extend'; -export function impOsmError() { - if (!(this instanceof impOsmError)) { - return (new impOsmError()).initialize(arguments); +export function iOsmError() { + if (!(this instanceof iOsmError)) { + return (new iOsmError()).initialize(arguments); } else if (arguments.length) { this.initialize(arguments); } @@ -11,17 +11,17 @@ export function impOsmError() { // ImproveOSM has no error IDs unfortunately // So no way to explicitly refer to each error in their DB -impOsmError.id = function() { - return impOsmError.id.next--; +iOsmError.id = function() { + return iOsmError.id.next--; }; -impOsmError.id.next = -1; +iOsmError.id.next = -1; -_extend(impOsmError.prototype, { +_extend(iOsmError.prototype, { - type: 'impOsmError', + type: 'iOsmError', source: 'iOSM', initialize: function(sources) { @@ -39,13 +39,13 @@ _extend(impOsmError.prototype, { } if (!this.id) { - this.id = impOsmError.id() + ''; // as string + this.id = iOsmError.id() + ''; // as string } return this; }, update: function(attrs) { - return impOsmError(this, attrs); // {v: 1 + (this.v || 0)} + return iOsmError(this, attrs); // {v: 1 + (this.v || 0)} } }); diff --git a/modules/osm/index.js b/modules/osm/index.js index 72f7b3ac4..d97cb8feb 100644 --- a/modules/osm/index.js +++ b/modules/osm/index.js @@ -1,7 +1,7 @@ export { osmChangeset } from './changeset'; export { osmEntity } from './entity'; export { krError } from './keepRight'; -export { impOsmError } from './improveOSM'; +export { iOsmError } from './improveOSM'; export { osmNode } from './node'; export { osmNote } from './note'; export { osmRelation } from './relation'; diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index dcb600f86..36ed4d678 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -9,7 +9,7 @@ import { json as d3_json } from 'd3-request'; import { request as d3_request } from 'd3-request'; import { geoExtent } from '../geo'; -import { impOsmError } from '../osm'; +import { iOsmError } from '../osm'; import { services } from './index'; import { t } from '../util/locale'; import { utilRebind, utilTiler, utilQsString } from '../util'; @@ -183,7 +183,7 @@ export default { var dir_of_travel = cardinalDirection(relativeBearing(p1, p2)); - var d = new impOsmError({ + var d = new iOsmError({ loc: pointAverage(feature.points), // TODO: This isn't great for curved roads, would be better to find actual midpoint of segment comments: null, error_subtype: '', @@ -223,7 +223,7 @@ export default { path: 'maki-shoe' }; - var d = new impOsmError({ + var d = new iOsmError({ loc: pointAverage(feature.points), comments: null, error_subtype: geoType, @@ -259,7 +259,7 @@ export default { var dir_of_travel = cardinalDirection(relativeBearing(p1, p2)); - var d = new impOsmError({ + var d = new iOsmError({ loc: [loc.lon, loc.lat], comments: null, error_subtype: '', @@ -374,7 +374,7 @@ export default { // replace a single error in the cache replaceError: function(error) { - if (!(error instanceof impOsmError) || !error.id) return; + if (!(error instanceof iOsmError) || !error.id) return; _erCache.data[error.id] = error; updateRtree(encodeErrorRtree(error), true); // true = replace @@ -383,7 +383,7 @@ export default { // remove a single error from the cache removeError: function(error) { - if (!(error instanceof impOsmError) || !error.id) return; + if (!(error instanceof iOsmError) || !error.id) return; delete _erCache.data[error.id]; updateRtree(encodeErrorRtree(error), false); // false = remove diff --git a/modules/ui/sidebar.js b/modules/ui/sidebar.js index 7b4b2bdea..3192b5315 100644 --- a/modules/ui/sidebar.js +++ b/modules/ui/sidebar.js @@ -9,7 +9,7 @@ import { selectAll as d3_selectAll } from 'd3-selection'; -import { osmEntity, osmNote, impOsmError, krError } from '../osm'; +import { osmEntity, osmNote, iOsmError, krError } from '../osm'; import { services } from '../services'; import { uiDataEditor, @@ -140,7 +140,7 @@ export function uiSidebar(context) { selection.selectAll('.sidebar-component') .classed('inspector-hover', true); - } else if (datum instanceof impOsmError) { + } else if (datum instanceof iOsmError) { _wasIOsmError = true; var improveOSM = services.improveOSM; From 45ce4e4c5783a5290a2a7c92d1c26128720249cb Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Wed, 30 Jan 2019 17:54:09 +0000 Subject: [PATCH 24/38] Move turn restriction error position slightly --- modules/services/improveOSM.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index 36ed4d678..7a347027e 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -8,7 +8,7 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; import { json as d3_json } from 'd3-request'; import { request as d3_request } from 'd3-request'; -import { geoExtent } from '../geo'; +import { geoExtent, geoVecAdd } from '../geo'; import { iOsmError } from '../osm'; import { services } from './index'; import { t } from '../util/locale'; @@ -178,6 +178,7 @@ export default { // Road segments at high zoom == oneways if (data.roadSegments) { data.roadSegments.forEach(function(feature) { + // Travel direction along way segment clarifies one-way direction var p1 = feature.points[0]; var p2 = feature.points[1]; @@ -248,19 +249,23 @@ export default { data.entities.forEach(function(feature) { var loc = feature.point; + // Bump position slightly so junction node is accessible + loc = geoVecAdd([loc.lon, loc.lat], [0, 0.00001]); + // Elements are presented in a strange way var ids = feature.id.split(','); var from_way = ids[0]; var via_node = ids[3]; var to_way = ids[2].split(':')[1]; + // Travel direction along from_way clarifies the turn restriction var p1 = feature.segments[0].points[0]; var p2 = feature.segments[0].points[1]; var dir_of_travel = cardinalDirection(relativeBearing(p1, p2)); var d = new iOsmError({ - loc: [loc.lon, loc.lat], + loc: loc, comments: null, error_subtype: '', error_type: k, From 4157cb1f257ea3c42e9eaff0e092561bdf405e9b Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Wed, 30 Jan 2019 18:04:37 +0000 Subject: [PATCH 25/38] Update QA help string --- data/core.yaml | 2 +- dist/locales/en.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index ea30294c4..214913dbc 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -1143,7 +1143,7 @@ en: title: Quality Assurance intro: "*Quality Assurance* (Q/A) tools can find improper tags, disconnected roads, and other issues with OpenStreetMap, which mappers can then fix. To view existing Q/A issues, click the {data} **Map data** panel to enable a specific Q/A layer." tools_h: "Tools" - tools: "The following tools are currently supported: [KeepRight](https://www.keepright.at/). Expect iD to support [Osmose](https://osmose.openstreetmap.fr/), [ImproveOSM](https://improveosm.org/en/), and more Q/A tools in the future." + tools: "The following tools are currently supported: [KeepRight](https://www.keepright.at/) and [ImproveOSM](https://improveosm.org/en/). Expect iD to support [Osmose](https://osmose.openstreetmap.fr/) and more Q/A tools in the future." issues_h: "Handling Issues" issues: "Handling Q/A issues is similar to handling notes. Click on a marker to view the issue details in the sidebar. Each tool has its own capabilities, but generally you can comment and/or close an issue." field: diff --git a/dist/locales/en.json b/dist/locales/en.json index 39ddf7426..8e2d529ed 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1383,7 +1383,7 @@ "title": "Quality Assurance", "intro": "*Quality Assurance* (Q/A) tools can find improper tags, disconnected roads, and other issues with OpenStreetMap, which mappers can then fix. To view existing Q/A issues, click the {data} **Map data** panel to enable a specific Q/A layer.", "tools_h": "Tools", - "tools": "The following tools are currently supported: [KeepRight](https://www.keepright.at/). Expect iD to support [Osmose](https://osmose.openstreetmap.fr/), [ImproveOSM](https://improveosm.org/en/), and more Q/A tools in the future.", + "tools": "The following tools are currently supported: [KeepRight](https://www.keepright.at/) and [ImproveOSM](https://improveosm.org/en/). Expect iD to support [Osmose](https://osmose.openstreetmap.fr/) and more Q/A tools in the future.", "issues_h": "Handling Issues", "issues": "Handling Q/A issues is similar to handling notes. Click on a marker to view the issue details in the sidebar. Each tool has its own capabilities, but generally you can comment and/or close an issue." }, From 60128b0402d02f9d6489ca93cb9f16409ca6a407 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Wed, 30 Jan 2019 20:11:52 +0000 Subject: [PATCH 26/38] Fix one-way error positioning and description The direction of travel in the description was misleading as the way segment wasn't necessary linear and so the direction could change as you travel along the way. This is more explicit as it specifies the detected start/end nodes (also useful to show that it's not the whole way that might be one-way). The position is now taken as the central node in the `feature.points` list (or the average of the central two when there are an even number of points). --- data/core.yaml | 2 +- dist/locales/en.json | 2 +- modules/services/improveOSM.js | 21 +++++++++++++++------ 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index 214913dbc..2bb4915b8 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -680,7 +680,7 @@ en: error_types: ow: title: Missing One-way - description: 'Along this section of {highway}, {percentage}% of {num_trips} recorded trips travel {travel_direction}. There may be missing a "oneway" tag.' + description: 'Along this section of {highway}, {percentage}% of {num_trips} recorded trips travel from {from_node} to {to_node}. There may be missing a "oneway" tag.' mr: title: Missing Geometry description: '{num_trips} recorded trips in this area suggest there may be unmapped {geometry_type} here.' diff --git a/dist/locales/en.json b/dist/locales/en.json index 8e2d529ed..f4ef0c186 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -824,7 +824,7 @@ "error_types": { "ow": { "title": "Missing One-way", - "description": "Along this section of {highway}, {percentage}% of {num_trips} recorded trips travel {travel_direction}. There may be missing a \"oneway\" tag." + "description": "Along this section of {highway}, {percentage}% of {num_trips} recorded trips travel from {from_node} to {to_node}. There may be missing a \"oneway\" tag." }, "mr": { "title": "Missing Geometry", diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index 7a347027e..84164de6b 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -178,14 +178,22 @@ export default { // Road segments at high zoom == oneways if (data.roadSegments) { data.roadSegments.forEach(function(feature) { - // Travel direction along way segment clarifies one-way direction - var p1 = feature.points[0]; - var p2 = feature.points[1]; + // Position error at the approximate middle of the segment + var points = feature.points; + var mid = points.length / 2; + var loc; - var dir_of_travel = cardinalDirection(relativeBearing(p1, p2)); + // Even number of points, find midpoint of the middle two + // Odd number of points, use position of very middle point + if (mid % 1 == 0) { + loc = pointAverage([points[mid - 1], points[mid]]); + } else { + mid = points[Math.floor(mid)]; + loc = [mid.lon, mid.lat]; + } var d = new iOsmError({ - loc: pointAverage(feature.points), // TODO: This isn't great for curved roads, would be better to find actual midpoint of segment + loc: loc, comments: null, error_subtype: '', error_type: k, @@ -205,7 +213,8 @@ export default { percentage: feature.percentOfTrips, num_trips: feature.numberOfTrips, highway: linkErrorObject(t('QA.keepRight.error_parts.highway')), - travel_direction: dir_of_travel + from_node: linkEntity('n' + feature.fromNodeId), + to_node: linkEntity('n' + feature.toNodeId) }; _erCache.data[d.id] = d; From d82757c392ba863807bfc96968d2ff8f4c13ceb0 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Wed, 30 Jan 2019 20:27:13 +0000 Subject: [PATCH 27/38] Include node in turn restriction description Clarifies some cases where geometry could be unclear with previous message. --- data/core.yaml | 2 +- dist/locales/en.json | 2 +- modules/services/improveOSM.js | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index 2bb4915b8..f61406ef3 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -686,7 +686,7 @@ en: description: '{num_trips} recorded trips in this area suggest there may be unmapped {geometry_type} here.' tr: title: Missing Turn Restriction - description: '{num_passed} of {num_trips} recorded trips (travelling {travel_direction}) make a turn from {from_way} to {to_way}. There may be a missing "{turn_restriction}" restriction.' + description: '{num_passed} of {num_trips} recorded trips (travelling {travel_direction}) make a turn from {from_way} to {to_way} at {junction}. There may be a missing "{turn_restriction}" restriction.' keepRight: title: KeepRight Error detail_title: Error diff --git a/dist/locales/en.json b/dist/locales/en.json index f4ef0c186..d3d73461f 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -832,7 +832,7 @@ }, "tr": { "title": "Missing Turn Restriction", - "description": "{num_passed} of {num_trips} recorded trips (travelling {travel_direction}) make a turn from {from_way} to {to_way}. There may be a missing \"{turn_restriction}\" restriction." + "description": "{num_passed} of {num_trips} recorded trips (travelling {travel_direction}) make a turn from {from_way} to {to_way} at {junction}. There may be a missing \"{turn_restriction}\" restriction." } } }, diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index 84164de6b..c253d79f7 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -292,7 +292,8 @@ export default { turn_restriction: feature.turnType.toLowerCase(), from_way: linkEntity('w' + from_way), to_way: linkEntity('w' + to_way), - travel_direction: dir_of_travel + travel_direction: dir_of_travel, + junction: linkErrorObject(t('QA.keepRight.error_parts.this_node')) }; _erCache.data[d.id] = d; From 183dda5999d62255217c2c6841cb8bdf85e25333 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Wed, 30 Jan 2019 21:51:50 +0000 Subject: [PATCH 28/38] Pacify eslint --- modules/services/improveOSM.js | 2 +- modules/ui/improveOSM_editor.js | 3 +-- modules/ui/improveOSM_header.js | 1 - 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index c253d79f7..4888249df 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -185,7 +185,7 @@ export default { // Even number of points, find midpoint of the middle two // Odd number of points, use position of very middle point - if (mid % 1 == 0) { + if (mid % 1 === 0) { loc = pointAverage([points[mid - 1], points[mid]]); } else { mid = points[Math.floor(mid)]; diff --git a/modules/ui/improveOSM_editor.js b/modules/ui/improveOSM_editor.js index d235b2101..c1c29c1b5 100644 --- a/modules/ui/improveOSM_editor.js +++ b/modules/ui/improveOSM_editor.js @@ -1,5 +1,4 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; -import { select as d3_select } from 'd3-selection'; import { t } from '../util/locale'; import { services } from '../services'; @@ -13,7 +12,7 @@ import { uiTooltipHtml } from './index'; -import { utilNoAuto, utilRebind } from '../util'; +import { utilRebind } from '../util'; export function uiImproveOsmEditor(context) { diff --git a/modules/ui/improveOSM_header.js b/modules/ui/improveOSM_header.js index d33835e23..aa393b22b 100644 --- a/modules/ui/improveOSM_header.js +++ b/modules/ui/improveOSM_header.js @@ -1,5 +1,4 @@ import { dataEn } from '../../data'; -import { svgIcon } from '../svg'; import { t } from '../util/locale'; From 21824e6377a77646c6c8662b808e46f979a00035 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Thu, 31 Jan 2019 10:59:12 +0000 Subject: [PATCH 29/38] Fix coincident errors Potential for multiple missing turn restrictions on one node and I've also seen a case of missing one-way along the same stretch of road in opposite directions! Missing geometry is tile based so can't really be coincident, but doesn't hurt to check in case they happen to land on a one-way or turn restriction. --- modules/services/improveOSM.js | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index 4888249df..8a30a1d8f 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -115,6 +115,20 @@ function cardinalDirection(bearing) { return t('QA.improveOSM.directions.' + compass[dir]); } +// Errors shouldn't obscure eachother +function preventCoincident(loc, bumpUp) { + var coincident = false; + do { + // first time, move marker up. after that, move marker right. + var delta = coincident ? [0.00001, 0] : (bumpUp ? [0, 0.00001] : [0, 0]); + loc = geoVecAdd(loc, delta); + var bbox = geoExtent(loc).bbox(); + coincident = _erCache.rtree.search(bbox).length; + } while (coincident); + + return loc; +} + export default { init: function() { if (!_erCache) { @@ -192,6 +206,9 @@ export default { loc = [mid.lon, mid.lat]; } + // One-ways can land on same segment in opposite direction + loc = preventCoincident(loc, false); + var d = new iOsmError({ loc: loc, comments: null, @@ -225,6 +242,13 @@ export default { // Tiles at high zoom == missing roads if (data.tiles) { data.tiles.forEach(function(feature) { + // Average of recorded points should land on the missing geometry + var loc = pointAverage(feature.points); + + // Missing geometry could happen to land on another error + loc = preventCoincident(loc, false); + + var geoType = feature.type.toLowerCase(); var geoIcons = { road: 'maki-car', @@ -234,7 +258,7 @@ export default { }; var d = new iOsmError({ - loc: pointAverage(feature.points), + loc: loc, comments: null, error_subtype: geoType, error_type: k, @@ -258,8 +282,9 @@ export default { data.entities.forEach(function(feature) { var loc = feature.point; - // Bump position slightly so junction node is accessible - loc = geoVecAdd([loc.lon, loc.lat], [0, 0.00001]); + // Turn restrictions could be missing at same junction + // We also want to bump the error up so node is accessible + loc = preventCoincident([loc.lon, loc.lat], true); // Elements are presented in a strange way var ids = feature.id.split(','); From 03a5c26e749d406c0fe21a72de94f92b9d9caf3c Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Fri, 1 Feb 2019 18:52:01 +0000 Subject: [PATCH 30/38] Refer to ImproveOSM QA as detections --- data/core.yaml | 2 +- dist/locales/en.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index f61406ef3..0cc322325 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -662,7 +662,7 @@ en: full_screen: Toggle Full Screen QA: improveOSM: - title: ImproveOSM Error + title: ImproveOSM Detection geometry_types: path: paths parking: parking diff --git a/dist/locales/en.json b/dist/locales/en.json index d3d73461f..7019717d9 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -804,7 +804,7 @@ "full_screen": "Toggle Full Screen", "QA": { "improveOSM": { - "title": "ImproveOSM Error", + "title": "ImproveOSM Detection", "geometry_types": { "path": "paths", "parking": "parking", From e9397aa14f868151d8299d46ee4bb27ab2a1d22f Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Fri, 1 Feb 2019 22:09:13 +0000 Subject: [PATCH 31/38] Fix incorrect usage of osm.userDetails callback Javascript is not my usual domain so still getting used to how scope works and working with callbacks. Believe this code is now written as intended. As a bonus a response status to the error update request which isn't the expected 200 now returns before visibly removing the error and adding it to the closed cache. --- modules/services/improveOSM.js | 64 ++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index 8a30a1d8f..a0811b6d2 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -342,49 +342,51 @@ export default { return callback({ message: 'Error update already inflight', status: -2 }, d); } - var osmUsername = services.osm.userDetails(function(err, user) { - if (err) return ''; - - return user.display_name; - }); - var that = this; - var type = d.error_type; - var payload = { - username: osmUsername - }; - // Each error type has different data for identification - if (type === 'ow') { - payload.roadSegments = [ d.identifier ]; - } else if (type === 'mr') { - payload.tiles = [ d.identifier ]; - } else if (type === 'tr') { - payload.targetIds = [ d.identifier ]; - } + // Payload can only be sent once username is established + services.osm.userDetails(sendPayload); - // Separate requests required to comment and change status - var url = _impOsmUrls[type] + '/comment'; + function sendPayload(err, user) { + if (err) { return callback(err, d); } - // Comments don't currently work - // if (d.newComment !== undefined) { - // payload.text = d.newComment; - // } + var type = d.error_type; + var url = _impOsmUrls[type] + '/comment'; + var payload = { + username: user.display_name + }; - if (d.newStatus !== d.status) { - payload.status = d.newStatus; - payload.text = 'status changed'; + // Each error type has different data for identification + if (type === 'ow') { + payload.roadSegments = [ d.identifier ]; + } else if (type === 'mr') { + payload.tiles = [ d.identifier ]; + } else if (type === 'tr') { + payload.targetIds = [ d.identifier ]; + } + + // Comments don't currently work, if they ever do in future + // it looks as though they require a separate post + // if (d.newComment !== undefined) { + // payload.text = d.newComment; + // } + + if (d.newStatus !== d.status) { + payload.status = d.newStatus; + payload.text = 'status changed'; + } _erCache.inflightPost[d.id] = d3_request(url) .header('Content-Type', 'application/json') .post(JSON.stringify(payload), function(err) { delete _erCache.inflightPost[d.id]; - if (d.newStatus === 'INVALID') { - that.removeError(d); - } else if (d.newStatus === 'SOLVED') { - that.removeError(d); + // Unsuccessful response status, keep issue open + if (err.status !== 200) { return callback(err, d); } + that.removeError(d); + + if (d.newStatus === 'SOLVED') { //TODO the identifiers are ugly and can't be used frontend, use error position instead? // or perhaps don't track this at all? //_erCache.closed[d.error_type + ':' + d.identifier] = true; From c9e249ee6c66867572ebce6f85a7161912b6490d Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Fri, 1 Feb 2019 23:07:26 +0000 Subject: [PATCH 32/38] Send only relevant query parameters --- modules/services/improveOSM.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index a0811b6d2..c84260eef 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -155,9 +155,7 @@ export default { loadErrors: function(projection) { var options = { client: 'iD', - confidenceLevel: 'C1', // most confident only, still have false positives status: 'OPEN', - type: 'PARKING,ROAD,BOTH,PATH', // exclude WATER as it doesn't seem useful zoom: '19' // Use a high zoom so that clusters aren't returned }; @@ -180,7 +178,10 @@ export default { var requests = {}; _forEach(_impOsmUrls, function(v, k) { - var url = v + '/search?' + utilQsString(params); + // We exclude WATER from missing geometry as it doesn't seem useful + // We use most confident one-way and turn restrictions only, still have false positives + var kParams = _extend({}, params, (k === 'mr') ? { type: 'PARKING,ROAD,BOTH,PATH' } : { confidenceLevel: 'C1' }); + var url = v + '/search?' + utilQsString(kParams); requests[k] = d3_json(url, function(err, data) { @@ -430,4 +431,4 @@ export default { delete _erCache.data[error.id]; updateRtree(encodeErrorRtree(error), false); // false = remove } -}; +}; \ No newline at end of file From 9403f71140b565cf62713429f9edbe993897bd87 Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Fri, 1 Feb 2019 23:29:08 +0000 Subject: [PATCH 33/38] Track closed ImproveOSM errors via coordinates --- modules/services/improveOSM.js | 13 +++++++++---- modules/ui/commit.js | 8 +++++++- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index c84260eef..e170f930d 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -215,7 +215,7 @@ export default { comments: null, error_subtype: '', error_type: k, - icon: 'fas-arrow-circle-up', //TODO: Change arrow based on direction? + icon: '', //TODO: Find suitable icon identifier: { // this is used to post changes to the error wayId: feature.wayId, fromNodeId: feature.fromNodeId, @@ -387,10 +387,10 @@ export default { that.removeError(d); + // No pretty identifier, so we just use coordinates if (d.newStatus === 'SOLVED') { - //TODO the identifiers are ugly and can't be used frontend, use error position instead? - // or perhaps don't track this at all? - //_erCache.closed[d.error_type + ':' + d.identifier] = true; + var closedID = d.loc[1].toFixed(5) + '/' + d.loc[0].toFixed(5); + _erCache.closed[d.error_type + ':' + closedID] = true; } return callback(err, d); @@ -430,5 +430,10 @@ export default { delete _erCache.data[error.id]; updateRtree(encodeErrorRtree(error), false); // false = remove + }, + + // Used to populate `closed:improveosm` changeset tag + getClosedIDs: function() { + return Object.keys(_erCache.closed).sort(); } }; \ No newline at end of file diff --git a/modules/ui/commit.js b/modules/ui/commit.js index 46201e30c..f1706fdc7 100644 --- a/modules/ui/commit.js +++ b/modules/ui/commit.js @@ -109,6 +109,12 @@ export function uiCommit(context) { tags['closed:keepright'] = krClosed.join(';').substr(0, 255); } } + if (services.improveOSM) { + var iOsmClosed = services.improveOSM.getClosedIDs(); + if (iOsmClosed.length) { + tags['closed:improveosm'] = iOsmClosed.join(';').substr(0, 255); + } + } _changeset = _changeset.update({ tags: tags }); @@ -461,4 +467,4 @@ export function uiCommit(context) { return utilRebind(commit, dispatch, 'on'); -} +} \ No newline at end of file From 6be72709d544b02d2c6aee8dd4d0e93b1738e0f0 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sat, 2 Feb 2019 11:51:37 -0500 Subject: [PATCH 34/38] Adjust tag_classes perimeter overrides to style barriers as lines (closes #5761) --- css/25_areas.css | 1 - css/50_misc.css | 6 +++++- modules/svg/tag_classes.js | 18 +++++++++++++----- test/spec/svg/tag_classes.js | 19 +++++++++++++++++++ 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/css/25_areas.css b/css/25_areas.css index ca964e54e..7e6d9f41a 100644 --- a/css/25_areas.css +++ b/css/25_areas.css @@ -31,7 +31,6 @@ path.stroke.tag-leisure-track, path.stroke.tag-leisure-golf_course, path.stroke.tag-leisure-garden, path.stroke.tag-leisure-park, -path.stroke.tag-barrier-hedge, path.stroke.tag-landuse-forest, path.stroke.tag-landuse-wood, path.stroke.tag-landuse-grass { diff --git a/css/50_misc.css b/css/50_misc.css index 0e3e726e2..a8dcd4c95 100644 --- a/css/50_misc.css +++ b/css/50_misc.css @@ -95,9 +95,13 @@ path.line.stroke.tag-natural-tree_row { /* barriers and similar */ -path.line.stroke.tag-barrier:not(.tag-barrier-hedge) { +path.line.stroke.tag-barrier { stroke: #ddd; } +path.line.stroke.tag-barrier-hedge { + stroke: rgb(140, 208, 95); +} + path.line.stroke.tag-barrier, path.line.stroke.tag-man_made-groyne, path.line.stroke.tag-man_made-breakwater { diff --git a/modules/svg/tag_classes.js b/modules/svg/tag_classes.js index 588fad521..8d4a195cd 100644 --- a/modules/svg/tag_classes.js +++ b/modules/svg/tag_classes.js @@ -30,21 +30,29 @@ export function svgTagClasses() { } var t = _tags(entity); - var isMultiPolygon = (t.type === 'multipolygon'); - var shouldRenderLineAsArea = isMultiPolygon && !entity.hasInterestingTags(); - var i, k, v; + // in some situations we want to render perimeter strokes a certain way + var overrideGeometry; + if (/\bstroke\b/.test(value)) { + if (!!t.barrier && t.barrier !== 'no') { + overrideGeometry = 'line'; + } else if (t.type === 'multipolygon' && !entity.hasInterestingTags()) { + overrideGeometry = 'area'; + } + } + // preserve base classes (nothing with `tag-`) var classes = value.trim().split(/\s+/) .filter(function(klass) { return klass.length && !/^tag-/.test(klass); }) - .map(function(klass) { // style multipolygon inner/outers as areas not lines - return (klass === 'line' && shouldRenderLineAsArea) ? 'area' : klass; + .map(function(klass) { // special overrides for some perimeter strokes + return (klass === 'line' || klass === 'area') ? (overrideGeometry || klass) : klass; }); + // pick at most one primary classification tag.. for (i = 0; i < primaries.length; i++) { k = primaries[i]; diff --git a/test/spec/svg/tag_classes.js b/test/spec/svg/tag_classes.js index 8baa177dd..2df7caeb1 100644 --- a/test/spec/svg/tag_classes.js +++ b/test/spec/svg/tag_classes.js @@ -209,6 +209,25 @@ describe('iD.svgTagClasses', function () { expect(selection.attr('class')).to.equal('selected'); }); + it('stroke overrides: renders areas with barriers as lines', function() { + selection + .attr('class', 'way area stroke') + .datum(iD.osmEntity({tags: {landuse: 'residential', barrier: 'hedge'}})) + .call(iD.svgTagClasses()); + expect(selection.classed('area')).to.be.false; + expect(selection.classed('line')).to.be.true; + }); + + it('stroke overrides: renders simple multipolygon lines as areas', function() { + var multipolygon = function () { return { type: 'multipolygon' }; }; + selection + .attr('class', 'way line stroke') + .datum(iD.osmEntity({tags: {}})) + .call(iD.svgTagClasses().tags(multipolygon)); + expect(selection.classed('area')).to.be.true; + expect(selection.classed('line')).to.be.false; + }); + it('works on SVG elements', function() { selection = d3.select(document.createElementNS('http://www.w3.org/2000/svg', 'g')); selection From 96a15934cac472a95380b969820130f5efb74d81 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sat, 2 Feb 2019 12:05:19 -0500 Subject: [PATCH 35/38] Don't snake_case values for `destination` and `destination:ref` fields (closes #5842) But continue to snake_case the values for `destination:symbol` --- data/presets/fields.json | 4 ++-- data/presets/fields/destination/ref_oneway.json | 3 ++- data/presets/fields/destination_oneway.json | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/data/presets/fields.json b/data/presets/fields.json index 4080e533d..c578d98a1 100644 --- a/data/presets/fields.json +++ b/data/presets/fields.json @@ -84,8 +84,8 @@ "denotation": {"key": "denotation", "type": "combo", "label": "Denotation"}, "description": {"key": "description", "type": "textarea", "label": "Description", "universal": true}, "design": {"key": "design", "type": "combo", "label": "Design"}, - "destination_oneway": {"key": "destination", "type": "semiCombo", "label": "Destinations", "prerequisiteTag": {"key": "oneway", "value": "yes"}}, - "destination/ref_oneway": {"key": "destination:ref", "type": "semiCombo", "label": "Destination Road Numbers", "prerequisiteTag": {"key": "oneway", "value": "yes"}}, + "destination_oneway": {"key": "destination", "type": "semiCombo", "label": "Destinations", "prerequisiteTag": {"key": "oneway", "value": "yes"}, "snake_case": false}, + "destination/ref_oneway": {"key": "destination:ref", "type": "semiCombo", "label": "Destination Road Numbers", "prerequisiteTag": {"key": "oneway", "value": "yes"}, "snake_case": false}, "destination/symbol_oneway": {"key": "destination:symbol", "type": "semiCombo", "label": "Destination Symbols", "prerequisiteTag": {"key": "oneway", "value": "yes"}}, "devices": {"key": "devices", "type": "number", "minValue": 0, "label": "Devices", "placeholder": "1, 2, 3..."}, "diaper": {"key": "diaper", "type": "combo", "label": "Diaper Changing Available", "options": ["yes", "no", "room", "1", "2", "3", "4", "5"]}, diff --git a/data/presets/fields/destination/ref_oneway.json b/data/presets/fields/destination/ref_oneway.json index fc360f211..71fe8dec1 100644 --- a/data/presets/fields/destination/ref_oneway.json +++ b/data/presets/fields/destination/ref_oneway.json @@ -5,5 +5,6 @@ "prerequisiteTag": { "key": "oneway", "value": "yes" - } + }, + "snake_case": false } diff --git a/data/presets/fields/destination_oneway.json b/data/presets/fields/destination_oneway.json index 2f71c079b..1d5bc9db8 100644 --- a/data/presets/fields/destination_oneway.json +++ b/data/presets/fields/destination_oneway.json @@ -5,5 +5,6 @@ "prerequisiteTag": { "key": "oneway", "value": "yes" - } + }, + "snake_case": false } From f1b5efc81a77701475a1a620d18318bbc4e22612 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sat, 2 Feb 2019 15:30:42 -0500 Subject: [PATCH 36/38] Revise the list of oneway aerialways (notably, drop `yes`) (re: #5843) --- modules/osm/tags.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/modules/osm/tags.js b/modules/osm/tags.js index 3b7824ec4..0e08dfa19 100644 --- a/modules/osm/tags.js +++ b/modules/osm/tags.js @@ -10,13 +10,14 @@ export function osmIsInterestingTag(key) { export var osmOneWayTags = { 'aerialway': { 'chair_lift': true, - 'mixed_lift': true, - 't-bar': true, + 'drag_lift': true, 'j-bar': true, + 'magic_carpet': true, + 'mixed_lift': true, 'platter': true, 'rope_tow': true, - 'magic_carpet': true, - 'yes': true + 't-bar': true, + 'zipline': true }, 'highway': { 'motorway': true From ebdf6431f73f159759886cf1cf58441c7b04abc7 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sat, 2 Feb 2019 15:32:36 -0500 Subject: [PATCH 37/38] Support classing of aerialway and piste, and adjust line widths (closes #5843) --- css/50_misc.css | 118 ++++++++++++++++++++++--------------- modules/svg/tag_classes.js | 8 ++- 2 files changed, 77 insertions(+), 49 deletions(-) diff --git a/css/50_misc.css b/css/50_misc.css index a8dcd4c95..265d641bb 100644 --- a/css/50_misc.css +++ b/css/50_misc.css @@ -1,4 +1,56 @@ +/* narrow width miscellanous things */ +path.line.shadow.tag-aerialway, +path.line.shadow.tag-attraction-summer_toboggan, +path.line.shadow.tag-attraction-water_slide, +path.line.shadow.tag-man_made-pipeline, +path.line.shadow.tag-natural-tree_row, +path.line.shadow.tag-piste { + stroke-width: 16; +} +path.line.casing.tag-aerialway, +path.line.casing.tag-attraction-summer_toboggan, +path.line.casing.tag-attraction-water_slide, +path.line.casing.tag-man_made-pipeline, +path.line.casing.tag-natural-tree_row, +path.line.casing.tag-piste { + stroke-width: 7; +} +path.line.stroke.tag-aerialway, +path.line.stroke.tag-attraction-summer_toboggan, +path.line.stroke.tag-attraction-water_slide, +path.line.stroke.tag-man_made-pipeline, +path.line.stroke.tag-natural-tree_row, +path.line.stroke.tag-piste { + stroke-width: 5; +} + +.low-zoom path.line.shadow.tag-aerialway, +.low-zoom path.line.shadow.tag-attraction-summer_toboggan, +.low-zoom path.line.shadow.tag-attraction-water_slide, +.low-zoom path.line.shadow.tag-man_made-pipeline, +.low-zoom path.line.shadow.tag-natural-tree_row, +.low-zoom path.line.shadow.tag-piste { + stroke-width: 12; +} +.low-zoom path.line.casing.tag-aerialway, +.low-zoom path.line.casing.tag-attraction-summer_toboggan, +.low-zoom path.line.casing.tag-attraction-water_slide, +.low-zoom path.line.casing.tag-man_made-pipeline, +.low-zoom path.line.casing.tag-natural-tree_row, +.low-zoom path.line.casing.tag-piste { + stroke-width: 5; +} +.low-zoom path.line.stroke.tag-aerialway, +.low-zoom path.line.stroke.tag-attraction-summer_toboggan, +.low-zoom path.line.stroke.tag-attraction-water_slide, +.low-zoom path.line.stroke.tag-man_made-pipeline, +.low-zoom path.line.stroke.tag-natural-tree_row, +.low-zoom path.line.stroke.tag-piste { + stroke-width: 3; +} + + /* ferry routes */ .preset-icon .icon.tag-route-ferry { color: #58a9ed; @@ -24,6 +76,22 @@ path.line.stroke.tag-route-ferry { } +/* aerialways */ +path.line.stroke.tag-aerialway { + stroke: #c55; +} +path.line.casing.tag-aerialway { + stroke: #444; +} + +/* pistes */ +path.line.stroke.tag-piste { + stroke: #9ac; +} +path.line.casing.tag-piste { + stroke: #444; +} + /* power and pipeline */ .preset-icon .icon.tag-man_made-pipeline, .preset-icon .icon.tag-power { @@ -31,6 +99,7 @@ path.line.stroke.tag-route-ferry { fill: #939393; } + /* power */ path.line.stroke.tag-power { stroke: #939393; @@ -40,21 +109,21 @@ path.line.casing.tag-power { stroke: none; } + /* pipeline */ path.line.stroke.tag-man_made-pipeline { stroke: #cbd0d8; stroke-linecap: butt; - stroke-width: 3; stroke-dasharray: 80, 1.25; } path.line.casing.tag-man_made-pipeline { stroke: #666; - stroke-width: 4.5; } .low-zoom path.line.stroke.tag-man_made-pipeline { stroke-dasharray: 40, 1; } + /* boundaries */ path.line.stroke.tag-boundary { stroke: #fff; @@ -73,27 +142,6 @@ path.line.casing.tag-boundary-national_park { } -/* Tree Rows */ -path.line.shadow.tag-natural-tree_row { - stroke-width: 16; -} -path.line.casing.tag-natural-tree_row { - stroke-width: 7; -} -path.line.stroke.tag-natural-tree_row { - stroke-width: 5; -} -.low-zoom path.line.shadow.tag-natural-tree_row { - stroke-width: 12; -} -.low-zoom path.line.casing.tag-natural-tree_row { - stroke-width: 5; -} -.low-zoom path.line.stroke.tag-natural-tree_row { - stroke-width: 3; -} - - /* barriers and similar */ path.line.stroke.tag-barrier { stroke: #ddd; @@ -416,30 +464,6 @@ path.line.stroke.tag-crossing.tag-crossing-zebra { } /* Attractions */ -path.line.shadow.tag-attraction-summer_toboggan, -path.line.shadow.tag-attraction-water_slide { - stroke-width: 16; -} -path.line.casing.tag-attraction-summer_toboggan, -path.line.casing.tag-attraction-water_slide { - stroke-width: 7; -} -path.line.stroke.tag-attraction-summer_toboggan, -path.line.stroke.tag-attraction-water_slide { - stroke-width: 5; -} -.low-zoom path.line.shadow.tag-attraction-summer_toboggan, -.low-zoom path.line.shadow.tag-attraction-water_slide { - stroke-width: 12; -} -.low-zoom path.line.casing.tag-attraction-summer_toboggan, -.low-zoom path.line.casing.tag-attraction-water_slide { - stroke-width: 5; -} -.low-zoom path.line.stroke.tag-attraction-summer_toboggan, -.low-zoom path.line.stroke.tag-attraction-water_slide { - stroke-width: 3; -} path.line.stroke.tag-attraction-summer_toboggan { stroke: #9e9e9e; } diff --git a/modules/svg/tag_classes.js b/modules/svg/tag_classes.js index 8d4a195cd..0e0a08479 100644 --- a/modules/svg/tag_classes.js +++ b/modules/svg/tag_classes.js @@ -4,8 +4,8 @@ import { osmPavedTags } from '../osm/tags'; export function svgTagClasses() { var primaries = [ - 'building', 'highway', 'railway', 'waterway', 'aeroway', - 'motorway', 'boundary', 'power', 'amenity', 'natural', 'landuse', + 'building', 'highway', 'railway', 'waterway', 'aeroway', 'aerialway', + 'piste:type', 'boundary', 'power', 'amenity', 'natural', 'landuse', 'leisure', 'military', 'place', 'man_made', 'route', 'attraction' ]; var statuses = [ @@ -59,6 +59,10 @@ export function svgTagClasses() { v = t[k]; if (!v || v === 'no') continue; + if (k === 'piste:type') { // avoid a ':' in the class name + k = 'piste'; + } + primary = k; if (statuses.indexOf(v) !== -1) { // e.g. `railway=abandoned` status = v; From e4e0d9d13ad0e4ea2349098a55188c87851c5e0d Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sun, 3 Feb 2019 23:55:16 -0500 Subject: [PATCH 38/38] Add oneway icon for ImproveOSM oneway markers --- build_data.js | 4 +++- modules/services/improveOSM.js | 4 ++-- svg/fontawesome/fas-long-arrow-alt-right.svg | 1 + 3 files changed, 6 insertions(+), 3 deletions(-) create mode 100644 svg/fontawesome/fas-long-arrow-alt-right.svg diff --git a/build_data.js b/build_data.js index deb949b89..2d72591d8 100644 --- a/build_data.js +++ b/build_data.js @@ -59,7 +59,9 @@ module.exports = function buildData() { }; // Font Awesome icons used - var faIcons = {}; + var faIcons = { + 'fas-long-arrow-alt-right': {} + }; // Start clean shell.rm('-f', [ diff --git a/modules/services/improveOSM.js b/modules/services/improveOSM.js index e170f930d..2031c4720 100644 --- a/modules/services/improveOSM.js +++ b/modules/services/improveOSM.js @@ -215,7 +215,7 @@ export default { comments: null, error_subtype: '', error_type: k, - icon: '', //TODO: Find suitable icon + icon: 'fas-long-arrow-alt-right', identifier: { // this is used to post changes to the error wayId: feature.wayId, fromNodeId: feature.fromNodeId, @@ -436,4 +436,4 @@ export default { getClosedIDs: function() { return Object.keys(_erCache.closed).sort(); } -}; \ No newline at end of file +}; diff --git a/svg/fontawesome/fas-long-arrow-alt-right.svg b/svg/fontawesome/fas-long-arrow-alt-right.svg new file mode 100644 index 000000000..a2b44d34b --- /dev/null +++ b/svg/fontawesome/fas-long-arrow-alt-right.svg @@ -0,0 +1 @@ + \ No newline at end of file