From b8ac9a7de3faa4b4f7f1e1683251634fe0f1727d Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Mon, 25 Feb 2019 15:31:05 -0500 Subject: [PATCH] Add toggle-able list of rules to the Issues pane (close #5979) Toggling a rule off prevents the validation from running --- css/80_app.css | 1 + data/core.yaml | 12 ++++++ dist/locales/en.json | 13 ++++++ modules/core/validator.js | 35 +++++++++++++++ modules/ui/issues.js | 90 ++++++++++++++++++++++++++++++++++++++- 5 files changed, 149 insertions(+), 2 deletions(-) diff --git a/css/80_app.css b/css/80_app.css index 265b21f9d..2dd8b885a 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -3099,6 +3099,7 @@ div.full-screen > button:hover { padding: 5px !important; display: flex; margin-top: 5px; + margin-bottom: 15px; } .issues-none .icon { color: #05AC10; diff --git a/data/core.yaml b/data/core.yaml index b2011a69d..4fe523c6f 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -1174,15 +1174,20 @@ en: list_title: "Errors ({count})" warnings: list_title: "Warnings ({count})" + rules: + title: Rules no_issues: message: Everything looks fine info: Any issues will show up here as you edit almost_junction: + title: Almost Junctions message: "{feature} is very close but not connected to {feature2}" highway-highway: tip: Intersecting highways should share a junction vertex. crossing_ways: + title: Crossings Ways message: "{feature} crosses {feature2}" + tip: Highways, railways, waterways, and buildings should not cross ambiguously. building-building: tip: "Buildings should not intersect except on different layers." building-highway: @@ -1216,25 +1221,30 @@ en: indoor-indoor_connectable: tip: "Crossing indoor features should be connected or use different levels." deprecated_tag: + title: Deprecated Tags single: message: '{feature} has the outdated tag "{tag}"' combination: message: '{feature} has an outdated tag combination: {tags}' tip: "Some tags become deprecated over time and should be replaced." disconnected_way: + title: Disconnected Ways highway: message: "{highway} is disconnected from other roads and paths" tip: "Highways should connect to other highways or building entrances." generic_name: + title: Generic Names message: '{feature} has the generic name "{name}"' tip: "Names should be the actual, on-the-ground names of features." many_deletions: + title: Many Deletions points-lines-areas: message: "Deleting {n} features: {p} points, {l} lines, and {a} areas" points-lines-areas-relations: message: "Deleting {n} features: {p} points, {l} lines, {a} areas, and {r} relations" tip: "Only redundant or nonexistent features should be deleted." missing_tag: + title: Missing Tags any: message: "{feature} has no tags" descriptive: @@ -1243,9 +1253,11 @@ en: message: '{feature} has no "{tag}" tag' tip: "Features must have tags that define what they are." old_multipolygon: + title: Misplaced Multipolygon Tags message: "{multipolygon} has misplaced tags" tip: "Multipolygons should be tagged on their relation, not their outer way." tag_suggests_area: + title: Lines Tagged as Areas message: '{feature} should be a closed area based on the tag "{tag}"' tip: "Areas must have connected endpoints." fix: diff --git a/dist/locales/en.json b/dist/locales/en.json index ff5557329..95390ba16 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1424,18 +1424,24 @@ "warnings": { "list_title": "Warnings ({count})" }, + "rules": { + "title": "Rules" + }, "no_issues": { "message": "Everything looks fine", "info": "Any issues will show up here as you edit" }, "almost_junction": { + "title": "Almost Junctions", "message": "{feature} is very close but not connected to {feature2}", "highway-highway": { "tip": "Intersecting highways should share a junction vertex." } }, "crossing_ways": { + "title": "Crossings Ways", "message": "{feature} crosses {feature2}", + "tip": "Highways, railways, waterways, and buildings should not cross ambiguously.", "building-building": { "tip": "Buildings should not intersect except on different layers." }, @@ -1486,6 +1492,7 @@ } }, "deprecated_tag": { + "title": "Deprecated Tags", "single": { "message": "{feature} has the outdated tag \"{tag}\"" }, @@ -1495,16 +1502,19 @@ "tip": "Some tags become deprecated over time and should be replaced." }, "disconnected_way": { + "title": "Disconnected Ways", "highway": { "message": "{highway} is disconnected from other roads and paths", "tip": "Highways should connect to other highways or building entrances." } }, "generic_name": { + "title": "Generic Names", "message": "{feature} has the generic name \"{name}\"", "tip": "Names should be the actual, on-the-ground names of features." }, "many_deletions": { + "title": "Many Deletions", "points-lines-areas": { "message": "Deleting {n} features: {p} points, {l} lines, and {a} areas" }, @@ -1514,6 +1524,7 @@ "tip": "Only redundant or nonexistent features should be deleted." }, "missing_tag": { + "title": "Missing Tags", "any": { "message": "{feature} has no tags" }, @@ -1526,10 +1537,12 @@ "tip": "Features must have tags that define what they are." }, "old_multipolygon": { + "title": "Misplaced Multipolygon Tags", "message": "{multipolygon} has misplaced tags", "tip": "Multipolygons should be tagged on their relation, not their outer way." }, "tag_suggests_area": { + "title": "Lines Tagged as Areas", "message": "{feature} should be a closed area based on the tag \"{tag}\"", "tip": "Areas must have connected endpoints." }, diff --git a/modules/core/validator.js b/modules/core/validator.js index 17bf56fae..4abf900bd 100644 --- a/modules/core/validator.js +++ b/modules/core/validator.js @@ -8,6 +8,7 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; import { geoExtent } from '../geo'; import { osmEntity } from '../osm'; +import { t } from '../util/locale'; import { utilRebind } from '../util/rebind'; import * as Validations from '../validations/index'; @@ -18,6 +19,8 @@ export function coreValidator(context) { var _issues = []; var _issuesByEntityID = {}; + var _disabledValidations = {}; + var validations = _filter(Validations, _isFunction).reduce(function(obj, validation) { var func = validation(); obj[func.type] = func; @@ -36,6 +39,13 @@ export function coreValidator(context) { } } + var validationIDsToDisplay = Object.keys(validations).filter(function(rule) { + return rule !== 'maprules'; + }); + validationIDsToDisplay.sort(function(rule1, rule2) { + return t('issues.' + rule1 + '.title') > t('issues.' + rule2 + '.title'); + }); + //self.featureApplicabilityOptions = ['edited', 'all']; /*var featureApplicability = context.storage('issue-features') || 'edited'; @@ -72,6 +82,22 @@ export function coreValidator(context) { return _issuesByEntityID[key]; }; + self.getRuleIDs = function(){ + return validationIDsToDisplay; + }; + + self.getDisabledRules = function(){ + return _disabledValidations; + }; + + self.toggleRule = function(ruleID) { + if (_disabledValidations[ruleID]) { + delete _disabledValidations[ruleID]; + } else { + _disabledValidations[ruleID] = true; + } + self.validate(); + }; function validateEntity(entity) { var _issues = []; @@ -81,6 +107,12 @@ export function coreValidator(context) { function runValidation(which) { if (ran[which]) return true; + if (_disabledValidations[which]) { + // don't run disabled validations but mark as having run + ran[which] = true; + return true; + } + var fn = validations[which]; var typeIssues = fn(entity, context); _issues = _issues.concat(typeIssues); @@ -134,6 +166,9 @@ export function coreValidator(context) { var graph = history.graph(); _issues = _flatten(changesValidationIDs.map(function(ruleID) { + if (_disabledValidations[ruleID]) { + return []; + } var validation = validations[ruleID]; return validation(changes, context); })); diff --git a/modules/ui/issues.js b/modules/ui/issues.js index f323e76f0..7343a35b9 100644 --- a/modules/ui/issues.js +++ b/modules/ui/issues.js @@ -18,8 +18,8 @@ import { utilHighlightEntities } from '../util'; export function uiIssues(context) { var key = t('issues.key'); //var _featureApplicabilityList = d3_select(null); - var _errorsList = d3_select(null); - var _warningsList = d3_select(null); + var _errorsList = d3_select(null), _warningsList = d3_select(null); + var _rulesList = d3_select(null); var _pane = d3_select(null); var _toggleButton = d3_select(null); var _shown = false; @@ -186,6 +186,18 @@ export function uiIssues(context) { .text(t('issues.no_issues.info')); } + function renderRulesList(selection) { + var container = selection.selectAll('.issue-rules-list') + .data([0]); + + _rulesList = container.enter() + .append('ul') + .attr('class', 'layer-list issue-rules-list') + .merge(container); + + updateRulesList(); + } + /* function showsFeatureApplicability(d) { return context.validator().getFeatureApplicability() === d; @@ -221,6 +233,20 @@ export function uiIssues(context) { .call(drawIssuesList, warnings); } + function updateRulesList() { + var rules = context.validator().getRuleIDs(); + _rulesList + .call(drawListItems, rules, 'checkbox', 'rule', toggleRule, ruleIsEnabled); + } + + function ruleIsEnabled(d) { + return !context.validator().getDisabledRules()[d]; + } + + function toggleRule(d) { + context.validator().toggleRule(d); + } + function update() { var errors = context.validator().getErrors(); @@ -259,6 +285,57 @@ export function uiIssues(context) { //if (!_pane.select('.disclosure-wrap-issues_options').classed('hide')) { // updateFeatureApplicabilityList(); //} + + if (!_pane.select('.disclosure-wrap-issues_rules').classed('hide')) { + updateRulesList(); + } + } + + function drawListItems(selection, data, type, name, change, active) { + var items = selection.selectAll('li') + .data(data); + + // Exit + items.exit() + .remove(); + + // Enter + var enter = items.enter() + .append('li') + .call(tooltip() + .title(function(d) { + if (d === 'disconnected_way') { + d += '.highway'; + } else if (d === 'almost_junction') { + d += '.highway-highway'; + } + return t('issues.' + d + '.tip'); + }) + .placement('top') + ); + + var label = enter + .append('label'); + + label + .append('input') + .attr('type', type) + .attr('name', name) + .on('change', change); + + label + .append('span') + .text(function(d) { return t('issues.' + d + '.title'); }); + + // Update + items = items + .merge(enter); + + items + .classed('active', active) + .selectAll('input') + .property('checked', active) + .property('indeterminate', false); } var paneTooltip = tooltip() @@ -364,6 +441,15 @@ export function uiIssues(context) { .content(renderWarningsList) ); + // rules + content + .append('div') + .attr('class', 'issues-rules') + .call(uiDisclosure(context, 'issues_rules', false) + .title(t('issues.rules.title')) + .content(renderRulesList) + ); + // options /* // add this back to core.yaml when re-enabling the options