Show suggestions on the issue pane

(closes #1623)

This closes #1623, but I'd eventually like to give the user better options for
filtering these out.  It's currently ok becasue only the `curb_nodes` rule
creates suggestions, and users can just turn off that one rule.

Also update the tooltips and strings

css/80_app.css

@@ -4227,7 +4227,6 @@ li.issue-fix-item button:not(.actionable) .fix-icon {
 }
 
 
-
 /* 3D Map
 ------------------------------------------------------- */
 #map3d_container {

data/core.yaml

@@ -1787,8 +1787,8 @@ en:
     warnings: Warnings
     suggestions: Suggestions
     rules: Rules
-    user_resolved_issues: Issues resolved by your edits
-    warnings_and_errors: Warnings and errors
+    open_tooltip: Detected errors, warnings, and suggestions
+    resolved_tooltip: Issues resolved by your edits
     no_issues:
       message:
         everything: Everything looks fine

data/l10n/core.en.json

@@ -2183,8 +2183,8 @@
       "warnings": "Warnings",
       "suggestions": "Suggestions",
       "rules": "Rules",
-      "user_resolved_issues": "Issues resolved by your edits",
-      "warnings_and_errors": "Warnings and errors",
+      "open_tooltip": "Detected errors, warnings, and suggestions",
+      "resolved_tooltip": "Issues resolved by your edits",
       "no_issues": {
         "message": {
           "everything": "Everything looks fine",

modules/ui/UiValidatorStatus.js

@@ -131,8 +131,8 @@ export class UiValidatorStatus {
       });
 
     // localize tooltips
-    this.IssuesTooltip.title(l10n.t('issues.warnings_and_errors'));
-    this.ResolvedTooltip.title(l10n.t('issues.user_resolved_issues'));
+    this.IssuesTooltip.title(l10n.t('issues.open_tooltip'));
+    this.ResolvedTooltip.title(l10n.t('issues.resolved_tooltip'));
   }
 
 

modules/ui/panes/issues.js

@@ -17,8 +17,9 @@ export function uiPaneIssues(context) {
     .sections([
       uiSectionValidationOptions(context),
       uiSectionValidationStatus(context),
-      uiSectionValidationIssues(context, 'issues-errors', 'error'),
-      uiSectionValidationIssues(context, 'issues-warnings', 'warning'),
+      uiSectionValidationIssues(context, 'error'),
+      uiSectionValidationIssues(context, 'warning'),
+      uiSectionValidationIssues(context, 'suggestion'),
       uiSectionValidationRules(context)
     ]);
 }

modules/ui/sections/validation_issues.js

@@ -12,10 +12,9 @@ const MAX_ISSUES = 1000;
 /**
  *  uiSectionValidateIssues
  *  @param  `context`    Global shared application context
- *  @param  `sectionID`  String 'issues-errors' or 'issues-warnings'
  *  @param  `severity`   String 'error', 'warning', or 'suggestion'
  */
-export function uiSectionValidationIssues(context, sectionID, severity) {
+export function uiSectionValidationIssues(context, severity) {
   const editor = context.systems.editor;
   const l10n = context.systems.l10n;
   const map = context.systems.map;
@@ -24,6 +23,7 @@ export function uiSectionValidationIssues(context, sectionID, severity) {
   const validator = context.systems.validator;
   const viewport = context.viewport;
 
+  const sectionID = `issues-${severity}`;
   const section = uiSection(context, sectionID)
     .label(sectionLabel)
     .shouldDisplay(sectionShouldDisplay)

modules/ui/sections/validation_options.js

@@ -1,61 +1,77 @@
+import { selection } from 'd3-selection';
+
 import { uiSection } from '../section.js';
 
 
 export function uiSectionValidationOptions(context) {
   const l10n = context.systems.l10n;
   const storage = context.systems.storage;
   const validator = context.systems.validator;
+  let _$parent;
 
   const section = uiSection(context, 'issues-options')
-    .content(renderContent);
-
+    .content(render);
+
+
+  /**
+   * render
+   * Accepts a parent selection, and renders the content under it.
+   * (The parent selection is required the first time, but can be inferred on subsequent renders)
+   * @param {d3-selection} $parent - A d3-selection to a HTMLElement that this component should render itself into
+   */
+  function render($parent = _$parent) {
+    if ($parent instanceof selection) {
+      _$parent = $parent;
+    } else {
+      return;   // no parent - called too early?
+    }
 
-  function renderContent(selection) {
-    let container = selection.selectAll('.issues-options-container')
+    let $wrap = $parent.selectAll('.issues-options-container')
       .data([0]);
 
-    container = container.enter()
+    $wrap = $wrap.enter()
       .append('div')
       .attr('class', 'issues-options-container')
-      .merge(container);
+      .merge($wrap);
 
     const data = [
       { key: 'what', values: ['edited', 'all'] },
       { key: 'where', values: ['visible', 'all'] }
     ];
 
-    let options = container.selectAll('.issues-option')
+    let $options = $wrap.selectAll('.issues-option')
       .data(data, d => d.key);
 
-    let optionsEnter = options.enter()
+    const $$options = $options.enter()
       .append('div')
       .attr('class', d => `issues-option issues-option-${d.key}`);
 
-    optionsEnter
+    $$options
       .append('div')
       .attr('class', 'issues-option-title')
       .text(d => l10n.t(`issues.options.${d.key}.title`));
 
-    let valuesEnter = optionsEnter.selectAll('label')
+    const $$labels = $$options.selectAll('label')
       .data(d => {
         return d.values.map(val => ({ value: val, key: d.key }) );
       })
       .enter()
       .append('label');
 
-    valuesEnter
+    $$labels
       .append('input')
       .attr('type', 'radio')
       .attr('name', d => `issues-option-${d.key}`)
       .attr('value', d => d.value)
       .property('checked', d => getOptions()[d.key] === d.value)
       .on('change', (d3_event, d) => updateOptionValue(d3_event, d.key, d.value));
 
-    valuesEnter
+    $$labels
       .append('span')
       .text(d => l10n.t(`issues.options.${d.key}.${d.value}`));
   }
 
+
   function getOptions() {
     return {
       what: storage.getItem('validate-what') || 'edited',  // 'all', 'edited'

test/browser/validations/curb_nodes.js

@@ -67,7 +67,7 @@ describe('validationCurbNodes', () => {
 
     const issue = issues[0];
     expect(issue.type).to.eql('curb_nodes');
-    expect(issue.severity).to.eql('warning');
+    expect(issue.severity).to.eql('suggestion');
     expect(issue.entityIds).to.have.lengthOf(1);
     expect(issue.entityIds[0]).to.eql('w-1');
   }
@@ -79,13 +79,13 @@ describe('validationCurbNodes', () => {
   });
 
   it('flags a crossing way and residential street if the street has no curb nodes', () => {
-    createWaysWithOneCrossingPoint({highway: 'footway', footway: 'crossing'}, {highway: 'residential'});
+    createWaysWithOneCrossingPoint({ highway: 'footway', footway: 'crossing' }, { highway: 'residential' });
     const issues = validate();
     verifySingleCurbNodeIssue(issues);
   });
 
   it('flags a crossing way with no curb nodes on a primary road', () => {
-    createWaysWithOneCrossingPoint({highway: 'footway', footway: 'crossing'}, {highway: 'primary'});
+    createWaysWithOneCrossingPoint({ highway: 'footway', footway: 'crossing' }, { highway: 'primary' });
     const issues = validate();
     verifySingleCurbNodeIssue(issues);
   });
@@ -110,14 +110,14 @@ describe('validationCurbNodes', () => {
   });
 
   it('flags a crossing way with a missing curb node on a secondary road', () => {
-    createWaysWithOneCrossingPoint({highway: 'footway', footway: 'crossing'}, {highway: 'secondary'});
+    createWaysWithOneCrossingPoint({ highway: 'footway', footway: 'crossing' }, { highway: 'secondary' });
     const issues = validate();
     verifySingleCurbNodeIssue(issues);
   });
 
   it('flags a crossing way with no curb nodes on a tertiary road', () => {
-    createWaysWithOneCrossingPoint({highway: 'footway', footway: 'crossing'}, {highway: 'tertiary'});
+    createWaysWithOneCrossingPoint({ highway: 'footway', footway: 'crossing' }, { highway: 'tertiary' });
     const issues = validate();
     verifySingleCurbNodeIssue(issues);
   });
-});
+});