From cf0214d06a2b08f29c1418316412987e768d819c Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Tue, 18 Dec 2018 11:11:15 -0500 Subject: [PATCH 001/166] Add the Issues pane --- data/core.yaml | 4 ++ dist/locales/en.json | 67 ++++-------------------- modules/ui/background.js | 4 +- modules/ui/help.js | 2 + modules/ui/init.js | 6 +++ modules/ui/issues.js | 110 +++++++++++++++++++++++++++++++++++++++ modules/ui/map_data.js | 2 + 7 files changed, 138 insertions(+), 57 deletions(-) create mode 100644 modules/ui/issues.js diff --git a/data/core.yaml b/data/core.yaml index 2f5214e3bf..39ee8def33 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -881,6 +881,10 @@ en: indirect: "**Some restrictions display the text \"(indirect)\" and are drawn lighter.**" indirect_example: "These restrictions exist because of another nearby restriction. For example, an \"Only Straight On\" restriction will indirectly create \"No Turn\" restrictions for all other paths through the intersection." indirect_noedit: "You may not edit indirect restrictions. Instead, edit the nearby direct restriction." + issues: + title: Issues + description: Issues + key: I intro: done: done ok: OK diff --git a/dist/locales/en.json b/dist/locales/en.json index c4ae1b817f..c0e35c271e 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1047,6 +1047,11 @@ } } }, + "issues": { + "title": "Issues", + "description": "Issues", + "key": "I" + }, "intro": { "done": "done", "ok": "OK", @@ -7550,6 +7555,9 @@ "SPW_PICC": { "name": "SPW(allonie) PICC numerical imagery" }, + "US-TIGER-Roads-2012": { + "name": "TIGER Roads 2012" + }, "US-TIGER-Roads-2014": { "description": "At zoom level 16+, public domain map data from the US Census. At lower zooms, only changes since 2006 minus changes already incorporated into OpenStreetMap", "name": "TIGER Roads 2014" @@ -7558,10 +7566,6 @@ "description": "Yellow = Public domain map data from the US Census. Red = Data not found in OpenStreetMap", "name": "TIGER Roads 2017" }, - "US-TIGER-Roads-2018": { - "description": "Yellow = Public domain map data from the US Census. Red = Data not found in OpenStreetMap", - "name": "TIGER Roads 2018" - }, "US_Forest_Service_roads_overlay": { "description": "Highway: Green casing = unclassified. Brown casing = track. Surface: gravel = light brown fill, Asphalt = black, paved = gray, ground =white, concrete = blue, grass = green. Seasonal = white bars", "name": "U.S. Forest Roads Overlay" @@ -7578,12 +7582,6 @@ }, "name": "UrbIS-Ortho 2017" }, - "UrbISOrtho2018": { - "attribution": { - "text": "Realized by means of Brussels UrbIS®© - Distribution & Copyright CIRB" - }, - "name": "UrbIS-Ortho 2018" - }, "UrbisAdmFR": { "attribution": { "text": "Realized by means of Brussels UrbIS®© - Distribution & Copyright CIRB" @@ -7668,33 +7666,11 @@ "description": "Japan GSI Standard Map. Widely covered.", "name": "Japan GSI Standard Map" }, - "helsingborg-orto": { - "attribution": { - "text": "© Helsingborg municipality" - }, - "description": "Orthophotos from the municipality of Helsingborg 2016, public domain", - "name": "Helsingborg Orthophoto" - }, - "kalmar-orto-2014": { - "attribution": { - "text": "© Kalmar municipality" - }, - "description": "Orthophotos for the north coast of the municipality of Kalmar 2014", - "name": "Kalmar North Orthophoto 2014" - }, - "kalmar-orto-2016": { + "hike_n_bike": { "attribution": { - "text": "© Kalmar municipality" - }, - "description": "Orthophotos for the south coast of the municipality of Kalmar 2016", - "name": "Kalmar South Orthophoto 2016" - }, - "kalmar-orto-2018": { - "attribution": { - "text": "© Kalmar municipality" + "text": "© OpenStreetMap contributors" }, - "description": "Orthophotos for urban areas of the municipality of Kalmar 2018", - "name": "Kalmar Urban Orthophoto 2018" + "name": "Hike & Bike" }, "kelkkareitit": { "attribution": { @@ -7717,20 +7693,6 @@ "description": "Mosaic of Swedish orthophotos from the period 1970–1980. Is under construction.", "name": "Lantmäteriet Historic Orthophoto 1975" }, - "lantmateriet-topowebb": { - "attribution": { - "text": "© Lantmäteriet, CC0" - }, - "description": "Topographic map of Sweden 1:50 000", - "name": "Lantmäteriet Topographic Map" - }, - "linkoping-orto": { - "attribution": { - "text": "© Linköping municipality" - }, - "description": "Orthophotos from the municipality of Linköping 2010, open data", - "name": "Linköping Orthophoto" - }, "mapbox_locator_overlay": { "attribution": { "text": "Terms & Feedback" @@ -7795,13 +7757,6 @@ }, "name": "Stamen Terrain" }, - "stockholm-orto": { - "attribution": { - "text": "© Stockholm municipality, CC0" - }, - "description": "Orthophotos from the municipality of Stockholm 2015, CC0 license", - "name": "Stockholm Orthophoto" - }, "tf-cycle": { "attribution": { "text": "Maps © Thunderforest, Data © OpenStreetMap contributors" diff --git a/modules/ui/background.js b/modules/ui/background.js index 5c2f3ebdbc..d82ff280aa 100644 --- a/modules/ui/background.js +++ b/modules/ui/background.js @@ -17,6 +17,7 @@ import { uiBackgroundOffset } from './background_offset'; import { uiCmd } from './cmd'; import { uiDisclosure } from './disclosure'; import { uiHelp } from './help'; +import { uiIssues } from './issues'; import { uiMapData } from './map_data'; import { uiMapInMap } from './map_in_map'; import { uiSettingsCustomBackground } from './settings/custom_background'; @@ -310,8 +311,9 @@ export function uiBackground(context) { _shown = show; if (show) { - uiMapData.hidePane(); uiHelp.hidePane(); + uiIssues.hidePane(); + uiMapData.hidePane(); update(); pane diff --git a/modules/ui/help.js b/modules/ui/help.js index 195c348194..20883e66c8 100644 --- a/modules/ui/help.js +++ b/modules/ui/help.js @@ -9,6 +9,7 @@ import { uiCmd } from './cmd'; import { uiBackground } from './background'; import { uiIntro } from './intro'; import { uiMapData } from './map_data'; +import { uiIssues } from './issues'; import { uiShortcuts } from './shortcuts'; import { uiTooltipHtml } from './tooltipHtml'; @@ -294,6 +295,7 @@ export function uiHelp(context) { if (show) { uiBackground.hidePane(); + uiIssues.hidePane(); uiMapData.hidePane(); pane.style('display', 'block') diff --git a/modules/ui/init.js b/modules/ui/init.js index 29d4defaa4..737b4bec70 100644 --- a/modules/ui/init.js +++ b/modules/ui/init.js @@ -21,6 +21,7 @@ import { uiGeolocate } from './geolocate'; import { uiHelp } from './help'; import { uiInfo } from './info'; import { uiIntro } from './intro'; +import { uiIssues } from './issues'; import { uiLoading } from './loading'; import { uiMapData } from './map_data'; import { uiMapInMap } from './map_in_map'; @@ -165,6 +166,11 @@ export function uiInit(context) { .attr('class', 'map-control map-data-control') .call(uiMapData(context)); + controls + .append('div') + .attr('class', 'map-control map-issues-control') + .call(uiIssues(context)); + controls .append('div') .attr('class', 'map-control help-control') diff --git a/modules/ui/issues.js b/modules/ui/issues.js new file mode 100644 index 0000000000..bcf2d08347 --- /dev/null +++ b/modules/ui/issues.js @@ -0,0 +1,110 @@ +import { + event as d3_event, + select as d3_select +} from 'd3-selection'; + +import { svgIcon } from '../svg'; +import { t, textDirection } from '../util/locale'; +import { tooltip } from '../util/tooltip'; +import { geoExtent } from '../geo'; +import { modeBrowse } from '../modes'; +import { uiBackground } from './background'; +import { uiDisclosure } from './disclosure'; +import { uiHelp } from './help'; +import { uiMapData } from './map_data'; +import { uiSettingsCustomData } from './settings/custom_data'; +import { uiTooltipHtml } from './tooltipHtml'; + + +export function uiIssues(context) { + var key = t('issues.key'); + var _shown = false; + + function update() { + + } + + function issues(selection) { + + function hidePane() { + setVisible(false); + } + + function togglePane() { + if (d3_event) d3_event.preventDefault(); + setVisible(!button.classed('active')); + } + + function setVisible(show) { + if (show !== _shown) { + button.classed('active', show); + _shown = show; + + if (show) { + uiBackground.hidePane(); + uiHelp.hidePane(); + uiMapData.hidePane(); + update(); + + pane + .style('display', 'block') + .style('right', '-300px') + .transition() + .duration(200) + .style('right', '0px'); + + } else { + pane + .style('display', 'block') + .style('right', '0px') + .transition() + .duration(200) + .style('right', '-300px') + .on('end', function() { + d3_select(this).style('display', 'none'); + }); + } + } + } + + var pane = selection + .append('div') + .attr('class', 'fillL map-pane hide'); + + var paneTooltip = tooltip() + .placement((textDirection === 'rtl') ? 'right' : 'left') + .html(true) + .title(uiTooltipHtml(t('issues.description'), key)); + + var button = selection + .append('button') + .attr('tabindex', -1) + .on('click', togglePane) + .call(svgIcon('#iD-icon-alert', 'light')) + .call(paneTooltip); + + var heading = pane + .append('div') + .attr('class', 'pane-heading'); + + heading + .append('h2') + .text(t('issues.title')); + + heading + .append('button') + .on('click', function() { uiIssues.hidePane(); }) + .call(svgIcon('#iD-icon-close')); + + update(); + + context.keybinding() + .on(key, togglePane); + + uiIssues.hidePane = hidePane; + uiIssues.togglePane = togglePane; + uiIssues.setVisible = setVisible; + } + + return issues; +} diff --git a/modules/ui/map_data.js b/modules/ui/map_data.js index 3fa8cfb8f5..868ae141c4 100644 --- a/modules/ui/map_data.js +++ b/modules/ui/map_data.js @@ -11,6 +11,7 @@ import { modeBrowse } from '../modes'; import { uiBackground } from './background'; import { uiDisclosure } from './disclosure'; import { uiHelp } from './help'; +import { uiIssues } from './issues'; import { uiSettingsCustomData } from './settings/custom_data'; import { uiTooltipHtml } from './tooltipHtml'; @@ -550,6 +551,7 @@ export function uiMapData(context) { if (show) { uiBackground.hidePane(); uiHelp.hidePane(); + uiIssues.hidePane(); update(); pane From c1240af464459809863a347b828ef945ba757895 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Tue, 18 Dec 2018 17:04:40 -0500 Subject: [PATCH 002/166] Added options UI in the Issues pane with edited vs all features selection Added basic issues list in the Issues pane --- data/core.yaml | 9 +++ dist/locales/en.json | 15 +++- modules/ui/issues.js | 189 ++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 210 insertions(+), 3 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index c5435603d6..1e25b11dc0 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -886,6 +886,15 @@ en: title: Issues description: Issues key: I + options: + title: Options + feature_applicability: + edited: + description: Edited features only + tooltip: Only issues for features you change will be flagged + all: + description: All features + tooltip: Issues for all loaded features will be flagged intro: done: done ok: OK diff --git a/dist/locales/en.json b/dist/locales/en.json index 064f48da2b..4f27dd7a33 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1051,7 +1051,20 @@ "issues": { "title": "Issues", "description": "Issues", - "key": "I" + "key": "I", + "options": { + "title": "Options" + }, + "feature_applicability": { + "edited": { + "description": "Edited features only", + "tooltip": "Only issues for features you change will be flagged" + }, + "all": { + "description": "All features", + "tooltip": "Issues for all loaded features will be flagged" + } + } }, "intro": { "done": "done", diff --git a/modules/ui/issues.js b/modules/ui/issues.js index bcf2d08347..694a5272cf 100644 --- a/modules/ui/issues.js +++ b/modules/ui/issues.js @@ -7,7 +7,9 @@ import { svgIcon } from '../svg'; import { t, textDirection } from '../util/locale'; import { tooltip } from '../util/tooltip'; import { geoExtent } from '../geo'; -import { modeBrowse } from '../modes'; +import { modeBrowse, + modeSelect +} from '../modes'; import { uiBackground } from './background'; import { uiDisclosure } from './disclosure'; import { uiHelp } from './help'; @@ -18,10 +20,171 @@ import { uiTooltipHtml } from './tooltipHtml'; export function uiIssues(context) { var key = t('issues.key'); + var featureApplicability = ['edited', 'all']; + var _issuesOptionsContainer = d3_select(null); + var _featureApplicabilityList = d3_select(null); + var _issuesList = d3_select(null); var _shown = false; + var _selectedFeatureApplicability = context.storage('issue-features') || 'edited'; + + function renderIssuesOptions(selection) { + var container = selection.selectAll('.issues-options-container') + .data([0]); + + _issuesOptionsContainer = container.enter() + .append('div') + .attr('class', 'issues-options-container') + .merge(container); + + _featureApplicabilityList = container.selectAll('.feature-applicability-list') + .data([0]); + + _featureApplicabilityList = container.enter() + .append('ul') + .attr('class', 'layer-list feature-applicability-list') + .merge(_featureApplicabilityList); + } + + function renderIssuesList(selection) { + _issuesList = selection.selectAll('.issues-list') + .data([0]); + + _issuesList = _issuesList.enter() + .append('ul') + .attr('class', 'layer-list issues-list') + .merge(_issuesList); + } + + 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') + .attr('class', 'layer') + .call(tooltip() + .html(true) + .title(function(d) { + var tip = t('issues.' + name + '.' + d + '.tooltip'); + return uiTooltipHtml(tip); + }) + .placement('bottom') + ); + + 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.' + name + '.' + d + '.description'); }); + + // Update + items = items + .merge(enter); + + items + .classed('active', active) + .selectAll('input') + .property('checked', active) + .property('indeterminate', function(d) { + return (name === 'feature' && autoHiddenFeature(d)); + }); + } + + function drawIssuesList(selection) { + + var name = 'issues_list'; + + var changes = context.history().changes(); + var validations = context.history().validate(changes); + + /*validations = _reduce(validations, function(validations, val) { + var severity = val.severity; + if (validations.hasOwnProperty(severity)) { + validations[severity].push(val); + } else { + validations[severity] = [val]; + } + return validations; + }, {});*/ + + var items = selection.selectAll('li') + .data(validations); + + // Exit + items.exit() + .remove(); + + // Enter + var enter = items.enter() + .append('li') + .attr('class', 'layer') + .call(tooltip() + .html(true) + .title(function(d) { + var tip = d.tooltip; + return uiTooltipHtml(tip); + }) + .placement('bottom') + ) + .on('click', function(d) { + context.enter(modeSelect(context, [d.entity.id])); + }); + + var label = enter + .append('label'); + + /*label + .append('input') + .attr('type', type) + .attr('name', name) + .on('change', change); +*/ + label + .append('span') + .text(function(d) { return d.message; }); + + // Update + items = items + .merge(enter); + + /* items + .classed('active', active) + .selectAll('input') + .property('checked', active) + .property('indeterminate', function(d) { + return (name === 'feature' && autoHiddenFeature(d)); + });*/ + } + + function showsFeatureApplicability(d) { + return _selectedFeatureApplicability === d; + } + + + function _setFeatureApplicability(d) { + _selectedFeatureApplicability = d; + context.storage('issue-features', d); + update(); + } function update() { + _featureApplicabilityList + .call(drawListItems, featureApplicability, 'radio', 'feature_applicability', _setFeatureApplicability, showsFeatureApplicability); + _issuesList + .call(drawIssuesList); } function issues(selection) { @@ -45,7 +208,7 @@ export function uiIssues(context) { uiHelp.hidePane(); uiMapData.hidePane(); update(); - + pane .style('display', 'block') .style('right', '-300px') @@ -96,6 +259,28 @@ export function uiIssues(context) { .on('click', function() { uiIssues.hidePane(); }) .call(svgIcon('#iD-icon-close')); + var content = pane + .append('div') + .attr('class', 'pane-content'); + + // issues + content + .append('div') + .attr('class', 'issues-issues') + .call(uiDisclosure(context, 'issues_issues', true) + .title(t('issues.title')) + .content(renderIssuesList) + ); + + // options + content + .append('div') + .attr('class', 'issues-options') + .call(uiDisclosure(context, 'issues_options', true) + .title(t('issues.options.title')) + .content(renderIssuesOptions) + ); + update(); context.keybinding() From 30f25c6e8d4adbbaf448f80037e96d6c090caa79 Mon Sep 17 00:00:00 2001 From: Andrew Wong Date: Tue, 18 Dec 2018 17:52:07 -0500 Subject: [PATCH 003/166] added IssueManager - IssueManager performs validation on the current graph and also stores the issues and notifies listeners when the issues changed --- modules/core/context.js | 7 +++++-- modules/ui/commit_warnings.js | 4 ++-- modules/validations/issueManager.js | 22 ++++++++++++++++++++++ 3 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 modules/validations/issueManager.js diff --git a/modules/core/context.js b/modules/core/context.js index 53385070d6..7d0fa6eb38 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -13,6 +13,7 @@ import { select as d3_select } from 'd3-selection'; import { t, currentLocale, addTranslation, setLocale } from '../util/locale'; import { coreHistory } from './history'; +import { IssueManager } from '../validations/issueManager'; import { dataLocales, dataEn } from '../../data'; import { geoRawMercator } from '../geo/raw_mercator'; import { modeSelect } from '../modes/select'; @@ -95,10 +96,10 @@ export function coreContext() { /* Straight accessors. Avoid using these if you can. */ - var connection, history; + var connection, history, issueManager; context.connection = function() { return connection; }; context.history = function() { return history; }; - + context.issueManager = function() { return issueManager; }; /* Connection */ context.preauth = function(options) { @@ -446,6 +447,8 @@ export function coreContext() { context.changes = history.changes; context.intersects = history.intersects; + issueManager = IssueManager(context); + // Debounce save, since it's a synchronous localStorage write, // and history changes can happen frequently (e.g. when dragging). context.debouncedSave = _debounce(context.save, 350); diff --git a/modules/ui/commit_warnings.js b/modules/ui/commit_warnings.js index 8b517c9c31..29ecf44a1e 100644 --- a/modules/ui/commit_warnings.js +++ b/modules/ui/commit_warnings.js @@ -11,8 +11,8 @@ export function uiCommitWarnings(context) { function commitWarnings(selection) { - var changes = context.history().changes(); - var validations = context.history().validate(changes); + // maybe call these issues now? + var validations = context.issueManager().validate(); validations = _reduce(validations, function(validations, val) { var severity = val.severity; diff --git a/modules/validations/issueManager.js b/modules/validations/issueManager.js new file mode 100644 index 0000000000..ae007a3cac --- /dev/null +++ b/modules/validations/issueManager.js @@ -0,0 +1,22 @@ +import * as d3 from 'd3'; +import { utilRebind } from '../util/rebind'; + +export function IssueManager(context) { + var dispatch = d3.dispatch('reload'), + self = {}, + issues = []; + + self.getIssues = function() { + self.validate(); + return issues; + }; + + self.validate = function() { + var changes = context.history().changes(); + issues = context.history().validate(changes); + dispatch.call('reload', self, issues); + return issues; + }; + + return utilRebind(self, dispatch, 'on'); +} From 858a9606d598cde17786bab0700a0678b94c705f Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Tue, 18 Dec 2018 18:05:27 -0500 Subject: [PATCH 004/166] Added entity display name or preset name to the disconnected highway message --- data/core.yaml | 2 +- dist/locales/en.json | 2 +- modules/core/history.js | 2 +- modules/validations/disconnected_highway.js | 14 ++++++++++++-- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index 1e25b11dc0..2ba0f89d76 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -623,7 +623,7 @@ en: on_wiki: "{tag} on wiki.osm.org" used_with: "used with {type}" validations: - disconnected_highway: Disconnected highway + disconnected_highway: "{entityLabel} is disconnected from other highways." disconnected_highway_tooltip: "Roads should be connected to other roads or building entrances." old_multipolygon: Multipolygon tags on outer way old_multipolygon_tooltip: "This style of multipolygon is deprecated. Please assign the tags to the parent multipolygon instead of the outer way." diff --git a/dist/locales/en.json b/dist/locales/en.json index 4f27dd7a33..0a04218ef5 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -760,7 +760,7 @@ "used_with": "used with {type}" }, "validations": { - "disconnected_highway": "Disconnected highway", + "disconnected_highway": "{entityLabel} is disconnected from other highways.", "disconnected_highway_tooltip": "Roads should be connected to other roads or building entrances.", "old_multipolygon": "Multipolygon tags on outer way", "old_multipolygon_tooltip": "This style of multipolygon is deprecated. Please assign the tags to the parent multipolygon instead of the outer way.", diff --git a/modules/core/history.js b/modules/core/history.js index db837d45ce..2699b92009 100644 --- a/modules/core/history.js +++ b/modules/core/history.js @@ -282,7 +282,7 @@ export function coreHistory(context) { validate: function(changes) { return _flatten(_map(Validations, function(fn) { - return fn()(changes, _stack[_index].graph); + return fn(context)(changes, _stack[_index].graph); })); }, diff --git a/modules/validations/disconnected_highway.js b/modules/validations/disconnected_highway.js index ef2d8cf8c0..cc07dba853 100644 --- a/modules/validations/disconnected_highway.js +++ b/modules/validations/disconnected_highway.js @@ -1,7 +1,8 @@ import { t } from '../util/locale'; +import { utilDisplayName } from '../util'; -export function validationDisconnectedHighway() { +export function validationDisconnectedHighway(context) { function isDisconnectedHighway(entity, graph) { if (!entity.tags.highway) return false; @@ -28,9 +29,18 @@ export function validationDisconnectedHighway() { 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) + } + } warnings.push({ id: 'disconnected_highway', - message: t('validations.disconnected_highway'), + message: t('validations.disconnected_highway', {entityLabel: entityLabel}), tooltip: t('validations.disconnected_highway_tooltip'), entity: entity }); From 0d0521c9368029fb8d482ba610d04ae579d7d3ca Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Wed, 19 Dec 2018 10:32:14 -0500 Subject: [PATCH 005/166] Moved feature applicability into the issue manager class --- modules/ui/issues.js | 20 +++++++++++--------- modules/validations/issueManager.js | 13 +++++++++++++ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/modules/ui/issues.js b/modules/ui/issues.js index 694a5272cf..1660aeba56 100644 --- a/modules/ui/issues.js +++ b/modules/ui/issues.js @@ -17,15 +17,12 @@ import { uiMapData } from './map_data'; import { uiSettingsCustomData } from './settings/custom_data'; import { uiTooltipHtml } from './tooltipHtml'; - export function uiIssues(context) { var key = t('issues.key'); - var featureApplicability = ['edited', 'all']; var _issuesOptionsContainer = d3_select(null); var _featureApplicabilityList = d3_select(null); var _issuesList = d3_select(null); var _shown = false; - var _selectedFeatureApplicability = context.storage('issue-features') || 'edited'; function renderIssuesOptions(selection) { var container = selection.selectAll('.issues-options-container') @@ -169,19 +166,24 @@ export function uiIssues(context) { } function showsFeatureApplicability(d) { - return _selectedFeatureApplicability === d; + return context.issueManager().getFeatureApplicability() === d; } - - function _setFeatureApplicability(d) { - _selectedFeatureApplicability = d; - context.storage('issue-features', d); + function setFeatureApplicability(d) { + context.issueManager().setFeatureApplicability(d); update(); } function update() { _featureApplicabilityList - .call(drawListItems, featureApplicability, 'radio', 'feature_applicability', _setFeatureApplicability, showsFeatureApplicability); + .call( + drawListItems, + context.issueManager().featureApplicabilityOptions, + 'radio', + 'feature_applicability', + setFeatureApplicability, + showsFeatureApplicability + ); _issuesList .call(drawIssuesList); diff --git a/modules/validations/issueManager.js b/modules/validations/issueManager.js index ae007a3cac..4bcdc162aa 100644 --- a/modules/validations/issueManager.js +++ b/modules/validations/issueManager.js @@ -6,6 +6,19 @@ export function IssueManager(context) { self = {}, issues = []; + self.featureApplicabilityOptions = ['edited', 'all']; + + var featureApplicability = context.storage('issue-features') || 'edited'; + + self.getFeatureApplicability = function() { + return featureApplicability; + }; + + self.setFeatureApplicability = function(applicability) { + featureApplicability = applicability; + context.storage('issue-features', applicability); + }; + self.getIssues = function() { self.validate(); return issues; From 07a53fe6ea785c62ee2cedbf9fa8454ec7861fa4 Mon Sep 17 00:00:00 2001 From: Xiaoming Gao Date: Tue, 18 Dec 2018 20:03:57 -0500 Subject: [PATCH 006/166] Extend data model for validation issues Add the Issues pane 1. Add a class to represent the validation issue 2. Extend the data model for an validation issue to (1) add a severity level field (useful for identify save-blocking issues later) (2) replace single entity with an array of entities (useful for issues involving multiple entities) (3) add a coordinates field for highlighting the location of the issue on the map (4) add a fixes field for possible automatic fixes 3. Update existing validation modules to use the new data model --- dist/locales/en.json | 62 +++++++++++++++++++-- modules/core/history.js | 9 ++- modules/services/maprules.js | 17 +++--- modules/ui/commit_warnings.js | 35 +++++++----- modules/ui/issues.js | 16 ++++-- modules/validations/deprecated_tag.js | 18 ++++-- modules/validations/disconnected_highway.js | 21 ++++--- modules/validations/index.js | 1 + modules/validations/many_deletions.js | 23 +++++--- modules/validations/mapcss_checks.js | 6 +- modules/validations/missing_tag.js | 19 ++++--- modules/validations/old_multipolygon.js | 19 ++++--- modules/validations/tag_suggests_area.js | 18 ++++-- modules/validations/validation_issue.js | 41 ++++++++++++++ 14 files changed, 225 insertions(+), 80 deletions(-) create mode 100644 modules/validations/validation_issue.js diff --git a/dist/locales/en.json b/dist/locales/en.json index 0a04218ef5..8434bdd1a9 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -7579,9 +7579,6 @@ "SPW_PICC": { "name": "SPW(allonie) PICC numerical imagery" }, - "US-TIGER-Roads-2012": { - "name": "TIGER Roads 2012" - }, "US-TIGER-Roads-2014": { "description": "At zoom level 16+, public domain map data from the US Census. At lower zooms, only changes since 2006 minus changes already incorporated into OpenStreetMap", "name": "TIGER Roads 2014" @@ -7590,6 +7587,10 @@ "description": "Yellow = Public domain map data from the US Census. Red = Data not found in OpenStreetMap", "name": "TIGER Roads 2017" }, + "US-TIGER-Roads-2018": { + "description": "Yellow = Public domain map data from the US Census. Red = Data not found in OpenStreetMap", + "name": "TIGER Roads 2018" + }, "US_Forest_Service_roads_overlay": { "description": "Highway: Green casing = unclassified. Brown casing = track. Surface: gravel = light brown fill, Asphalt = black, paved = gray, ground =white, concrete = blue, grass = green. Seasonal = white bars", "name": "U.S. Forest Roads Overlay" @@ -7606,6 +7607,12 @@ }, "name": "UrbIS-Ortho 2017" }, + "UrbISOrtho2018": { + "attribution": { + "text": "Realized by means of Brussels UrbIS®© - Distribution & Copyright CIRB" + }, + "name": "UrbIS-Ortho 2018" + }, "UrbisAdmFR": { "attribution": { "text": "Realized by means of Brussels UrbIS®© - Distribution & Copyright CIRB" @@ -7690,11 +7697,33 @@ "description": "Japan GSI Standard Map. Widely covered.", "name": "Japan GSI Standard Map" }, - "hike_n_bike": { + "helsingborg-orto": { "attribution": { - "text": "© OpenStreetMap contributors" + "text": "© Helsingborg municipality" + }, + "description": "Orthophotos from the municipality of Helsingborg 2016, public domain", + "name": "Helsingborg Orthophoto" + }, + "kalmar-orto-2014": { + "attribution": { + "text": "© Kalmar municipality" + }, + "description": "Orthophotos for the north coast of the municipality of Kalmar 2014", + "name": "Kalmar North Orthophoto 2014" + }, + "kalmar-orto-2016": { + "attribution": { + "text": "© Kalmar municipality" }, - "name": "Hike & Bike" + "description": "Orthophotos for the south coast of the municipality of Kalmar 2016", + "name": "Kalmar South Orthophoto 2016" + }, + "kalmar-orto-2018": { + "attribution": { + "text": "© Kalmar municipality" + }, + "description": "Orthophotos for urban areas of the municipality of Kalmar 2018", + "name": "Kalmar Urban Orthophoto 2018" }, "kelkkareitit": { "attribution": { @@ -7717,6 +7746,20 @@ "description": "Mosaic of Swedish orthophotos from the period 1970–1980. Is under construction.", "name": "Lantmäteriet Historic Orthophoto 1975" }, + "lantmateriet-topowebb": { + "attribution": { + "text": "© Lantmäteriet, CC0" + }, + "description": "Topographic map of Sweden 1:50 000", + "name": "Lantmäteriet Topographic Map" + }, + "linkoping-orto": { + "attribution": { + "text": "© Linköping municipality" + }, + "description": "Orthophotos from the municipality of Linköping 2010, open data", + "name": "Linköping Orthophoto" + }, "mapbox_locator_overlay": { "attribution": { "text": "Terms & Feedback" @@ -7781,6 +7824,13 @@ }, "name": "Stamen Terrain" }, + "stockholm-orto": { + "attribution": { + "text": "© Stockholm municipality, CC0" + }, + "description": "Orthophotos from the municipality of Stockholm 2015, CC0 license", + "name": "Stockholm Orthophoto" + }, "tf-cycle": { "attribution": { "text": "Maps © Thunderforest, Data © OpenStreetMap contributors" diff --git a/modules/core/history.js b/modules/core/history.js index 2699b92009..7db328d11a 100644 --- a/modules/core/history.js +++ b/modules/core/history.js @@ -281,9 +281,12 @@ export function coreHistory(context) { validate: function(changes) { - return _flatten(_map(Validations, function(fn) { - return fn(context)(changes, _stack[_index].graph); - })); + return _flatten(_map( + _filter(Validations, _isFunction), + function(fn) { + return fn(context)(changes, _stack[_index].graph); + } + )); }, diff --git a/modules/services/maprules.js b/modules/services/maprules.js index 9e2f44a4fd..f3b34450d4 100644 --- a/modules/services/maprules.js +++ b/modules/services/maprules.js @@ -206,14 +206,17 @@ export default { } }, // when geometries match and tag matches are present, return a warning... - findWarnings: function (entity, graph, warnings) { + findIssues: function (entity, graph, issues) { if (this.geometryMatches(entity, graph) && this.matches(entity)) { - var type = Object.keys(selector).indexOf('error') > -1 ? 'error' : 'warning'; - warnings.push({ - severity: type, - message: selector[type], - entity: entity - }); + var severity = Object.keys(selector).indexOf('error') > -1 + ? ValidationIssueSeverity.error + : ValidationIssueSeverity.warning; + issues.push(new validationIssue({ + type: ValidationIssueType.map_rule_issue, + severity: severity, + message: selector[severity], + entities: [entity], + })); } } }; diff --git a/modules/ui/commit_warnings.js b/modules/ui/commit_warnings.js index 29ecf44a1e..537288ea3c 100644 --- a/modules/ui/commit_warnings.js +++ b/modules/ui/commit_warnings.js @@ -1,3 +1,5 @@ +import _map from 'lodash-es/map'; + import { t } from '../util/locale'; import { modeSelect } from '../modes'; import { svgIcon } from '../svg'; @@ -11,10 +13,9 @@ export function uiCommitWarnings(context) { function commitWarnings(selection) { - // maybe call these issues now? - var validations = context.issueManager().validate(); + var issues = context.issueManager().validate(); - validations = _reduce(validations, function(validations, val) { + validations = _reduce(issues, function(validations, val) { var severity = val.severity; if (validations.hasOwnProperty(severity)) { validations[severity].push(val); @@ -24,10 +25,10 @@ export function uiCommitWarnings(context) { return validations; }, {}); - _forEach(validations, function(instances, type) { + _forEach(validations, function(instances, severity) { instances = _uniqBy(instances, function(val) { return val.id + '_' + val.message.replace(/\s+/g,''); }); - var section = type + '-section'; - var instanceItem = type + '-item'; + var section = severity + '-section'; + var instanceItem = severity + '-item'; var container = selection.selectAll('.' + section) .data(instances.length ? [0] : []); @@ -41,7 +42,7 @@ export function uiCommitWarnings(context) { containerEnter .append('h3') - .text(type === 'warning' ? t('commit.warnings') : t('commit.errors')); + .text(severity === 'warning' ? t('commit.warnings') : t('commit.errors')); containerEnter .append('ul') @@ -77,31 +78,35 @@ export function uiCommitWarnings(context) { items = itemsEnter .merge(items); + items .on('mouseover', mouseover) .on('mouseout', mouseout) .on('click', warningClick); - function mouseover(d) { - if (d.entity) { + if (d.entities) { context.surface().selectAll( - utilEntityOrMemberSelector([d.entity.id], context.graph()) + utilEntityOrMemberSelector( + _map(d.entities, function(e) { return e.id; }), + context.graph() + ) ).classed('hover', true); } } - function mouseout() { context.surface().selectAll('.hover') .classed('hover', false); } - function warningClick(d) { - if (d.entity) { - context.map().zoomTo(d.entity); - context.enter(modeSelect(context, [d.entity.id])); + if (d.entities) { + context.map().zoomTo(d.entities[0]); + context.enter(modeSelect( + context, + _map(d.entities, function(e) { return e.id; }), + )); } } }); diff --git a/modules/ui/issues.js b/modules/ui/issues.js index 1660aeba56..531bec958b 100644 --- a/modules/ui/issues.js +++ b/modules/ui/issues.js @@ -1,3 +1,4 @@ +import _map from 'lodash-es/map'; import { event as d3_event, select as d3_select @@ -104,9 +105,9 @@ export function uiIssues(context) { var name = 'issues_list'; var changes = context.history().changes(); - var validations = context.history().validate(changes); + var issues = context.history().validate(changes); - /*validations = _reduce(validations, function(validations, val) { + /*validations = _reduce(issues, function(validations, val) { var severity = val.severity; if (validations.hasOwnProperty(severity)) { validations[severity].push(val); @@ -117,7 +118,7 @@ export function uiIssues(context) { }, {});*/ var items = selection.selectAll('li') - .data(validations); + .data(issues); // Exit items.exit() @@ -130,13 +131,18 @@ export function uiIssues(context) { .call(tooltip() .html(true) .title(function(d) { - var tip = d.tooltip; + var tip = d.tooltip ? d.tooltip : ''; return uiTooltipHtml(tip); }) .placement('bottom') ) .on('click', function(d) { - context.enter(modeSelect(context, [d.entity.id])); + if (d.entities) { + context.enter(modeSelect( + context, + _map(d.entities, function(e) { return e.id; }) + )); + } }); var label = enter diff --git a/modules/validations/deprecated_tag.js b/modules/validations/deprecated_tag.js index 3690e714e8..50f8606c0a 100644 --- a/modules/validations/deprecated_tag.js +++ b/modules/validations/deprecated_tag.js @@ -2,27 +2,33 @@ import _isEmpty from 'lodash-es/isEmpty'; import { t } from '../util/locale'; import { utilTagText } from '../util/index'; +import { + ValidationIssueType, + ValidationIssueSeverity, + validationIssue, +} from './validation_issue'; export function validationDeprecatedTag() { var validation = function(changes) { - var warnings = []; + var issues = []; for (var i = 0; i < changes.created.length; i++) { var change = changes.created[i], deprecatedTags = change.deprecatedTags(); if (!_isEmpty(deprecatedTags)) { var tags = utilTagText({ tags: deprecatedTags }); - warnings.push({ - id: 'deprecated_tags', + issues.push(new validationIssue({ + type: ValidationIssueType.deprecated_tags, + severity: ValidationIssueSeverity.warning, message: t('validations.deprecated_tags', { tags: tags }), - entity: change - }); + entities: [change], + })); } } - return warnings; + return issues; }; diff --git a/modules/validations/disconnected_highway.js b/modules/validations/disconnected_highway.js index cc07dba853..1691c1755d 100644 --- a/modules/validations/disconnected_highway.js +++ b/modules/validations/disconnected_highway.js @@ -1,5 +1,10 @@ import { t } from '../util/locale'; import { utilDisplayName } from '../util'; +import { + ValidationIssueType, + ValidationIssueSeverity, + validationIssue, +} from './validation_issue'; export function validationDisconnectedHighway(context) { @@ -24,7 +29,7 @@ export function validationDisconnectedHighway(context) { var validation = function(changes, graph) { - var warnings = []; + var issues = []; for (var i = 0; i < changes.created.length; i++) { var entity = changes.created[i]; @@ -38,16 +43,18 @@ export function validationDisconnectedHighway(context) { entityLabel = utilDisplayType(entity.id) } } - warnings.push({ - id: 'disconnected_highway', - message: t('validations.disconnected_highway', {entityLabel: entityLabel}), + + issues.push(new validationIssue({ + type: ValidationIssueType.disconnected_highway, + severity: ValidationIssueSeverity.error, + message: t('validations.disconnected_highway'), tooltip: t('validations.disconnected_highway_tooltip'), - entity: entity - }); + entities: [entity], + })); } } - return warnings; + return issues; }; diff --git a/modules/validations/index.js b/modules/validations/index.js index 112b38e6cc..ead99f117c 100644 --- a/modules/validations/index.js +++ b/modules/validations/index.js @@ -1,5 +1,6 @@ export { validationDeprecatedTag } from './deprecated_tag'; export { validationDisconnectedHighway } from './disconnected_highway'; +export { ValidationIssueType, ValidationIssueSeverity } from './validation_issue'; export { validationManyDeletions } from './many_deletions'; export { validationMapCSSChecks } from './mapcss_checks'; export { validationMissingTag } from './missing_tag'; diff --git a/modules/validations/many_deletions.js b/modules/validations/many_deletions.js index b0b0c437ea..9624c2baac 100644 --- a/modules/validations/many_deletions.js +++ b/modules/validations/many_deletions.js @@ -1,11 +1,15 @@ import { t } from '../util/locale'; - +import { + ValidationIssueType, + ValidationIssueSeverity, + validationIssue, +} from './validation_issue'; export function validationManyDeletions() { var threshold = 100; var validation = function(changes, graph) { - var warnings = []; + var issues = []; var nodes=0, ways=0, areas=0, relations=0; changes.deleted.forEach(function(c) { @@ -15,14 +19,17 @@ export function validationManyDeletions() { else if (c.type === 'relation') {relations++;} }); if (changes.deleted.length > threshold) { - warnings.push({ - id: 'many_deletions', - message: t('validations.many_deletions', - { n: changes.deleted.length, p: nodes, l: ways, a:areas, r: relations }) - }); + issues.push(new validationIssue({ + type: ValidationIssueType.many_deletions, + severity: ValidationIssueSeverity.warning, + message: t( + 'validations.many_deletions', + { n: changes.deleted.length, p: nodes, l: ways, a:areas, r: relations } + ), + })); } - return warnings; + return issues; }; diff --git a/modules/validations/mapcss_checks.js b/modules/validations/mapcss_checks.js index 13fa6fe2b7..6804279da2 100644 --- a/modules/validations/mapcss_checks.js +++ b/modules/validations/mapcss_checks.js @@ -5,7 +5,7 @@ export function validationMapCSSChecks() { if (!services.maprules) return []; var rules = services.maprules.validationRules(); - var warnings = []; + var issues = []; var createdModified = ['created', 'modified']; for (var i = 0; i < rules.length; i++) { @@ -14,12 +14,12 @@ export function validationMapCSSChecks() { var type = createdModified[j]; var entities = changes[type]; for (var k = 0; k < entities.length; k++) { - rule.findWarnings(entities[k], graph, warnings); + rule.findIssues(entities[k], graph, issues); } } } - return warnings; + return issues; }; return validation; } diff --git a/modules/validations/missing_tag.js b/modules/validations/missing_tag.js index 6785ab247a..53f977e3b2 100644 --- a/modules/validations/missing_tag.js +++ b/modules/validations/missing_tag.js @@ -1,6 +1,10 @@ import _without from 'lodash-es/without'; import { t } from '../util/locale'; - +import { + ValidationIssueType, + ValidationIssueSeverity, + validationIssue, +} from './validation_issue'; export function validationMissingTag() { @@ -12,23 +16,24 @@ export function validationMissingTag() { var validation = function(changes, graph) { var types = ['point', 'line', 'area', 'relation'], - warnings = []; + issues = []; for (var i = 0; i < changes.created.length; i++) { var change = changes.created[i], geometry = change.geometry(graph); if (types.indexOf(geometry) !== -1 && !hasTags(change, graph)) { - warnings.push({ - id: 'missing_tag', + issues.push(new validationIssue({ + type: ValidationIssueType.missing_tag, + severity: ValidationIssueSeverity.error, message: t('validations.untagged_' + geometry), tooltip: t('validations.untagged_' + geometry + '_tooltip'), - entity: change - }); + entities: [change], + })); } } - return warnings; + return issues; }; diff --git a/modules/validations/old_multipolygon.js b/modules/validations/old_multipolygon.js index 6e6c640b85..e1b2da7be5 100644 --- a/modules/validations/old_multipolygon.js +++ b/modules/validations/old_multipolygon.js @@ -1,23 +1,28 @@ import { t } from '../util/locale'; import { osmIsSimpleMultipolygonOuterMember } from '../osm'; - +import { + ValidationIssueType, + ValidationIssueSeverity, + validationIssue, +} from './validation_issue'; export function validationOldMultipolygon() { return function validation(changes, graph) { - var warnings = []; + var issues = []; for (var i = 0; i < changes.created.length; i++) { var entity = changes.created[i]; var parent = osmIsSimpleMultipolygonOuterMember(entity, graph); if (parent) { - warnings.push({ - id: 'old_multipolygon', + issues.push(new validationIssue({ + type: ValidationIssueType.old_multipolygon, + severity: ValidationIssueSeverity.warning, message: t('validations.old_multipolygon'), tooltip: t('validations.old_multipolygon_tooltip'), - entity: parent - }); + entities: [parent], + })); } } - return warnings; + return issues; }; } diff --git a/modules/validations/tag_suggests_area.js b/modules/validations/tag_suggests_area.js index a631f4a797..4ef06f39c6 100644 --- a/modules/validations/tag_suggests_area.js +++ b/modules/validations/tag_suggests_area.js @@ -1,5 +1,10 @@ import _isEmpty from 'lodash-es/isEmpty'; import { t } from '../util/locale'; +import { + ValidationIssueType, + ValidationIssueSeverity, + validationIssue, +} from './validation_issue'; // https://github.com/openstreetmap/josm/blob/mirror/src/org/ @@ -25,22 +30,23 @@ export function validationTagSuggestsArea() { var validation = function(changes, graph) { - var warnings = []; + var issues = []; for (var i = 0; i < changes.created.length; i++) { var change = changes.created[i], geometry = change.geometry(graph), suggestion = (geometry === 'line' ? tagSuggestsArea(change.tags) : undefined); if (suggestion) { - warnings.push({ - id: 'tag_suggests_area', + issues.push(new validationIssue({ + type: ValidationIssueType.tag_suggests_area, + severity: ValidationIssueSeverity.warning, message: t('validations.tag_suggests_area', { tag: suggestion }), - entity: change - }); + entities: [change], + })); } } - return warnings; + return issues; }; diff --git a/modules/validations/validation_issue.js b/modules/validations/validation_issue.js new file mode 100644 index 0000000000..c671a0bd31 --- /dev/null +++ b/modules/validations/validation_issue.js @@ -0,0 +1,41 @@ +import _isObject from 'lodash-es/isObject'; + + +var ValidationIssueType = Object.freeze({ + deprecated_tags: 'deprecated_tags', + disconnected_highway: 'disconnected_highway', + many_deletions: 'many_deletions', + missing_tag: 'missing_tag', + old_multipolygon: 'old_multipolygon', + tag_suggests_area: 'tag_suggests_area', + map_rule_issue: 'map_rule_issue', +}); + + +var ValidationIssueSeverity = Object.freeze({ + warning: 'warning', + error: 'error', +}); + + +export { ValidationIssueType, ValidationIssueSeverity }; + + +export function validationIssue(attrs) { + if (!_isObject(attrs)) throw new Error('Input attrs is not an object'); + if (!attrs.type || !ValidationIssueType.hasOwnProperty(attrs.type)) { + throw new Error('Invalid attrs.type: ' + attrs.type); + } + if (!attrs.severity || !ValidationIssueSeverity.hasOwnProperty(attrs.severity)) { + throw new Error('Invalid attrs.severity: ' + attr.severity); + } + if (!attrs.message) throw new Error('attrs.message is empty'); + + this.type = attrs.type; + this.severity = attrs.severity; + this.message = attrs.message; + this.tooltip = attrs.tooltip; + this.entities = attrs.entities; // expect an array of entities + this.coordinates = attrs.coordinates; // expect an array of [lon, lat] + this.fixes = attrs.fixes; // expect an array of functions for possible fixes +} From 903cb63efe7885f5661bcdfe1b0c68bb434da6e7 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Wed, 19 Dec 2018 10:41:58 -0500 Subject: [PATCH 007/166] Renamed validations to issues in relevant places Made the issue pane get the issues from the issue manager rather than validating itself --- modules/ui/commit_warnings.js | 15 +++++++-------- modules/ui/issues.js | 5 ++--- modules/validations/issueManager.js | 1 - 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/modules/ui/commit_warnings.js b/modules/ui/commit_warnings.js index 29ecf44a1e..24bb2fbba9 100644 --- a/modules/ui/commit_warnings.js +++ b/modules/ui/commit_warnings.js @@ -11,20 +11,19 @@ export function uiCommitWarnings(context) { function commitWarnings(selection) { - // maybe call these issues now? - var validations = context.issueManager().validate(); + var issues = context.issueManager().getIssues(); - validations = _reduce(validations, function(validations, val) { + issues = _reduce(issues, function(issues, val) { var severity = val.severity; - if (validations.hasOwnProperty(severity)) { - validations[severity].push(val); + if (issues.hasOwnProperty(severity)) { + issues[severity].push(val); } else { - validations[severity] = [val]; + issues[severity] = [val]; } - return validations; + return issues; }, {}); - _forEach(validations, function(instances, type) { + _forEach(issues, function(instances, type) { instances = _uniqBy(instances, function(val) { return val.id + '_' + val.message.replace(/\s+/g,''); }); var section = type + '-section'; var instanceItem = type + '-item'; diff --git a/modules/ui/issues.js b/modules/ui/issues.js index 1660aeba56..e122928061 100644 --- a/modules/ui/issues.js +++ b/modules/ui/issues.js @@ -103,8 +103,7 @@ export function uiIssues(context) { var name = 'issues_list'; - var changes = context.history().changes(); - var validations = context.history().validate(changes); + var issues = context.issueManager().getIssues(); /*validations = _reduce(validations, function(validations, val) { var severity = val.severity; @@ -117,7 +116,7 @@ export function uiIssues(context) { }, {});*/ var items = selection.selectAll('li') - .data(validations); + .data(issues); // Exit items.exit() diff --git a/modules/validations/issueManager.js b/modules/validations/issueManager.js index 4bcdc162aa..b290d181fd 100644 --- a/modules/validations/issueManager.js +++ b/modules/validations/issueManager.js @@ -28,7 +28,6 @@ export function IssueManager(context) { var changes = context.history().changes(); issues = context.history().validate(changes); dispatch.call('reload', self, issues); - return issues; }; return utilRebind(self, dispatch, 'on'); From a05e259628c8aed0882f1dcbdb55c688f3c50d8c Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Wed, 19 Dec 2018 11:28:29 -0500 Subject: [PATCH 008/166] Added some initial styling for issues list --- css/80_app.css | 15 +++++++++++++++ modules/ui/issues.js | 4 +++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/css/80_app.css b/css/80_app.css index 296359eb28..27af234452 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -2824,6 +2824,21 @@ div.full-screen > button:hover { padding-bottom: 10px; } +/* Issues List */ + +.issues-list label > span { + white-space: normal; +} +.issues-list li { + height: auto; + color: inherit; +} +.issues-list li.severity-warning { + background: #ffb; +} +.issues-list li.severity-error { + background: #FFD5D4; +} /* Background - Display Options Sliders ------------------------------------------------------- */ diff --git a/modules/ui/issues.js b/modules/ui/issues.js index 8d49dbf2a0..751151f736 100644 --- a/modules/ui/issues.js +++ b/modules/ui/issues.js @@ -126,7 +126,9 @@ export function uiIssues(context) { // Enter var enter = items.enter() .append('li') - .attr('class', 'layer') + .attr('class', function (d) { + return 'layer severity-' + d.severity; + }) .call(tooltip() .html(true) .title(function(d) { From c1e3a7396f2863823ab870ef2fbaf569d4c7c7e4 Mon Sep 17 00:00:00 2001 From: Xiaoming Gao Date: Wed, 19 Dec 2018 12:05:02 -0500 Subject: [PATCH 009/166] filter validations early in history Follow @wonga00's comment in https://github.com/openstreetmap/iD/pull/5627 --- modules/core/history.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/core/history.js b/modules/core/history.js index 7db328d11a..239034733c 100644 --- a/modules/core/history.js +++ b/modules/core/history.js @@ -40,6 +40,7 @@ export function coreHistory(context) { var _stack; var _index; var _tree; + var validations = _filter(Validations, _isFunction); // internal _act, accepts list of actions and eased time @@ -282,7 +283,7 @@ export function coreHistory(context) { validate: function(changes) { return _flatten(_map( - _filter(Validations, _isFunction), + validations, function(fn) { return fn(context)(changes, _stack[_index].graph); } From a95af00b1dd03ef664e74fe9415e996ccfdc47f0 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Wed, 19 Dec 2018 12:40:42 -0500 Subject: [PATCH 010/166] Added icons to issues list --- css/80_app.css | 1 + modules/ui/issues.js | 6 ++++++ svg/iD-sprite/icons/icon-error.svg | 4 ++++ 3 files changed, 11 insertions(+) create mode 100644 svg/iD-sprite/icons/icon-error.svg diff --git a/css/80_app.css b/css/80_app.css index 27af234452..893e9f158d 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -2827,6 +2827,7 @@ div.full-screen > button:hover { /* Issues List */ .issues-list label > span { + display: inline; white-space: normal; } .issues-list li { diff --git a/modules/ui/issues.js b/modules/ui/issues.js index 751151f736..6edf81a4da 100644 --- a/modules/ui/issues.js +++ b/modules/ui/issues.js @@ -149,6 +149,12 @@ export function uiIssues(context) { var label = enter .append('label'); + label + .call( + svgIcon(function(d) { + var iconSuffix = d.severity === 'warning' ? 'alert' : 'error'; + return '#iD-icon-'+iconSuffix; + }, 'pre-text')); /*label .append('input') .attr('type', type) diff --git a/svg/iD-sprite/icons/icon-error.svg b/svg/iD-sprite/icons/icon-error.svg new file mode 100644 index 0000000000..d6d799b23e --- /dev/null +++ b/svg/iD-sprite/icons/icon-error.svg @@ -0,0 +1,4 @@ + + + + From 6a931f9be646c8b07a9be6c93d91ee067dd4a4da Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Wed, 19 Dec 2018 13:07:47 -0500 Subject: [PATCH 011/166] Added colors to issue list icons Corrected icon for warnings Made disconnected_highway issue a warning instead of an error --- css/80_app.css | 10 +++++++++- modules/ui/issues.js | 12 ++++++------ modules/validations/disconnected_highway.js | 2 +- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/css/80_app.css b/css/80_app.css index 893e9f158d..0aff3413d2 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -2825,7 +2825,9 @@ div.full-screen > button:hover { } /* Issues List */ - +.issues-list label { + padding: 5px; +} .issues-list label > span { display: inline; white-space: normal; @@ -2837,9 +2839,15 @@ div.full-screen > button:hover { .issues-list li.severity-warning { background: #ffb; } +.issues-list li.severity-warning .icon { + color: #FFB300 +} .issues-list li.severity-error { background: #FFD5D4; } +.issues-list li.severity-error .icon { + color: #DD1400 +} /* Background - Display Options Sliders ------------------------------------------------------- */ diff --git a/modules/ui/issues.js b/modules/ui/issues.js index 6edf81a4da..00d6631fb7 100644 --- a/modules/ui/issues.js +++ b/modules/ui/issues.js @@ -149,12 +149,12 @@ export function uiIssues(context) { var label = enter .append('label'); - label - .call( - svgIcon(function(d) { - var iconSuffix = d.severity === 'warning' ? 'alert' : 'error'; - return '#iD-icon-'+iconSuffix; - }, 'pre-text')); + label.each(function(d) { + var iconSuffix = d.severity === 'warning' ? 'alert' : 'error'; + d3_select(this) + .call(svgIcon('#iD-icon-' + iconSuffix, 'pre-text')); + }); + /*label .append('input') .attr('type', type) diff --git a/modules/validations/disconnected_highway.js b/modules/validations/disconnected_highway.js index 5de9d585e5..86a2dd0c01 100644 --- a/modules/validations/disconnected_highway.js +++ b/modules/validations/disconnected_highway.js @@ -46,7 +46,7 @@ export function validationDisconnectedHighway(context) { issues.push(new validationIssue({ type: ValidationIssueType.disconnected_highway, - severity: ValidationIssueSeverity.error, + severity: ValidationIssueSeverity.warning, message: t('validations.disconnected_highway', {entityLabel: entityLabel}), tooltip: t('validations.disconnected_highway_tooltip'), entities: [entity], From fb182dca37ba23179b674f612a03853653d82734 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Wed, 19 Dec 2018 13:10:03 -0500 Subject: [PATCH 012/166] Removed unneeded code in the issues pane --- modules/ui/issues.js | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/modules/ui/issues.js b/modules/ui/issues.js index 00d6631fb7..408d359a42 100644 --- a/modules/ui/issues.js +++ b/modules/ui/issues.js @@ -94,16 +94,13 @@ export function uiIssues(context) { items .classed('active', active) .selectAll('input') - .property('checked', active) - .property('indeterminate', function(d) { - return (name === 'feature' && autoHiddenFeature(d)); - }); + .property('checked', active); } function drawIssuesList(selection) { var name = 'issues_list'; - + var issues = context.issueManager().getIssues(); /*validations = _reduce(issues, function(validations, val) { @@ -168,14 +165,6 @@ export function uiIssues(context) { // Update items = items .merge(enter); - - /* items - .classed('active', active) - .selectAll('input') - .property('checked', active) - .property('indeterminate', function(d) { - return (name === 'feature' && autoHiddenFeature(d)); - });*/ } function showsFeatureApplicability(d) { From 59b0252b0e3bd2ea320a78514a849b0ffc48d007 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Wed, 19 Dec 2018 13:22:48 -0500 Subject: [PATCH 013/166] Fixed several linting errors --- modules/ui/commit_warnings.js | 2 +- modules/ui/issues.js | 13 +++---------- modules/validations/disconnected_highway.js | 7 +++++-- modules/validations/validation_issue.js | 2 +- 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/modules/ui/commit_warnings.js b/modules/ui/commit_warnings.js index 93c11cc399..c9f5085302 100644 --- a/modules/ui/commit_warnings.js +++ b/modules/ui/commit_warnings.js @@ -105,7 +105,7 @@ export function uiCommitWarnings(context) { context.map().zoomTo(d.entities[0]); context.enter(modeSelect( context, - _map(d.entities, function(e) { return e.id; }), + _map(d.entities, function(e) { return e.id; }) )); } } diff --git a/modules/ui/issues.js b/modules/ui/issues.js index 408d359a42..204a8dfdd2 100644 --- a/modules/ui/issues.js +++ b/modules/ui/issues.js @@ -7,20 +7,15 @@ import { import { svgIcon } from '../svg'; import { t, textDirection } from '../util/locale'; import { tooltip } from '../util/tooltip'; -import { geoExtent } from '../geo'; -import { modeBrowse, - modeSelect -} from '../modes'; +import { modeSelect } from '../modes'; import { uiBackground } from './background'; import { uiDisclosure } from './disclosure'; import { uiHelp } from './help'; import { uiMapData } from './map_data'; -import { uiSettingsCustomData } from './settings/custom_data'; import { uiTooltipHtml } from './tooltipHtml'; export function uiIssues(context) { var key = t('issues.key'); - var _issuesOptionsContainer = d3_select(null); var _featureApplicabilityList = d3_select(null); var _issuesList = d3_select(null); var _shown = false; @@ -29,7 +24,7 @@ export function uiIssues(context) { var container = selection.selectAll('.issues-options-container') .data([0]); - _issuesOptionsContainer = container.enter() + container = container.enter() .append('div') .attr('class', 'issues-options-container') .merge(container); @@ -37,7 +32,7 @@ export function uiIssues(context) { _featureApplicabilityList = container.selectAll('.feature-applicability-list') .data([0]); - _featureApplicabilityList = container.enter() + _featureApplicabilityList = _featureApplicabilityList.enter() .append('ul') .attr('class', 'layer-list feature-applicability-list') .merge(_featureApplicabilityList); @@ -99,8 +94,6 @@ export function uiIssues(context) { function drawIssuesList(selection) { - var name = 'issues_list'; - var issues = context.issueManager().getIssues(); /*validations = _reduce(issues, function(validations, val) { diff --git a/modules/validations/disconnected_highway.js b/modules/validations/disconnected_highway.js index 86a2dd0c01..6a3cd088f7 100644 --- a/modules/validations/disconnected_highway.js +++ b/modules/validations/disconnected_highway.js @@ -1,5 +1,8 @@ import { t } from '../util/locale'; -import { utilDisplayName } from '../util'; +import { + utilDisplayName, + utilDisplayType +} from '../util'; import { ValidationIssueType, ValidationIssueSeverity, @@ -40,7 +43,7 @@ export function validationDisconnectedHighway(context) { if (preset && preset.name()) { entityLabel = preset.name(); } else { - entityLabel = utilDisplayType(entity.id) + entityLabel = utilDisplayType(entity.id); } } diff --git a/modules/validations/validation_issue.js b/modules/validations/validation_issue.js index c671a0bd31..2b159c6a9d 100644 --- a/modules/validations/validation_issue.js +++ b/modules/validations/validation_issue.js @@ -27,7 +27,7 @@ export function validationIssue(attrs) { throw new Error('Invalid attrs.type: ' + attrs.type); } if (!attrs.severity || !ValidationIssueSeverity.hasOwnProperty(attrs.severity)) { - throw new Error('Invalid attrs.severity: ' + attr.severity); + throw new Error('Invalid attrs.severity: ' + attrs.severity); } if (!attrs.message) throw new Error('attrs.message is empty'); From c11ac6ed896a0ab2046413c08190da66bb4db0ef Mon Sep 17 00:00:00 2001 From: Andrew Wong Date: Wed, 19 Dec 2018 12:53:25 -0500 Subject: [PATCH 014/166] added missing_tag test --- test/index.html | 1 + test/spec/validations/missing_tag.js | 43 ++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 test/spec/validations/missing_tag.js diff --git a/test/index.html b/test/index.html index b455624b35..30f39cf6cc 100644 --- a/test/index.html +++ b/test/index.html @@ -143,6 +143,7 @@ + + + + + + + + - + - + - +