From d58867df7c3ac8cf191d473c30e149505d2b8f55 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Thu, 20 Dec 2018 14:04:15 -0500 Subject: [PATCH] Add entity display label util function Condense message and tooltip text for for the untagged feature issue Use lighter border color for error issues --- css/80_app.css | 2 +- data/core.yaml | 17 ++++------------- dist/locales/en.json | 20 ++++---------------- modules/util/index.js | 1 + modules/util/util.js | 16 ++++++++++++++++ modules/validations/disconnected_highway.js | 17 +++-------------- modules/validations/missing_tag.js | 10 +++++++--- 7 files changed, 36 insertions(+), 47 deletions(-) diff --git a/css/80_app.css b/css/80_app.css index 89e53d976..631d970b1 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -2874,7 +2874,7 @@ div.full-screen > button:hover { border-color: #FFDF5C; } .entity-issues .issue.severity-error { - border-color: #F5817D; + border-color: #f59c90; } .entity-issues .issue:not(:last-of-type) { margin-bottom: 20px; diff --git a/data/core.yaml b/data/core.yaml index 3764707af..44da62b61 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -883,23 +883,14 @@ en: error: error warning: warning disconnected_highway: - message: "{entityLabel} is disconnected from other highways." + message: "{highway} is disconnected from other highways" tooltip: "Roads should be connected to other roads or building entrances." old_multipolygon: message: Multipolygon tags on outer way tooltip: "This style of multipolygon is deprecated. Please assign the tags to the parent multipolygon instead of the outer way." - untagged_point: - message: Untagged point - tooltip: "Select a feature type that describes what this point is." - untagged_line: - message: Untagged line - tooltip: "Select a feature type that describes what this line is." - untagged_area: - message: Untagged area - tooltip: "Select a feature type that describes what this area is." - untagged_relation: - message: Untagged relation - tooltip: "Select a feature type that describes what this relation is." + untagged_feature: + message: "{feature} has no tags" + tooltip: "Select a feature type that describes what this is." many_deletions: message: "You're deleting {n} features: {p} nodes, {l} lines, {a} areas, {r} relations. Are you sure you want to do this? This will delete them from the map that everyone else sees on openstreetmap.org." tag_suggests_area: diff --git a/dist/locales/en.json b/dist/locales/en.json index 51ce3eabf..8516d2696 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1053,28 +1053,16 @@ "warning": "warning" }, "disconnected_highway": { - "message": "{entityLabel} is disconnected from other highways.", + "message": "{highway} is disconnected from other highways", "tooltip": "Roads should be connected to other roads or building entrances." }, "old_multipolygon": { "message": "Multipolygon tags on outer way", "tooltip": "This style of multipolygon is deprecated. Please assign the tags to the parent multipolygon instead of the outer way." }, - "untagged_point": { - "message": "Untagged point", - "tooltip": "Select a feature type that describes what this point is." - }, - "untagged_line": { - "message": "Untagged line", - "tooltip": "Select a feature type that describes what this line is." - }, - "untagged_area": { - "message": "Untagged area", - "tooltip": "Select a feature type that describes what this area is." - }, - "untagged_relation": { - "message": "Untagged relation", - "tooltip": "Select a feature type that describes what this relation is." + "untagged_feature": { + "message": "{feature} has no tags", + "tooltip": "Select a feature type that describes what this is." }, "many_deletions": { "message": "You're deleting {n} features: {p} nodes, {l} lines, {a} areas, {r} relations. Are you sure you want to do this? This will delete them from the map that everyone else sees on openstreetmap.org." diff --git a/modules/util/index.js b/modules/util/index.js index 0517ce874..9a0ad1251 100644 --- a/modules/util/index.js +++ b/modules/util/index.js @@ -4,6 +4,7 @@ export { utilCleanTags } from './clean_tags'; export { utilDisplayName } from './util'; export { utilDisplayNameForPath } from './util'; export { utilDisplayType } from './util'; +export { utilDisplayLabel } from './util'; export { utilEditDistance } from './util'; export { utilEntitySelector } from './util'; export { utilEntityOrMemberSelector } from './util'; diff --git a/modules/util/util.js b/modules/util/util.js index d9c613263..216a207ae 100644 --- a/modules/util/util.js +++ b/modules/util/util.js @@ -121,6 +121,22 @@ export function utilDisplayType(id) { } +export function utilDisplayLabel(entity, context) { + var displayName = utilDisplayName(entity); + if (displayName) { + // use the display name if there is one + return displayName; + } + var preset = context.presets().match(entity, context.graph()); + if (preset && preset.name()) { + // use the preset name if there is a match + return preset.name(); + } + // fallback to the display type (node/way/relation) + return utilDisplayType(entity.id); +} + + export function utilStringQs(str) { return str.split('&').reduce(function(obj, pair){ var parts = pair.split('='); diff --git a/modules/validations/disconnected_highway.js b/modules/validations/disconnected_highway.js index 4805a0d29..42ef61238 100644 --- a/modules/validations/disconnected_highway.js +++ b/modules/validations/disconnected_highway.js @@ -1,7 +1,6 @@ import { t } from '../util/locale'; import { - utilDisplayName, - utilDisplayType + utilDisplayLabel } from '../util'; import { ValidationIssueType, @@ -35,22 +34,12 @@ export function validationDisconnectedHighway(context) { var issues = []; for (var i = 0; i < changes.created.length; i++) { var entity = changes.created[i]; - if (isDisconnectedHighway(entity, graph)) { - var entityLabel = utilDisplayName(entity); - if (!entityLabel) { - var preset = context.presets().match(entity, graph); - if (preset && preset.name()) { - entityLabel = preset.name(); - } else { - entityLabel = utilDisplayType(entity.id); - } - } - + var entityLabel = utilDisplayLabel(entity, context); issues.push(new validationIssue({ type: ValidationIssueType.disconnected_highway, severity: ValidationIssueSeverity.warning, - message: t('issues.disconnected_highway.message', {entityLabel: entityLabel}), + message: t('issues.disconnected_highway.message', {highway: entityLabel}), tooltip: t('issues.disconnected_highway.tooltip'), entities: [entity], })); diff --git a/modules/validations/missing_tag.js b/modules/validations/missing_tag.js index 40adb5acd..3b1d9f8bb 100644 --- a/modules/validations/missing_tag.js +++ b/modules/validations/missing_tag.js @@ -1,12 +1,15 @@ import _without from 'lodash-es/without'; import { t } from '../util/locale'; +import { + utilDisplayLabel +} from '../util'; import { ValidationIssueType, ValidationIssueSeverity, validationIssue, } from './validation_issue'; -export function validationMissingTag() { +export function validationMissingTag(context) { // Slightly stricter check than Entity#isUsed (#3091) function hasTags(entity, graph) { @@ -23,11 +26,12 @@ export function validationMissingTag() { geometry = change.geometry(graph); if (types.indexOf(geometry) !== -1 && !hasTags(change, graph)) { + var entityLabel = utilDisplayLabel(change, context); issues.push(new validationIssue({ type: ValidationIssueType.missing_tag, severity: ValidationIssueSeverity.error, - message: t('issues.untagged_' + geometry + '.message'), - tooltip: t('issues.untagged_' + geometry + '.tooltip'), + message: t('issues.untagged_feature.message', {feature: entityLabel}), + tooltip: t('issues.untagged_feature.tooltip'), entities: [change], })); }