From e42bc34e4b1ae7867c71c5b2f3c0d24385ffed9d Mon Sep 17 00:00:00 2001
From: Quincy Morgan <quincy.morgan@icloud.com>
Date: Wed, 19 Feb 2020 17:28:51 -0800
Subject: [PATCH] Move validation warnings and errors to their own section
 objects Allow function parameters for disclosure titles

---
 modules/ui/disclosure.js                 |   7 +-
 modules/ui/panes/issues.js               | 288 +++--------------------
 modules/ui/section.js                    |   8 +-
 modules/ui/sections/validation_issues.js | 238 +++++++++++++++++++
 modules/ui/sections/validation_rules.js  |   2 +-
 5 files changed, 286 insertions(+), 257 deletions(-)
 create mode 100644 modules/ui/sections/validation_issues.js

diff --git a/modules/ui/disclosure.js b/modules/ui/disclosure.js
index 17392d856..83828379d 100644
--- a/modules/ui/disclosure.js
+++ b/modules/ui/disclosure.js
@@ -2,6 +2,7 @@ import { dispatch as d3_dispatch } from 'd3-dispatch';
 import { event as d3_event } from 'd3-selection';
 
 import { svgIcon } from '../svg/icon';
+import { utilFunctor } from '../util';
 import { utilRebind } from '../util/rebind';
 import { uiToggle } from './toggle';
 import { textDirection } from '../util/locale';
@@ -11,7 +12,7 @@ export function uiDisclosure(context, key, expandedDefault) {
     var dispatch = d3_dispatch('toggled');
     var _preference = (context.storage('disclosure.' + key + '.expanded'));
     var _expanded = (_preference === null ? !!expandedDefault : (_preference === 'true'));
-    var _title;
+    var _title = utilFunctor('');
     var _updatePreference = true;
     var _content = function () {};
 
@@ -40,7 +41,7 @@ export function uiDisclosure(context, key, expandedDefault) {
             .classed('expanded', _expanded);
 
         hideToggle.selectAll('.hide-toggle-text')
-            .text(_title);
+            .text(_title());
 
         hideToggle.selectAll('.hide-toggle-icon')
             .attr('xlink:href', _expanded ? '#iD-icon-down'
@@ -96,7 +97,7 @@ export function uiDisclosure(context, key, expandedDefault) {
 
     disclosure.title = function(val) {
         if (!arguments.length) return _title;
-        _title = val;
+        _title = utilFunctor(val);
         return disclosure;
     };
 
diff --git a/modules/ui/panes/issues.js b/modules/ui/panes/issues.js
index 9088697fd..84753b9b0 100644
--- a/modules/ui/panes/issues.js
+++ b/modules/ui/panes/issues.js
@@ -1,208 +1,40 @@
 import _debounce from 'lodash-es/debounce';
-
-import { event as d3_event, select as d3_select } from 'd3-selection';
+import { event as d3_event } from 'd3-selection';
 
 import { t } from '../../util/locale';
-
-//import { actionNoop } from '../actions/noop';
-import { geoSphericalDistance } from '../../geo';
 import { svgIcon } from '../../svg/icon';
-import { uiDisclosure } from '../disclosure';
-import { utilHighlightEntities } from '../../util';
 import { uiPane } from '../pane';
-import { uiValidationRules } from '../sections/validation_rules';
+import { uiSectionValidationIssues } from '../sections/validation_issues';
+import { uiSectionValidationRules } from '../sections/validation_rules';
 
 
 export function uiPaneIssues(context) {
 
-    var _errorsSelection = d3_select(null);
-    var _warningsSelection = d3_select(null);
+    var _validationRules = uiSectionValidationRules(context);
+    var _validationErrors = uiSectionValidationIssues('issues-errors', 'error', context);
+    var _validationWarnings = uiSectionValidationIssues('issues-warnings', 'warning', context);
 
-    var _rulesListContainer = d3_select(null);
+    function getOptions() {
+        return {
+            what: context.storage('validate-what') || 'edited',    // 'all', 'edited'
+            where: context.storage('validate-where') || 'all'  // 'all', 'visible'
+        };
+    }
 
-    var _validationRules = uiValidationRules(context);
-
-    var _errors = [];
-    var _warnings = [];
-    var _options = {
-        what: context.storage('validate-what') || 'edited',    // 'all', 'edited'
-        where: context.storage('validate-where') || 'all'  // 'all', 'visible'
-    };
-
-    // listeners
-    context.validator().on('validated.uiIssues',
+    // listen for updates that affect the "no issues" box
+    context.validator().on('validated.uiPaneIssues',
         function() { window.requestIdleCallback(update); }
     );
-    context.map().on('move.uiIssues',
+    context.map().on('move.uiPaneIssues',
         _debounce(function() { window.requestIdleCallback(update); }, 1000)
     );
 
 
-    function renderErrorsList(selection) {
-        _errorsSelection = selection
-            .call(drawIssuesList, 'errors', _errors);
-    }
-
-
-    function renderWarningsList(selection) {
-        _warningsSelection = selection
-            .call(drawIssuesList, 'warnings', _warnings);
-    }
-
-
-    function drawIssuesList(selection, which, issues) {
-        var list = selection.selectAll('.issues-list')
-            .data([0]);
-
-        list = list.enter()
-            .append('ul')
-            .attr('class', 'layer-list issues-list ' + which + '-list')
-            .merge(list);
-
-
-        var items = list.selectAll('li')
-            .data(issues, function(d) { return d.id; });
-
-        // Exit
-        items.exit()
-            .remove();
-
-        // Enter
-        var itemsEnter = items.enter()
-            .append('li')
-            .attr('class', function (d) { return 'issue severity-' + d.severity; })
-            .on('click', function(d) {
-                context.validator().focusIssue(d);
-            })
-            .on('mouseover', function(d) {
-                utilHighlightEntities(d.entityIds, true, context);
-            })
-            .on('mouseout', function(d) {
-                utilHighlightEntities(d.entityIds, false, context);
-            });
-
-
-        var labelsEnter = itemsEnter
-            .append('div')
-            .attr('class', 'issue-label');
-
-        var textEnter = labelsEnter
-            .append('span')
-            .attr('class', 'issue-text');
-
-        textEnter
-            .append('span')
-            .attr('class', 'issue-icon')
-            .each(function(d) {
-                var iconName = '#iD-icon-' + (d.severity === 'warning' ? 'alert' : 'error');
-                d3_select(this)
-                    .call(svgIcon(iconName));
-            });
-
-        textEnter
-            .append('span')
-            .attr('class', 'issue-message');
-
-        /*
-        labelsEnter
-            .append('span')
-            .attr('class', 'issue-autofix')
-            .each(function(d) {
-                if (!d.autoFix) return;
-
-                d3_select(this)
-                    .append('button')
-                    .attr('title', t('issues.fix_one.title'))
-                    .datum(d.autoFix)  // set button datum to the autofix
-                    .attr('class', 'autofix action')
-                    .on('click', function(d) {
-                        d3_event.preventDefault();
-                        d3_event.stopPropagation();
-
-                        var issuesEntityIDs = d.issue.entityIds;
-                        utilHighlightEntities(issuesEntityIDs.concat(d.entityIds), false, context);
-
-                        context.perform.apply(context, d.autoArgs);
-                        context.validator().validate();
-                    })
-                    .call(svgIcon('#iD-icon-wrench'));
-            });
-        */
-
-        // Update
-        items = items
-            .merge(itemsEnter)
-            .order();
-
-        items.selectAll('.issue-message')
-            .text(function(d) {
-                return d.message(context);
-            });
-
-        /*
-        // autofix
-        var canAutoFix = issues.filter(function(issue) { return issue.autoFix; });
-
-        var autoFixAll = selection.selectAll('.autofix-all')
-            .data(canAutoFix.length ? [0] : []);
-
-        // exit
-        autoFixAll.exit()
-            .remove();
-
-        // enter
-        var autoFixAllEnter = autoFixAll.enter()
-            .insert('div', '.issues-list')
-            .attr('class', 'autofix-all');
-
-        var linkEnter = autoFixAllEnter
-            .append('a')
-            .attr('class', 'autofix-all-link')
-            .attr('href', '#');
-
-        linkEnter
-            .append('span')
-            .attr('class', 'autofix-all-link-text')
-            .text(t('issues.fix_all.title'));
-
-        linkEnter
-            .append('span')
-            .attr('class', 'autofix-all-link-icon')
-            .call(svgIcon('#iD-icon-wrench'));
-
-        if (which === 'warnings') {
-            renderIgnoredIssuesReset(selection);
-        }
-
-        // update
-        autoFixAll = autoFixAll
-            .merge(autoFixAllEnter);
-
-        autoFixAll.selectAll('.autofix-all-link')
-            .on('click', function() {
-                context.pauseChangeDispatch();
-                context.perform(actionNoop());
-                canAutoFix.forEach(function(issue) {
-                    var args = issue.autoFix.autoArgs.slice();  // copy
-                    if (typeof args[args.length - 1] !== 'function') {
-                        args.pop();
-                    }
-                    args.push(t('issues.fix_all.annotation'));
-                    context.replace.apply(context, args);
-                });
-                context.resumeChangeDispatch();
-                context.validator().validate();
-            });
-        */
-    }
-
-
     function updateOptionValue(d, val) {
         if (!val && d3_event && d3_event.target) {
             val = d3_event.target.value;
         }
 
-        _options[d] = val;
         context.storage('validate-' + d, val);
         context.validator().validate();
     }
@@ -246,7 +78,7 @@ export function uiPaneIssues(context) {
             .attr('type', 'radio')
             .attr('name', function(d) { return 'issues-option-' + d.key; })
             .attr('value', function(d) { return d.value; })
-            .property('checked', function(d) { return _options[d.key] === d.value; })
+            .property('checked', function(d) { return getOptions()[d.key] === d.value; })
             .on('change', function(d) { updateOptionValue(d.key, d.value); });
 
         valuesEnter
@@ -314,10 +146,12 @@ export function uiPaneIssues(context) {
 
     function setNoIssuesText() {
 
+        var opts = getOptions();
+
         function checkForHiddenIssues(cases) {
             for (var type in cases) {
-                var opts = cases[type];
-                var hiddenIssues = context.validator().getIssues(opts);
+                var hiddenOpts = cases[type];
+                var hiddenIssues = context.validator().getIssues(hiddenOpts);
                 if (hiddenIssues.length) {
                     issuesPane.selection().select('.issues-none .details')
                         .text(t(
@@ -333,7 +167,7 @@ export function uiPaneIssues(context) {
 
         var messageType;
 
-        if (_options.what === 'edited' && _options.where === 'visible') {
+        if (opts.what === 'edited' && opts.where === 'visible') {
 
             messageType = 'edits_in_view';
 
@@ -347,7 +181,7 @@ export function uiPaneIssues(context) {
                 ignored_issues_elsewhere: { what: 'edited', where: 'all', includeIgnored: 'only' }
             });
 
-        } else if (_options.what === 'edited' && _options.where === 'all') {
+        } else if (opts.what === 'edited' && opts.where === 'all') {
 
             messageType = 'edits';
 
@@ -357,7 +191,7 @@ export function uiPaneIssues(context) {
                 ignored_issues: { what: 'edited', where: 'all', includeIgnored: 'only' }
             });
 
-        } else if (_options.what === 'all' && _options.where === 'visible') {
+        } else if (opts.what === 'all' && opts.where === 'visible') {
 
             messageType = 'everything_in_view';
 
@@ -368,7 +202,7 @@ export function uiPaneIssues(context) {
                 ignored_issues: { what: 'all', where: 'visible', includeIgnored: 'only' },
                 ignored_issues_elsewhere: { what: 'all', where: 'all', includeIgnored: 'only' }
             });
-        } else if (_options.what === 'all' && _options.where === 'all') {
+        } else if (opts.what === 'all' && opts.where === 'all') {
 
             messageType = 'everything';
 
@@ -378,7 +212,7 @@ export function uiPaneIssues(context) {
             });
         }
 
-        if (_options.what === 'edited' && context.history().difference().summary().length === 0) {
+        if (opts.what === 'edited' && context.history().difference().summary().length === 0) {
             messageType = 'no_edits';
         }
 
@@ -389,47 +223,9 @@ export function uiPaneIssues(context) {
 
 
     function update() {
-        var issuesBySeverity = context.validator().getIssuesBySeverity(_options);
+        var issues = context.validator().getIssues(getOptions());
 
-        // sort issues by distance away from the center of the map
-        var center = context.map().center();
-        var graph = context.graph();
-        _errors = issuesBySeverity.error.map(withDistance).sort(byDistance);
-        _warnings = issuesBySeverity.warning.map(withDistance).sort(byDistance);
-
-        // cut off at 1000
-        var errorCount = _errors.length > 1000 ? '1000+' : String(_errors.length);
-        var warningCount = _warnings.length > 1000 ? '1000+' : String(_warnings.length);
-        _errors = _errors.slice(0, 1000);
-        _warnings = _warnings.slice(0, 1000);
-
-
-        issuesPane.selection().selectAll('.issues-errors')
-            .classed('hide', _errors.length === 0);
-
-        if (_errors.length > 0) {
-            issuesPane.selection().selectAll('.hide-toggle-issues_errors .hide-toggle-text')
-                .text(t('issues.errors.list_title', { count: errorCount }));
-            if (!issuesPane.selection().select('.disclosure-wrap-issues_errors').classed('hide')) {
-                _errorsSelection
-                    .call(drawIssuesList, 'errors', _errors);
-            }
-        }
-
-        issuesPane.selection().selectAll('.issues-warnings')
-            .classed('hide', _warnings.length === 0);
-
-        if (_warnings.length > 0) {
-            issuesPane.selection().selectAll('.hide-toggle-issues_warnings .hide-toggle-text')
-                .text(t('issues.warnings.list_title', { count: warningCount }));
-            if (!issuesPane.selection().select('.disclosure-wrap-issues_warnings').classed('hide')) {
-                _warningsSelection
-                    .call(drawIssuesList, 'warnings', _warnings);
-                renderIgnoredIssuesReset(_warningsSelection);
-            }
-        }
-
-        var hasIssues = _warnings.length > 0 || _errors.length > 0;
+        var hasIssues = issues.length > 0;
 
         var issuesNone = issuesPane.selection().select('.issues-none');
         issuesNone.classed('hide', hasIssues);
@@ -438,18 +234,14 @@ export function uiPaneIssues(context) {
             setNoIssuesText();
         }
 
-        _rulesListContainer
+        issuesPane.selection().select('.issues-errors')
+            .call(_validationErrors.render);
+
+        issuesPane.selection().select('.issues-warnings')
+            .call(_validationWarnings.render);
+
+        issuesPane.selection().select('.issues-rules')
             .call(_validationRules.render);
-
-        function byDistance(a, b) {
-            return a.dist - b.dist;
-        }
-
-        function withDistance(issue) {
-            var extent = issue.extent(graph);
-            var dist = extent ? geoSphericalDistance(center, extent.center()) : 0;
-            return Object.assign(issue, { dist: dist });
-        }
     }
 
 
@@ -475,21 +267,15 @@ export function uiPaneIssues(context) {
         // errors
         content
             .append('div')
-            .attr('class', 'issues-errors')
-            .call(uiDisclosure(context, 'issues_errors', true)
-                .content(renderErrorsList)
-            );
+            .attr('class', 'issues-errors');
 
         // warnings
         content
             .append('div')
-            .attr('class', 'issues-warnings')
-            .call(uiDisclosure(context, 'issues_warnings', true)
-                .content(renderWarningsList)
-            );
+            .attr('class', 'issues-warnings');
 
         // rules list
-        _rulesListContainer = content
+        content
             .append('div')
             .attr('class', 'issues-rules');
 
diff --git a/modules/ui/section.js b/modules/ui/section.js
index a5f7372f4..27daa814f 100644
--- a/modules/ui/section.js
+++ b/modules/ui/section.js
@@ -49,13 +49,17 @@ export function uiSection(id, context) {
             .call(section.renderContent);
     };
 
+    section.containerSelection = function() {
+        return _containerSelection;
+    };
+
     // may be called multiple times
     section.renderContent = function(containerSelection) {
 
-        if (section.renderDisclosureContent && _title) {
+        if (section.renderDisclosureContent) {
             if (!_disclosure) {
                 _disclosure = uiDisclosure(context, id.replace(/-/g, '_'), _expandedByDefault)
-                    .title(_title)
+                    .title(_title || '')
                     .content(section.renderDisclosureContent);
             }
             containerSelection
diff --git a/modules/ui/sections/validation_issues.js b/modules/ui/sections/validation_issues.js
new file mode 100644
index 000000000..eb722b5db
--- /dev/null
+++ b/modules/ui/sections/validation_issues.js
@@ -0,0 +1,238 @@
+import _debounce from 'lodash-es/debounce';
+import {
+    select as d3_select
+} from 'd3-selection';
+
+//import { actionNoop } from '../actions/noop';
+import { geoSphericalDistance } from '../../geo';
+import { svgIcon } from '../../svg/icon';
+import { t } from '../../util/locale';
+import { utilHighlightEntities } from '../../util';
+import { uiSection } from '../section';
+
+export function uiSectionValidationIssues(id, severity, context) {
+
+    var _issues = [];
+
+    var section = uiSection(id, context)
+        .title(function() {
+            if (!_issues) return '';
+            var issueCountText = _issues.length > 1000 ? '1000+' : String(_issues.length);
+            return t('issues.' + severity + 's.list_title', { count: issueCountText });
+        });
+
+    function getOptions() {
+        return {
+            what: context.storage('validate-what') || 'edited',
+            where: context.storage('validate-where') || 'all'
+        };
+    }
+
+    // get and cache the issues to display, unordered
+    function reloadIssues() {
+        _issues = context.validator().getIssuesBySeverity(getOptions())[severity];
+    }
+
+    var _parentRenderContent = section.renderContent;
+
+    section.renderContent = function(selection) {
+
+        var isHidden = !_issues || !_issues.length;
+
+        selection.classed('hide', isHidden);
+
+        if (!isHidden) {
+            selection.call(_parentRenderContent);
+        }
+    };
+
+    section.renderDisclosureContent = function(selection) {
+
+        var center = context.map().center();
+        var graph = context.graph();
+
+        // sort issues by distance away from the center of the map
+        var issues = _issues.map(function withDistance(issue) {
+                var extent = issue.extent(graph);
+                var dist = extent ? geoSphericalDistance(center, extent.center()) : 0;
+                return Object.assign(issue, { dist: dist });
+            })
+            .sort(function byDistance(a, b) {
+                return a.dist - b.dist;
+            });
+
+        // cut off at 1000
+        issues = issues.slice(0, 1000);
+
+        //renderIgnoredIssuesReset(_warningsSelection);
+
+        selection
+            .call(drawIssuesList, issues);
+    };
+
+    function drawIssuesList(selection, issues) {
+        var list = selection.selectAll('.issues-list')
+            .data([0]);
+
+        list = list.enter()
+            .append('ul')
+            .attr('class', 'layer-list issues-list ' + severity + 's-list')
+            .merge(list);
+
+
+        var items = list.selectAll('li')
+            .data(issues, function(d) { return d.id; });
+
+        // Exit
+        items.exit()
+            .remove();
+
+        // Enter
+        var itemsEnter = items.enter()
+            .append('li')
+            .attr('class', function (d) { return 'issue severity-' + d.severity; })
+            .on('click', function(d) {
+                context.validator().focusIssue(d);
+            })
+            .on('mouseover', function(d) {
+                utilHighlightEntities(d.entityIds, true, context);
+            })
+            .on('mouseout', function(d) {
+                utilHighlightEntities(d.entityIds, false, context);
+            });
+
+
+        var labelsEnter = itemsEnter
+            .append('div')
+            .attr('class', 'issue-label');
+
+        var textEnter = labelsEnter
+            .append('span')
+            .attr('class', 'issue-text');
+
+        textEnter
+            .append('span')
+            .attr('class', 'issue-icon')
+            .each(function(d) {
+                var iconName = '#iD-icon-' + (d.severity === 'warning' ? 'alert' : 'error');
+                d3_select(this)
+                    .call(svgIcon(iconName));
+            });
+
+        textEnter
+            .append('span')
+            .attr('class', 'issue-message');
+
+        /*
+        labelsEnter
+            .append('span')
+            .attr('class', 'issue-autofix')
+            .each(function(d) {
+                if (!d.autoFix) return;
+
+                d3_select(this)
+                    .append('button')
+                    .attr('title', t('issues.fix_one.title'))
+                    .datum(d.autoFix)  // set button datum to the autofix
+                    .attr('class', 'autofix action')
+                    .on('click', function(d) {
+                        d3_event.preventDefault();
+                        d3_event.stopPropagation();
+
+                        var issuesEntityIDs = d.issue.entityIds;
+                        utilHighlightEntities(issuesEntityIDs.concat(d.entityIds), false, context);
+
+                        context.perform.apply(context, d.autoArgs);
+                        context.validator().validate();
+                    })
+                    .call(svgIcon('#iD-icon-wrench'));
+            });
+        */
+
+        // Update
+        items = items
+            .merge(itemsEnter)
+            .order();
+
+        items.selectAll('.issue-message')
+            .text(function(d) {
+                return d.message(context);
+            });
+
+        /*
+        // autofix
+        var canAutoFix = issues.filter(function(issue) { return issue.autoFix; });
+
+        var autoFixAll = selection.selectAll('.autofix-all')
+            .data(canAutoFix.length ? [0] : []);
+
+        // exit
+        autoFixAll.exit()
+            .remove();
+
+        // enter
+        var autoFixAllEnter = autoFixAll.enter()
+            .insert('div', '.issues-list')
+            .attr('class', 'autofix-all');
+
+        var linkEnter = autoFixAllEnter
+            .append('a')
+            .attr('class', 'autofix-all-link')
+            .attr('href', '#');
+
+        linkEnter
+            .append('span')
+            .attr('class', 'autofix-all-link-text')
+            .text(t('issues.fix_all.title'));
+
+        linkEnter
+            .append('span')
+            .attr('class', 'autofix-all-link-icon')
+            .call(svgIcon('#iD-icon-wrench'));
+
+        if (severity === 'warning') {
+            renderIgnoredIssuesReset(selection);
+        }
+
+        // update
+        autoFixAll = autoFixAll
+            .merge(autoFixAllEnter);
+
+        autoFixAll.selectAll('.autofix-all-link')
+            .on('click', function() {
+                context.pauseChangeDispatch();
+                context.perform(actionNoop());
+                canAutoFix.forEach(function(issue) {
+                    var args = issue.autoFix.autoArgs.slice();  // copy
+                    if (typeof args[args.length - 1] !== 'function') {
+                        args.pop();
+                    }
+                    args.push(t('issues.fix_all.annotation'));
+                    context.replace.apply(context, args);
+                });
+                context.resumeChangeDispatch();
+                context.validator().validate();
+            });
+        */
+    }
+
+    context.validator().on('validated.uiSectionValidationIssues' + id, function() {
+        window.requestIdleCallback(function() {
+            reloadIssues();
+            section.rerenderContent();
+        });
+    });
+
+    context.map().on('move.uiSectionValidationIssues' + id,
+        _debounce(function() {
+            if (getOptions().where === 'visible') {
+                // must refetch issues if they are viewport-dependent
+                reloadIssues();
+            }
+            // always reload list to re-sort-by-distance
+            window.requestIdleCallback(section.rerenderContent);
+        }, 1000)
+    );
+
+    return section;
+}
diff --git a/modules/ui/sections/validation_rules.js b/modules/ui/sections/validation_rules.js
index ca8eb532a..28601222a 100644
--- a/modules/ui/sections/validation_rules.js
+++ b/modules/ui/sections/validation_rules.js
@@ -8,7 +8,7 @@ import { utilGetSetValue, utilNoAuto } from '../../util';
 import { tooltip } from '../../util/tooltip';
 import { uiSection } from '../section';
 
-export function uiValidationRules(context) {
+export function uiSectionValidationRules(context) {
 
     var MINSQUARE = 0;
     var MAXSQUARE = 20;