Validator: Add lower issue severity for 'suggestions'

(re: #1623)

This adds the 'suggestion' severity type and styling for the suggestions.
I still need to add them to the Issues Pane, also filtering so the user
can disable them if they want to.

data/core.yaml

@@ -1783,13 +1783,10 @@ en:
           indirect_noedit: "You may not edit indirect restrictions. Instead, edit the nearby direct restriction."
   issues:
     title: Issues
-    list_title: Issues
-    errors:
-      list_title: Errors
-    warnings:
-      list_title: Warnings
-    rules:
-      title: Rules
+    errors: Errors
+    warnings: Warnings
+    suggestions: Suggestions
+    rules: Rules
     user_resolved_issues: Issues resolved by your edits
     warnings_and_errors: Warnings and errors
     no_issues:
@@ -1812,7 +1809,7 @@ en:
       what:
         title: "Check:"
         edited: My Edits
-        all: "Everything"
+        all: Everything
       where:
         title: "Where:"
         visible: In View

data/l10n/core.en.json

@@ -2179,16 +2179,10 @@
     },
     "issues": {
       "title": "Issues",
-      "list_title": "Issues",
-      "errors": {
-        "list_title": "Errors"
-      },
-      "warnings": {
-        "list_title": "Warnings"
-      },
-      "rules": {
-        "title": "Rules"
-      },
+      "errors": "Errors",
+      "warnings": "Warnings",
+      "suggestions": "Suggestions",
+      "rules": "Rules",
       "user_resolved_issues": "Issues resolved by your edits",
       "warnings_and_errors": "Warnings and errors",
       "no_issues": {

modules/core/ValidationSystem.js

@@ -363,15 +363,34 @@ export class ValidationSystem extends AbstractSystem {
    * @param   {Object}  options - see `getIssues`
    * @return  {Object}  result like:
    *   {
-   *     error:    Array of errors,
-   *     warning:  Array of warnings
+   *     error:       Array<ValidationIssue>,
+   *     warning:     Array<ValidationIssue>,
+   *     suggestion:  Array<ValidationIssue>
    *   }
    */
   getIssuesBySeverity(options) {
-    let groups = utilArrayGroupBy(this.getIssues(options), 'severity');
-    groups.error = groups.error ?? [];
-    groups.warning = groups.warning ?? [];
-    return groups;
+    const groups = utilArrayGroupBy(this.getIssues(options), 'severity');
+    return {       // note, we want them in this order
+      error:       groups.error ?? [],
+      warning:     groups.warning ?? [],
+      suggestion:  groups.suggestion ?? []
+    };
+  }
+
+
+  /**
+   * getSeverityIcon
+   * @param   {string}  severity - one of 'error', 'warning', 'suggestion', or 'resolved'
+   * @return  {string}  The name of the icon to use
+   */
+  getSeverityIcon(severity) {
+    const icons = {
+      error: '#rapid-icon-error',
+      warning: '#fas-triangle-exclamation',
+      suggestion: '#fas-circle-arrow-up',
+      resolved: '#rapid-icon-apply'
+    };
+    return icons[severity] || '#fas-triangle-exclamation';
   }
 
 
@@ -470,7 +489,7 @@ export class ValidationSystem extends AbstractSystem {
    * disableRules
    * Disables given validation rules,
    * then reruns the validation so that the user sees something happen in the UI
-   * @param   ruleIDs  Complete set of rules that should be disabled
+   * @param  {Array<string>}  ruleIDs - Complete set of rules that should be disabled
    */
   disableRules(ruleIDs = []) {
     this._disabledRuleIDs = new Set(ruleIDs);

modules/core/lib/ValidationIssue.js

@@ -11,7 +11,7 @@ export class ValidationIssue {
 
     this.type = props.type;                  // required - name of rule that created the issue (e.g. 'missing_tag')
     this.subtype = props.subtype;            // optional - category of the issue within the type (e.g. 'relation_type' under 'missing_tag')
-    this.severity = props.severity;          // required - 'warning' or 'error'
+    this.severity = props.severity;          // required - 'warning', 'error', 'suggestion'
     this.entityIds = props.entityIds;        // required - Array of IDs of entities involved in the issue
     this.loc = props.loc;                    // optional - [lon, lat] to zoom in on to see the issue
     this.data = props.data;                  // optional - object containing extra data for the fixes
@@ -44,8 +44,8 @@ export class ValidationIssue {
     // (bhousel - why is this?  so they can use the latest graph?)
     let fixes = (typeof this.dynamicFixes === 'function') ? this.dynamicFixes() : [];
 
-    // For warnings, create an "ignore" option
-    if (this.severity === 'warning') {
+    // For minor issues, create an "ignore" option
+    if (this.severity !== 'error') {
       const l10n = this.context.systems.l10n;
       const validator = this.context.systems.validator;
 

modules/ui/UiFilterStatus.js

@@ -56,12 +56,12 @@ export class UiFilterStatus {
     const l10n = context.systems.l10n;
 
     // Create/remove wrapper div if necessary
-    let $wrap = $parent.selectAll('.feature-warning')
+    let $wrap = $parent.selectAll('.filter-info')
       .data([0]);
 
     const $$wrap = $wrap.enter()
       .append('div')
-      .attr('class', 'feature-warning');
+      .attr('class', 'filter-info');
 
     const $$chip = $$wrap
       .append('a')

modules/ui/UiValidatorStatus.js

@@ -68,40 +68,35 @@ export class UiValidatorStatus {
 
 
     // Gather info to display
-    const warningsItem = {
-      id: 'warnings',
-      count: 0,
-      iconID: 'rapid-icon-alert',
-      tooltip: this.IssuesTooltip
-    };
-
-    const resolvedItem = {
-      id: 'resolved',
-      count: 0,
-      iconID: 'rapid-icon-apply',
-      tooltip: this.ResolvedTooltip
-    };
-
-    const shownItems = [];
-    const liveIssues = validator.getIssues({
+    const chips = [];
+    const openIssues = validator.getIssuesBySeverity({
       what: storage.getItem('validate-what') ?? 'edited',
       where: storage.getItem('validate-where') ?? 'all'
     });
-    if (liveIssues.length) {
-      warningsItem.count = liveIssues.length;
-      shownItems.push(warningsItem);
+
+    for (const [severity, issues] of Object.entries(openIssues)) {
+      if (issues.length) {
+        chips.push({
+          id: severity,
+          count: issues.length,
+          tooltip: this.IssuesTooltip
+        });
+      }
     }
 
     if (storage.getItem('validate-what') === 'all') {
       const resolvedIssues = validator.getResolvedIssues();
       if (resolvedIssues.length) {
-        resolvedItem.count = resolvedIssues.length;
-        shownItems.push(resolvedItem);
+        chips.push({
+          id: 'resolved',
+          count: resolvedIssues.length,
+          tooltip: this.ResolvedTooltip
+        });
       }
     }
 
     let $chips = $wrap.selectAll('.chip')
-      .data(shownItems, d => d.id);
+      .data(chips, d => d.id);
 
     $chips.exit()
       .remove();
@@ -117,7 +112,7 @@ export class UiValidatorStatus {
         $$chip
           .on('click', this.click)
           .call(d.tooltip)
-          .call(uiIcon(`#${d.iconID}`));
+          .call(uiIcon(validator.getSeverityIcon(d.id)));
 
         $$chip
           .append('span')

modules/ui/commit_warnings.js

@@ -10,10 +10,10 @@ export function uiCommitWarnings(context) {
     const issuesBySeverity = validator
       .getIssuesBySeverity({ what: 'edited', where: 'all', includeDisabledRules: true });
 
-    for (let severity in issuesBySeverity) {
+    for (const severity of ['error', 'warning']) {  // no 'suggestions' here
       let issues = issuesBySeverity[severity];
 
-      if (severity !== 'error') {      // exclude 'fixme' and similar - iD#8603
+      if (severity === 'warning') {      // exclude 'fixme' and similar - iD#8603
         issues = issues.filter(issue => issue.type !== 'help_request');
       }
 
@@ -72,7 +72,7 @@ export function uiCommitWarnings(context) {
         });
 
       buttons
-        .call(uiIcon('#rapid-icon-alert', 'pre-text'));
+        .call(uiIcon(validator.getSeverityIcon(severity), 'pre-text'));
 
       buttons
         .append('strong')

modules/ui/icon.js

@@ -1,9 +1,11 @@
 export function uiIcon(href, klass = '', title = '') {
   const iconID = href.replace('#', '');
+  const prefix = iconID.split('-')[0];
 
   return function render(selection) {
     const classList = ['icon'];
-    if (iconID) classList.push(`icon-${iconID}`);
+    if (prefix) classList.push(`icon-${prefix}`);    // 'icon-fas', 'icon-rapid'
+    if (iconID) classList.push(`icon-${iconID}`);    // 'icon-fas-triangle-exclamation', 'icon-rapid-icon-error'
     if (klass)  classList.push(klass);
 
     const svgEnter = selection.selectAll(`svg.icon-${iconID}`)

modules/ui/sections/entity_issues.js

@@ -23,7 +23,7 @@ export function uiSectionEntityIssues(context) {
   let section = uiSection(context, 'entity-issues')
     .shouldDisplay(() => _issues.length)
     .label(() => {
-      return l10n.t('inspector.title_count', { title: l10n.t('issues.list_title'), count: _issues.length });
+      return l10n.t('inspector.title_count', { title: l10n.t('issues.title'), count: _issues.length });
     })
     .disclosureContent(renderDisclosureContent);
 
@@ -89,9 +89,8 @@ export function uiSectionEntityIssues(context) {
 
     textEnter
       .each((d, i, nodes) => {
-        const which = (d.severity === 'warning') ? 'alert' : 'error';
         d3_select(nodes[i])
-          .call(uiIcon(`#rapid-icon-${which}`, 'issue-icon'));
+          .call(uiIcon(validator.getSeverityIcon(d.severity), 'issue-icon'));
       });
 
     textEnter

modules/ui/sections/validation_issues.js

@@ -13,7 +13,7 @@ const MAX_ISSUES = 1000;
  *  uiSectionValidateIssues
  *  @param  `context`    Global shared application context
  *  @param  `sectionID`  String 'issues-errors' or 'issues-warnings'
- *  @param  `severity`   String 'error' or 'warning'
+ *  @param  `severity`   String 'error', 'warning', or 'suggestion'
  */
 export function uiSectionValidationIssues(context, sectionID, severity) {
   const editor = context.systems.editor;
@@ -34,7 +34,7 @@ export function uiSectionValidationIssues(context, sectionID, severity) {
 
   function sectionLabel() {
     const countText = _issues.length > MAX_ISSUES ? `${MAX_ISSUES}+` : String(_issues.length);
-    const titleText = l10n.t(`issues.${severity}s.list_title`);
+    const titleText = l10n.t(`issues.${severity}s`);
     return l10n.t('inspector.title_count', { title: titleText, count: countText });
   }
 
@@ -74,7 +74,7 @@ export function uiSectionValidationIssues(context, sectionID, severity) {
 
     list = list.enter()
       .append('ul')
-      .attr('class', `layer-list issues-list ${severity}s-list`)
+      .attr('class', `layer-list issues-list ${severity}-list`)
       .merge(list);
 
 
@@ -105,9 +105,8 @@ export function uiSectionValidationIssues(context, sectionID, severity) {
       .append('span')
       .attr('class', 'issue-icon')
       .each((d, i, nodes) => {
-        const which = (d.severity === 'warning') ? 'alert' : 'error';
         d3_select(nodes[i])
-          .call(uiIcon(`#rapid-icon-${which}`));
+          .call(uiIcon(validator.getSeverityIcon(d.severity)));
       });
 
     textEnter

modules/ui/sections/validation_rules.js

@@ -16,7 +16,7 @@ export function uiSectionValidationRules(context) {
 
   const section = uiSection(context, 'issues-rules')
     .disclosureContent(renderDisclosureContent)
-    .label(l10n.t('issues.rules.title'));
+    .label(l10n.t('issues.rules'));
 
 
   let _ruleKeys = validator.getRuleKeys()

modules/validations/curb_nodes.js

@@ -33,7 +33,7 @@ export function validationCurbNodes(context) {
       issues.push(new ValidationIssue(context, {
         type,
         subtype: 'missing_curb_nodes',
-        severity: 'warning',
+        severity: 'suggestion',
         message: () => way ? l10n.t('issues.curb_nodes.message', { feature: l10n.displayLabel(way, graph) }) : 'Way not found',
         reference: showReference,
         entityIds: [wayID],
@@ -279,13 +279,13 @@ export function validationCurbNodes(context) {
   function getIconForCurbNode(tags) {
     let iconID = 'default-icon'; // Default icon
     if (tags.barrier === 'kerb' && tags.kerb === 'flush') {
-        iconID = 'temaki-kerb-flush';
+      iconID = 'temaki-kerb-flush';
     } else if (tags.barrier === 'kerb' && tags.kerb === 'raised') {
-        iconID = 'temaki-kerb-raised';
+      iconID = 'temaki-kerb-raised';
     } else if (tags.barrier === 'kerb' && tags.kerb === 'lowered') {
-        iconID = 'temaki-kerb-lowered';
+      iconID = 'temaki-kerb-lowered';
     } else if (tags.barrier === 'kerb' && tags.kerb === 'unspecified') {
-        iconID = 'temaki-kerb-unspecified';
+      iconID = 'temaki-kerb-unspecified';
     }
     return iconID;
   }

scripts/build_data.js

@@ -94,6 +94,7 @@ function buildDataAsync() {
       // Start with icons we want to use in the UI that aren't tied to other data.
       const icons = new Set([
         'far-star',
+        'fas-circle-arrow-up',
         'fas-arrow-rotate-left',
         'fas-arrow-rotate-right',
         'fas-backward-step',
@@ -105,6 +106,7 @@ function buildDataAsync() {
         'fas-question',
         'fas-star',
         'fas-th-list',
+        'fas-triangle-exclamation',
         'fas-user-cog'
       ]);