Improve links in KeepRight error messages

(re: #5679) Also move the localizeable string dictionary to data/keepRight.json
2026-02-13 01:02:58 +00:00 · 2019-01-07 16:10:01 -05:00
parent 58665d0d2e
commit d9015baa47
4 changed files with 167 additions and 118 deletions
--- a/data/keepRight.json
+++ b/data/keepRight.json
@@ -1,4 +1,35 @@
 {
+  "localizeStrings": {
+    "node": "node",
+    "way": "way",
+    "relation": "relation",
+    "highway": "highway",
+    "railway": "railway",
+    "waterway": "waterway",
+    "cycleway": "cycleway",
+    "footpath": "footpath",
+    "'cycleway/footpath": "cycleway_footpath",
+    "riverbank": "riverbank",
+    "bridge": "bridge",
+    "tunnel": "tunnel",
+    "place_of_worship": "place_of_worship",
+    "pub": "pub",
+    "restaurant": "restaurant",
+    "school": "school",
+    "university": "university",
+    "hospital": "hospital",
+    "library": "library",
+    "theatre": "theatre",
+    "courthouse": "courthouse",
+    "bank": "bank",
+    "cinema": "cinema",
+    "pharmacy": "pharmacy",
+    "cafe": "cafe",
+    "fast_food": "fast_food",
+    "fuel": "fuel",
+    "from": "from",
+    "to": "to"
+  },
  "errorTypes": {
    "20": {
      "title": "multiple nodes on the same spot",
--- a/modules/services/keepRight.js
+++ b/modules/services/keepRight.js
@@ -13,7 +13,7 @@ import { krError } from '../osm';
 import { t } from '../util/locale';
 import { utilRebind, utilTiler, utilQsString } from '../util';

-import { errorTypes } from '../../data/keepRight.json';
+import { errorTypes, localizeStrings } from '../../data/keepRight.json';


 var tiler = utilTiler();
@@ -22,37 +22,6 @@ var dispatch = d3_dispatch('loaded');
 var _krCache;
 var _krZoom = 14;
 var _krUrlRoot = 'https://www.keepright.at/';
-var _krLocalize = {
-    node: 'node',
-    way: 'way',
-    relation: 'relation',
-    highway: 'highway',
-    railway: 'railway',
-    waterway: 'waterway',
-    cycleway: 'cycleway',
-    footpath: 'footpath',
-    'cycleway/footpath': 'cycleway_footpath',
-    riverbank: 'riverbank',
-    bridge: 'bridge',
-    tunnel: 'tunnel',
-    place_of_worship: 'place_of_worship',
-    pub: 'pub',
-    restaurant: 'restaurant',
-    school: 'school',
-    university: 'university',
-    hospital: 'hospital',
-    library: 'library',
-    theatre: 'theatre',
-    courthouse: 'courthouse',
-    bank: 'bank',
-    cinema: 'cinema',
-    pharmacy: 'pharmacy',
-    cafe: 'cafe',
-    fast_food: 'fast_food',
-    fuel: 'fuel',
-    from: 'from',
-    to: 'to'
-};

 var _krRuleset = [
    // no 20 - multiple node on same spot - these are mostly boundaries overlapping roads
@@ -119,7 +88,7 @@ function tokenReplacements(d) {
    // some descriptions are just fixed text
    if (!errorTemplate.regex) return;

-    // regex pattern should match description with variable details captured as groups
+    // regex pattern should match description with variable details captured
    var errorRegex = new RegExp(errorTemplate.regex, 'i');
    var errorMatch = errorRegex.exec(d.description);
    if (!errorMatch) {
@@ -132,33 +101,62 @@ function tokenReplacements(d) {
    }

    for (var i = 1; i < errorMatch.length; i++) {   // skip first
-        var group = errorMatch[i];
+        var capture = errorMatch[i];
        var idType;

        idType = 'IDs' in errorTemplate ? errorTemplate.IDs[i-1] : '';
-        if (idType && group) {   // link IDs if present in the group
-            group = parseError(group, idType);
-        } else if (htmlRegex.test(group)) {   // escape any html in non-IDs
-            group = '\\' +  group + '\\';
-        } else if (_krLocalize[group]) {   // some replacement strings can be localized
-            group = t('QA.keepRight.error_parts.' + _krLocalize[group]);
+        if (idType && capture) {   // link IDs if present in the capture
+            capture = parseError(capture, idType);
+        } else if (htmlRegex.test(capture)) {   // escape any html in non-IDs
+            capture = '\\' +  capture + '\\';
+        } else if (localizeStrings[capture]) {  // some replacement strings can be localized
+            capture = t('QA.keepRight.error_parts.' + localizeStrings[capture]);
        }

-        replacements['var' + i] = group;
+        replacements['var' + i] = capture;
    }

    return replacements;
 }


-function parseError(group, idType) {
+function parseError(capture, idType) {
+
+    switch (idType) {
+        // simple case just needs a linking span
+        case 'n':
+        case 'w':
+        case 'r':
+            capture = linkEntity(idType + capture);
+            break;
+
+        // some errors have more complex ID lists/variance
+        case '20':
+            capture = parse20(capture);
+            break;
+        case '211':
+            capture = parse211(capture);
+            break;
+        case '231':
+            capture = parse231(capture);
+            break;
+        case '294':
+            capture = parse294(capture);
+            break;
+        case '370':
+            capture = parse370(capture);
+            break;
+    }
+
+    return capture;
+

    function linkEntity(d) {
-        return '<span><a class="kr_error_description-id">' + d + '</a></span>';
+        return '<a class="kr_error_entity_link">' + d + '</a>';
    }

    // arbitrary node list of form: #ID, #ID, #ID...
-    function parseError211(capture) {
+    function parse211(capture) {
        var newList = [];
        var items = capture.split(', ');

@@ -172,7 +170,7 @@ function parseError(group, idType) {
    }

    // arbitrary way list of form: #ID(layer),#ID(layer),#ID(layer)...
-    function parseError231(capture) {
+    function parse231(capture) {
        var newList = [];
        // unfortunately 'layer' can itself contain commas, so we split on '),'
        var items = capture.split('),');
@@ -190,7 +188,7 @@ function parseError(group, idType) {
    }

    // arbitrary node/relation list of form: from node #ID,to relation #ID,to node #ID...
-    function parseError294(capture) {
+    function parse294(capture) {
        var newList = [];
        var items = capture.split(',');

@@ -220,7 +218,7 @@ function parseError(group, idType) {
    }

    // may or may not include the string "(including the name 'name')"
-    function parseError370(capture) {
+    function parse370(capture) {
        if (!capture) return '';

        var match = capture.match(/\(including the name (\'.+\')\)/);
@@ -231,7 +229,7 @@ function parseError(group, idType) {
    }

    // arbitrary node list of form: #ID,#ID,#ID...
-    function parseWarning20(capture) {
+    function parse20(capture) {
        var newList = [];
        var items = capture.split(',');

@@ -243,32 +241,6 @@ function parseError(group, idType) {

        return newList.join(', ');
    }
-
-    switch (idType) {
-        // simple case just needs a linking span
-        case 'n':
-        case 'w':
-        case 'r':
-            group = linkEntity(idType + group);
-            break;
-        // some errors have more complex ID lists/variance
-        case '211':
-            group = parseError211(group);
-            break;
-        case '231':
-            group = parseError231(group);
-            break;
-        case '294':
-            group = parseError294(group);
-            break;
-        case '370':
-            group = parseError370(group);
-            break;
-        case '20':
-            group = parseWarning20(group);
-    }
-
-    return group;
 }


@@ -338,7 +310,7 @@ export default {
                        var coincident = false;
                        do {
                            // first time, move marker up. after that, move marker right.
-                            var delta = coincident ? [0.00002, 0] : [0, 0.00002];
+                            var delta = coincident ? [0.00001, 0] : [0, 0.00001];
                            loc = geoVecAdd(loc, delta);
                            var bbox = geoExtent(loc).bbox();
                            coincident = _krCache.rtree.search(bbox).length;
--- a/modules/ui/keepRight_details.js
+++ b/modules/ui/keepRight_details.js
@@ -1,7 +1,12 @@
-import { event as d3_event } from 'd3-selection';
+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 } from '../util';


 export function uiKeepRightDetails(context) {
@@ -46,28 +51,60 @@ export function uiKeepRightDetails(context) {


        // description
-        var description = detailsEnter
+        var descriptionEnter = detailsEnter
            .append('div')
            .attr('class', 'kr_error-details-description');

-        description
+        descriptionEnter
            .append('h4')
            .text(function() { return t('QA.keepRight.detail_description'); });

-        description
+        descriptionEnter
            .append('div')
            .attr('class', 'kr_error-details-description-text')
            .html(errorDetail);

-        description.selectAll('.kr_error_description-id')
-            .on('click', function() { clickLink(context, this.text); });
+        // If there are entity links in the error message..
+        descriptionEnter.selectAll('.kr_error_entity_link')
+            .each(function() {
+                var link = d3_select(this);
+                var entityID = this.innerText;
+                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);
+                        context.enter(modeSelect(context, [entityID]));
+                    });

-        function clickLink(context, entityID) {
-            d3_event.preventDefault();
-            context.layers().layer('osm').enabled(true);
-            context.zoomToEntity(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) {
+                        var preset = context.presets().match(entity, context.graph());
+                        name = preset && !preset.isFallback() && preset.name();  // fallback to preset name
+                    }
+
+                    if (name) {
+                        this.innerText = name;
+                    }
+                }
+            });
    }


--- a/modules/util/util.js
+++ b/modules/util/util.js
@@ -37,6 +37,7 @@ export function utilEntityOrMemberSelector(ids, graph) {
 export function utilEntityOrDeepMemberSelector(ids, graph) {
    var seen = {};
    var allIDs = [];
+
    function addEntityAndMembersIfNotYetSeen(id) {
        // avoid infinite recursion for circular relations by skipping seen entities
        if (seen[id]) return;
@@ -53,6 +54,7 @@ export function utilEntityOrDeepMemberSelector(ids, graph) {
            }
        }
    }
+
    ids.forEach(function(id) {
        addEntityAndMembersIfNotYetSeen(id);
    });
@@ -85,9 +87,9 @@ export function utilGetAllNodes(ids, graph) {


 export function utilDisplayName(entity) {
-    var localizedNameKey = 'name:' + utilDetect().locale.toLowerCase().split('-')[0],
-        name = entity.tags[localizedNameKey] || entity.tags.name || '',
-        network = entity.tags.cycle_network || entity.tags.network;
+    var localizedNameKey = 'name:' + utilDetect().locale.toLowerCase().split('-')[0];
+    var name = entity.tags[localizedNameKey] || entity.tags.name || '';
+    var network = entity.tags.cycle_network || entity.tags.network;

    if (!name && entity.tags.ref) {
        name = entity.tags.ref;
@@ -145,11 +147,12 @@ export function utilStringQs(str) {


 export function utilQsString(obj, noencode) {
+    // encode everything except special characters used in certain hash parameters:
+    // "/" in map states, ":", ",", {" and "}" in background
    function softEncode(s) {
-      // encode everything except special characters used in certain hash parameters:
-      // "/" in map states, ":", ",", {" and "}" in background
-      return encodeURIComponent(s).replace(/(%2F|%3A|%2C|%7B|%7D)/g, decodeURIComponent);
+        return encodeURIComponent(s).replace(/(%2F|%3A|%2C|%7B|%7D)/g, decodeURIComponent);
    }
+
    return Object.keys(obj).sort().map(function(key) {
        return encodeURIComponent(key) + '=' + (
            noencode ? softEncode(obj[key]) : encodeURIComponent(obj[key]));
@@ -158,36 +161,41 @@ export function utilQsString(obj, noencode) {


 export function utilPrefixDOMProperty(property) {
-    var prefixes = ['webkit', 'ms', 'moz', 'o'],
-        i = -1,
-        n = prefixes.length,
-        s = document.body;
+    var prefixes = ['webkit', 'ms', 'moz', 'o'];
+    var i = -1;
+    var n = prefixes.length;
+    var s = document.body;

    if (property in s)
        return property;

    property = property.substr(0, 1).toUpperCase() + property.substr(1);

-    while (++i < n)
-        if (prefixes[i] + property in s)
+    while (++i < n) {
+        if (prefixes[i] + property in s) {
            return prefixes[i] + property;
+        }
+    }

    return false;
 }


 export function utilPrefixCSSProperty(property) {
-    var prefixes = ['webkit', 'ms', 'Moz', 'O'],
-        i = -1,
-        n = prefixes.length,
-        s = document.body.style;
+    var prefixes = ['webkit', 'ms', 'Moz', 'O'];
+    var i = -1;
+    var n = prefixes.length;
+    var s = document.body.style;

-    if (property.toLowerCase() in s)
+    if (property.toLowerCase() in s) {
        return property.toLowerCase();
+    }

-    while (++i < n)
-        if (prefixes[i] + property in s)
+    while (++i < n) {
+        if (prefixes[i] + property in s) {
            return '-' + prefixes[i].toLowerCase() + property.replace(/([A-Z])/g, '-$1').toLowerCase();
+        }
+    }

    return false;
 }
@@ -195,10 +203,9 @@ export function utilPrefixCSSProperty(property) {

 var transformProperty;
 export function utilSetTransform(el, x, y, scale) {
-    var prop = transformProperty = transformProperty || utilPrefixCSSProperty('Transform'),
-        translate = utilDetect().opera ?
-            'translate('   + x + 'px,' + y + 'px)' :
-            'translate3d(' + x + 'px,' + y + 'px,0)';
+    var prop = transformProperty = transformProperty || utilPrefixCSSProperty('Transform');
+    var translate = utilDetect().opera ? 'translate('   + x + 'px,' + y + 'px)'
+        : 'translate3d(' + x + 'px,' + y + 'px,0)';
    return el.style(prop, translate + (scale ? ' scale(' + scale + ')' : ''));
 }

@@ -233,11 +240,12 @@ export function utilEditDistance(a, b) {
 // 1. Only works on HTML elements, not SVG
 // 2. Does not cause style recalculation
 export function utilFastMouse(container) {
-    var rect = container.getBoundingClientRect(),
-        rectLeft = rect.left,
-        rectTop = rect.top,
-        clientLeft = +container.clientLeft,
-        clientTop = +container.clientTop;
+    var rect = container.getBoundingClientRect();
+    var rectLeft = rect.left;
+    var rectTop = rect.top;
+    var clientLeft = +container.clientLeft;
+    var clientTop = +container.clientTop;
+
    if (textDirection === 'rtl') {
        rectLeft = 0;
    }
@@ -255,9 +263,9 @@ export var utilGetPrototypeOf = Object.getPrototypeOf || function(obj) { return


 export function utilAsyncMap(inputs, func, callback) {
-    var remaining = inputs.length,
-        results = [],
-        errors = [];
+    var remaining = inputs.length;
+    var results = [];
+    var errors = [];

    inputs.forEach(function(d, i) {
        func(d, function done(err, data) {
@@ -272,8 +280,9 @@ export function utilAsyncMap(inputs, func, callback) {

 // wraps an index to an interval [0..length-1]
 export function utilWrap(index, length) {
-    if (index < 0)
+    if (index < 0) {
        index += Math.ceil(-index/length)*length;
+    }
    return index % length;
 }