diff --git a/css/80_app.css b/css/80_app.css index b5b29af13..784db9f46 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -2753,6 +2753,10 @@ input.key-trap { [dir='rtl'] .error-details-description-text::first-letter { text-transform: none; /* #5877 */ } +.error-details-description +.error-details-subtitle { + padding-top: 10px; +} .note-save .new-comment-input, .error-save .new-comment-input { @@ -5590,4 +5594,4 @@ li.hide + li.version .badge .tooltip .popover-arrow { } [dir='rtl'] .list-item-photos.list-item-mapillary-map-features .request-data-link { float: left; -} +} \ No newline at end of file diff --git a/data/core.yaml b/data/core.yaml index df409495b..98d36b3cc 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -829,94 +829,12 @@ en: QA: osmose: title: Osmose Issue - error_types: - 0: - description: '{0} and {1} intersect.' - 0-3: - description: '{0} is a suspisciously small building.' - 0-4: - description: 'A gap exists between {0} and {1} which looks suspiciously small.' - 1040-1: - description: '{0} intersects itself.' - 1050-1: - description: '{0} looks like a roundabout mapped in the wrong direction for this location.' - 1050-1050: - description: '{0} looks like a mini-roundabout with the incorrect "direction" tag for this location.' - 1070: - description: '{1} intersects with {0}, but there is no junction node, bridge, or tunnel.' - 1150: - description: '{0} and {1} overlap.' - 1190: - description: '{0} looks like it could be mapped more accurately.' - 1280-1: - description: '{0} should be on the highway or part of an "enforcement" relation.' - 2110-21101: - description: '{0} has a name but no feature tag.' - 2110-21102: - description: '{0} is missing a "type" tag.' - 3040-3040: - description: '{0} has a non-conventional tag value: "{1}".' - 3090-3090: - description: '{0} is tagged with an invalid date value: "{1}".' - 3161: - description: 'There is no highway leading to {0}.' - 3200-32001: - description: '{0} is tagged "area=yes", but this feature is an area by default.' - 3200-32002: - description: '{0} has an "area" tag, but no feature tag.' - 3200-32003: - description: '{0} is tagged "area=no", but this feature already isn''t an area.' - 3220: - description: '{0} has an "access=yes" or "access=permissive" tag which permits use by all transport modes (ski, horse, hazmat, etc.).' - 3250-32501: - description: '{0} has an invalid value for the "opening_hours" tag.' - 4010: - description: '{0} uses tag "{1}" which is deprecated in favour of "{2}".' - 4030-900: - description: '{0} is tagged with both "{1}" and "{2}".' - 4080: - description: '{0} and {1} both appear to represent the same object.' - 5010-803: - description: 'The "name" of {0} is written in all capitals.' - 5010-903: - description: 'The "name" of {0} has excessive space characters.' - 5070-50703: - description: '{0} has an unexpected symbol, {2}, in the "{1}" tag.' - 5070-50704: - description: '{0} has an unbalanced {1} character in its name.' - 5070-50705: - description: '{0} has an unexpected {1} character in its name.' - 7040: - description: 'A power line appears to be unfinished at {0}.' - 7040-1: - description: '{0} is a power tower or pole with no connected power lines.' - 7040-4: - description: 'Power lines should only have nodes at supports and {0} is not tagged as a pole or tower.' - 7090-1: - description: '{0} is a level crossing at a non-junction node.' - 7090-3: - description: '{0} is highway feature or barrier with no connected highway.' - 8300: - description: 'Traffic signs detected by Mapillary suggest there may be an unmapped {0} nearby.' - parts: - 1: 'speed limit' - 20: 'max height limit' - 21: 'max weight limit' - 32: 'roundabout' - 34: 'set of speed bumps' - 39: 'living street' - 8360: - description: 'Object detection by Mapillary suggests there may be an unmapped {0} nearby.' - parts: - 1: 'bench' - 2: 'type of bicycle parking' - 3: 'surveillance camera' - 4: 'fire hydrant' - 5: 'set of traffic signals' - 9010-9010001: - description: '{0} is redundantly tagged with "{1}".' - 9010-9010003: - description: '{0} appears to have a descriptive name: "{1}".' + elems_title: Issue Elements + details: + values: Issue Values + tags: Issue Tags + chars: Issue Characters + sug_tags: Suggested Tags improveOSM: title: ImproveOSM Detection geometry_types: diff --git a/dist/locales/en.json b/dist/locales/en.json index 98dda7275..819618a99 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1033,133 +1033,12 @@ "QA": { "osmose": { "title": "Osmose Issue", - "error_types": { - "0": { - "description": "{0} and {1} intersect." - }, - "1070": { - "description": "{1} intersects with {0}, but there is no junction node, bridge, or tunnel." - }, - "1150": { - "description": "{0} and {1} overlap." - }, - "1190": { - "description": "{0} looks like it could be mapped more accurately." - }, - "3161": { - "description": "There is no highway leading to {0}." - }, - "3220": { - "description": "{0} has an \"access=yes\" or \"access=permissive\" tag which permits use by all transport modes (ski, horse, hazmat, etc.)." - }, - "4010": { - "description": "{0} uses tag \"{1}\" which is deprecated in favour of \"{2}\"." - }, - "4080": { - "description": "{0} and {1} both appear to represent the same object." - }, - "7040": { - "description": "A power line appears to be unfinished at {0}." - }, - "8300": { - "description": "Traffic signs detected by Mapillary suggest there may be an unmapped {0} nearby.", - "parts": { - "1": "speed limit", - "20": "max height limit", - "21": "max weight limit", - "32": "roundabout", - "34": "set of speed bumps", - "39": "living street" - } - }, - "8360": { - "description": "Object detection by Mapillary suggests there may be an unmapped {0} nearby.", - "parts": { - "1": "bench", - "2": "type of bicycle parking", - "3": "surveillance camera", - "4": "fire hydrant", - "5": "set of traffic signals" - } - }, - "0-3": { - "description": "{0} is a suspisciously small building." - }, - "0-4": { - "description": "A gap exists between {0} and {1} which looks suspiciously small." - }, - "1040-1": { - "description": "{0} intersects itself." - }, - "1050-1": { - "description": "{0} looks like a roundabout mapped in the wrong direction for this location." - }, - "1050-1050": { - "description": "{0} looks like a mini-roundabout with the incorrect \"direction\" tag for this location." - }, - "1280-1": { - "description": "{0} should be on the highway or part of an \"enforcement\" relation." - }, - "2110-21101": { - "description": "{0} has a name but no feature tag." - }, - "2110-21102": { - "description": "{0} is missing a \"type\" tag." - }, - "3040-3040": { - "description": "{0} has a non-conventional tag value: \"{1}\"." - }, - "3090-3090": { - "description": "{0} is tagged with an invalid date value: \"{1}\"." - }, - "3200-32001": { - "description": "{0} is tagged \"area=yes\", but this feature is an area by default." - }, - "3200-32002": { - "description": "{0} has an \"area\" tag, but no feature tag." - }, - "3200-32003": { - "description": "{0} is tagged \"area=no\", but this feature already isn't an area." - }, - "3250-32501": { - "description": "{0} has an invalid value for the \"opening_hours\" tag." - }, - "4030-900": { - "description": "{0} is tagged with both \"{1}\" and \"{2}\"." - }, - "5010-803": { - "description": "The \"name\" of {0} is written in all capitals." - }, - "5010-903": { - "description": "The \"name\" of {0} has excessive space characters." - }, - "5070-50703": { - "description": "{0} has an unexpected symbol, {2}, in the \"{1}\" tag." - }, - "5070-50704": { - "description": "{0} has an unbalanced {1} character in its name." - }, - "5070-50705": { - "description": "{0} has an unexpected {1} character in its name." - }, - "7040-1": { - "description": "{0} is a power tower or pole with no connected power lines." - }, - "7040-4": { - "description": "Power lines should only have nodes at supports and {0} is not tagged as a pole or tower." - }, - "7090-1": { - "description": "{0} is a level crossing at a non-junction node." - }, - "7090-3": { - "description": "{0} is highway feature or barrier with no connected highway." - }, - "9010-9010001": { - "description": "{0} is redundantly tagged with \"{1}\"." - }, - "9010-9010003": { - "description": "{0} appears to have a descriptive name: \"{1}\"." - } + "elems_title": "Issue Elements", + "details": { + "values": "Issue Values", + "tags": "Issue Tags", + "chars": "Issue Characters", + "sug_tags": "Suggested Tags" } }, "improveOSM": { diff --git a/modules/services/osmose.js b/modules/services/osmose.js index abc9ed70c..f58d94097 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -3,7 +3,7 @@ import RBush from 'rbush'; import { dispatch as d3_dispatch } from 'd3-dispatch'; import { json as d3_json } from 'd3-fetch'; -import { currentLocale, t } from '../util/locale'; +import { currentLocale } from '../util/locale'; import { geoExtent, geoVecAdd } from '../geo'; import { qaError } from '../osm'; import { utilRebind, utilTiler, utilQsString } from '../util'; @@ -47,10 +47,6 @@ function updateRtree(item, replace) { } } -function linkEntity(d) { - return `${d}`; -} - // Errors shouldn't obscure eachother function preventCoincident(loc) { let coincident = false; @@ -139,22 +135,9 @@ export default { item // category of the issue for styling }); - // Special handling for some error types - switch (d.item) { - case 8300: - case 8360: { - let k = error_class; - - // First 17 classes are all speed limits - if (item === 8300 && error_class <= 17) { - k = 1; - } - - // Setting elems here prevents UI error detail requests - d.replacements = [t(`QA.osmose.error_types.${d.item}.parts.${k}`)]; - d.elems = []; - break; - } + // Setting elems here prevents UI error detail requests + if (d.item === 8300 || d.item === 8360) { + d.elems = []; } _erCache.data[d.id] = d; @@ -187,35 +170,50 @@ export default { // Assign directly for immediate use in the callback d.elems = data.elems.map(e => e.type.substring(0,1) + e.id); - // Element links used in the error description - d.replacements = d.elems.map(linkEntity); - // Some error types have details in their subtitle const special = { - '3040-3040': /Bad value for (.+)/i, - '3090-3090': /Incorrect date "(.+)"/i, - '4010-4010': /Tag (.+) is deprecated: (.+)/i, - '4010-40102': /Tag (.+) is deprecated: (.+)/i, - '4030-900': /Conflict between tags: (.+), (.+)/i, - '5070-50703': /"(.+)"=".+" unexpected symbol char \(.+, (.+)\)/i, - '5070-50704': /Umbalanced (.+)/i, - '5070-50705': /Unexpected char (.+)/i, - '9010-9010003': /(.+)/ + tags: { + '3040-3040': /Bad tag value: "(.+)"/i, + '4010-4010': /Tag (.+) is deprecated/i, + '4010-40102': /Tag (.+) is deprecated/i, + '4030-900': /Conflict between tags: (.+), (.+)/i, + '5070-50703': /"(.+)"=".+" unexpected/i, + '9010-9010001': /(.+) is unnecessary/i + }, + values: { + '3090-3090': /Incorrect date "(.+)"/i, + '9010-9010003': /(.+)/ + }, + chars: { + '5070-50703': /unexpected symbol char \(.+, (.+)\)/i, + '5070-50704': /Umbalanced (.+)/i, + '5070-50705': /Unexpected char (.+)/i + }, + sug_tags: { + '4010-4010': /Tag .+ is deprecated: (.+)/i, + '4010-40102': /Tag .+ is deprecated: (.+)/i, + } }; - if (d.error_type in special) { - let [, ...details] = special[d.error_type].exec(data.subtitle); - d.replacements.push(...details); + for (let type in special) { + if (d.error_type in special[type]) { + // Destructuring doesn't work if no match returns null, hence [] + let [, ...details] = special[type][d.error_type].exec(data.subtitle) || []; - if (d.error_type === '5070-50703') { - d.replacements[2] = String.fromCharCode(details[1]); + if ( + d.error_type === '5070-50703' + && type === 'chars' + && details + ) { + details[0] = String.fromCharCode(details[0]); + } + + // This error has a rare subtitle variant + if (d.error_type === '9010-9010001' && !details) { + [, ...details] = /\. Remove (.+)/i.exec(data.subtitle) || []; + } + + if (details) d[type] = details; } - } else if (d.error_type === '9010-9010001') { - // This error has a rare subtitle variant - let details = /(.+) is unnecessary/i.exec(data.subtitle); - if (details == null) { - details = /\. Remove (.+)/i.exec(data.subtitle); - } - d.replacements.push(details[1]); } this.replaceError(d); @@ -235,7 +233,7 @@ export default { const langs = { [locale]: true }; // Need English strings if not already fetched for fallback values - if (locale != 'en' && !('en' in _stringCache)) { + if (locale !== 'en' && !('en' in _stringCache)) { langs.en = true; } @@ -284,7 +282,7 @@ export default { getStrings(issueType, locale=currentLocale) { const l = (locale in _stringCache) ? _stringCache[locale][issueType] : {}; - const en = ('en' in _stringCache) ? _stringCache['en'][issueType] : {}; + const en = ('en' in _stringCache) ? _stringCache.en[issueType] : {}; // Fallback to English if string is untranslated return Object.assign({}, en, l); diff --git a/modules/svg/osmose.js b/modules/svg/osmose.js index 70b3c1aa1..409dfa9d3 100644 --- a/modules/svg/osmose.js +++ b/modules/svg/osmose.js @@ -64,6 +64,8 @@ export function svgOsmose(projection, context, dispatch) { // Enable the layer. This shows the errors and transitions them to visible. function layerOn() { // Strings supplied by Osmose fetched before showing layer for first time + // TODO: If layer is toggled quickly multiple requests are sent + // TODO: No error handling in place getService().loadStrings(editOn); drawLayer diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js index 756a5dd5a..2f876f00a 100644 --- a/modules/ui/osmose_details.js +++ b/modules/ui/osmose_details.js @@ -3,7 +3,6 @@ import { select as d3_select } from 'd3-selection'; -import { dataEn } from '../../data'; import { modeSelect } from '../modes/select'; import { t } from '../util/locale'; import { services } from '../services'; @@ -14,23 +13,14 @@ export function uiOsmoseDetails(context) { var _error; - function errorDetail(d) { + function issueDetail(d) { var unknown = t('inspector.unknown'); if (!d) return unknown; - // Some errors fall back to their category for strings - var i, et; - var keys = [d.error_type, d.item]; - for (i = 0; i < 2; i++) { - et = dataEn.QA.osmose.error_types[keys[i]]; - - if (et && et.description) { - return t('QA.osmose.error_types.' + keys[i] + '.description', d.replacements); - } - } - - return unknown; + // Issue description supplied by Osmose + var s = services.osmose.getStrings(d.error_type); + return ('description' in s) ? s.description : unknown; } @@ -58,16 +48,30 @@ export function uiOsmoseDetails(context) { .append('h4') .text(function() { return t('QA.keepRight.detail_description'); }); + descriptionEnter + .append('div') + .attr('class', 'error-details-description-text') + .html(issueDetail); + + descriptionEnter + .append('h4') + .attr('class', 'error-details-subtitle') + .text(function() { return t('QA.osmose.elems_title'); }); + + var elementList = descriptionEnter + .append('ul') + .attr('class', 'error-details-elements'); + services.osmose.loadErrorDetail(_error, function(err, d) { - if (d.elems === undefined) { return; } + if (d.elems === undefined) return; - descriptionEnter - .append('div') - .attr('class', 'error-details-description-text') - .html(errorDetail); - - // If there are entity links in the error message.. - descriptionEnter.selectAll('.error_entity_link, .error_object_link') + elementList.selectAll('.error_entity_link') + .data(d.elems) + .enter() + .append('li') + .append('a') + .attr('class', 'error_entity_link') + .text(function(d) { return d; }) .each(function() { var link = d3_select(this); var entityID = this.textContent; @@ -117,6 +121,26 @@ export function uiOsmoseDetails(context) { } }); + // Things like keys and values are dynamic details + const special = { tags: true, values: true, chars: true, sug_tags: true }; + for (let type in special) { + if (type in d) { + descriptionEnter + .append('h4') + .attr('class', 'error-details-subtitle') + .text(function() { return t(`QA.osmose.details.${type}`); }); + + descriptionEnter + .append('ul') + .attr('class', 'error-details-list') + .selectAll('li') + .data(d[type]) + .enter() + .append('li') + .html(function(d) { return d; }); + } + } + // Don't hide entities related to this error - #5880 context.features().forceVisible(d.elems); context.map().pan([0,0]); // trigger a redraw diff --git a/modules/ui/osmose_header.js b/modules/ui/osmose_header.js index 36ce763b9..c7049591e 100644 --- a/modules/ui/osmose_header.js +++ b/modules/ui/osmose_header.js @@ -1,5 +1,5 @@ import { services } from '../services'; -import { currentLocale, t } from '../util/locale'; +import { t } from '../util/locale'; export function uiOsmoseHeader() { @@ -13,11 +13,7 @@ export function uiOsmoseHeader() { // Issue titles supplied by Osmose var s = services.osmose.getStrings(d.error_type); - if ('title' in s) { - return s.title; - } - - return unknown; + return ('title' in s) ? s.title : unknown; }