Fix JS error on clicking review request

Give different instances of the raw tag editor distinct classes and disclosure states
Correctly expand raw tags section by default when selecting a feature with a fallback preset
Fix raw tag editor tests

css/80_app.css

@@ -2465,17 +2465,17 @@ img.tag-reference-wiki-image {
     position: relative;
     width: 100%;
 }
-.section-raw-tag-editor .tag-reference-body {
+.raw-tag-editor .tag-reference-body {
     width: 100%;
 }
-.section-raw-tag-editor .tag-row.readonly .tag-reference-body {
+.raw-tag-editor .tag-row.readonly .tag-reference-body {
     background: #f6f6f6;
     color: #333;
 }
-.section-raw-tag-editor .tag-row:not(:last-child) .tag-reference-body.expanded {
+.raw-tag-editor .tag-row:not(:last-child) .tag-reference-body.expanded {
     border-bottom: 1px solid #ccc;
 }
-.section-raw-tag-editor .tag-row.readonly .tag-reference-body.expanded {
+.raw-tag-editor .tag-row.readonly .tag-reference-body.expanded {
     border-top: 1px solid #ccc;
 }
 
@@ -2806,11 +2806,11 @@ input.key-trap {
 }
 
 /* custom data editor - no info/delete buttons */
-.data-editor.section-raw-tag-editor .tag-row button {
+.data-editor.raw-tag-editor .tag-row button {
     display: none;
 }
-.data-editor.section-raw-tag-editor .tag-row .key-wrap,
-.data-editor.section-raw-tag-editor .tag-row .value-wrap {
+.data-editor.raw-tag-editor .tag-row .key-wrap,
+.data-editor.raw-tag-editor .tag-row .value-wrap {
     width: 50%;
 }
 

modules/ui/commit.js

@@ -43,8 +43,9 @@ export function uiCommit(context) {
 
     var changesetEditor = uiChangesetEditor(context)
         .on('change', changeTags);
-    var rawTagEditor = uiSectionRawTagEditor(context)
-        .on('change', changeTags);
+    var rawTagEditor = uiSectionRawTagEditor('changeset-tag-editor', context)
+        .on('change', changeTags)
+        .readOnlyTags(readOnlyTags);
     var commitChanges = uiSectionChanges(context);
     var commitWarnings = uiCommitWarnings(context);
 
@@ -393,11 +394,8 @@ export function uiCommit(context) {
             .attr('class', 'modal-section tag-section raw-tag-editor')
             .merge(tagSection);
 
-        var expanded = !tagSection.selectAll('a.hide-toggle.expanded').empty();
         tagSection
             .call(rawTagEditor
-                .expanded(expanded)
-                .readOnlyTags(readOnlyTags)
                 .tags(Object.assign({}, _changeset.tags))   // shallow copy
                 .render
             );
@@ -418,12 +416,10 @@ export function uiCommit(context) {
             var rr = requestReviewInput.property('checked');
             updateChangeset({ review_requested: (rr ? 'yes' : undefined) });
 
-            var expanded = !tagSection.selectAll('a.hide-toggle.expanded').empty();
             tagSection
                 .call(rawTagEditor
-                    .expanded(expanded)
-                    .readOnlyTags(readOnlyTags)
                     .tags(Object.assign({}, _changeset.tags))   // shallow copy
+                    .render
                 );
         }
     }

modules/ui/data_editor.js

@@ -11,7 +11,9 @@ import { uiTooltipHtml } from './tooltipHtml';
 export function uiDataEditor(context) {
     var dataHeader = uiDataHeader();
     var quickLinks = uiQuickLinks();
-    var rawTagEditor = uiSectionRawTagEditor(context);
+    var rawTagEditor = uiSectionRawTagEditor('custom-data-tag-editor', context)
+        .expandedByDefault(true)
+        .readOnlyTags([/./]);
     var _datum;
 
 
@@ -77,8 +79,6 @@ export function uiDataEditor(context) {
             .attr('class', 'raw-tag-editor data-editor')
             .merge(rte)
             .call(rawTagEditor
-                .expanded(true)
-                .readOnlyTags([/./])
                 .tags((_datum && _datum.properties) || {})
                 .state('hover')
                 .render

modules/ui/disclosure.js

@@ -10,14 +10,21 @@ import { textDirection } from '../util/locale';
 
 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 _expanded;
     var _title = utilFunctor('');
     var _updatePreference = true;
     var _content = function () {};
 
 
     var disclosure = function(selection) {
+
+        if (_expanded === undefined || _expanded === null) {
+            // loading _expanded here allows it to be reset by calling `disclosure.expanded(null)`
+
+            var preference = context.storage('disclosure.' + key + '.expanded');
+            _expanded = preference === null ? !!expandedDefault : (preference === 'true');
+        }
+
         var hideToggle = selection.selectAll('.hide-toggle-' + key)
             .data([0]);
 

modules/ui/entity_editor.js

@@ -1,5 +1,5 @@
 import { dispatch as d3_dispatch } from 'd3-dispatch';
-import { event as d3_event, selectAll as d3_selectAll, select as d3_select } from 'd3-selection';
+import { event as d3_event, selectAll as d3_selectAll } from 'd3-selection';
 import deepEqual from 'fast-deep-equal';
 
 import { t, textDirection } from '../util/locale';
@@ -89,7 +89,7 @@ export function uiEntityEditor(context) {
                 }),
                 uiSectionEntityIssues(context),
                 uiSectionPresetFields(context).on('change', changeTags).on('revert', revertTags),
-                uiSectionRawTagEditor(context).on('change', changeTags),
+                uiSectionRawTagEditor('raw-tag-editor', context).on('change', changeTags),
                 uiSectionRawMemberEditor(context),
                 uiSectionRawMembershipEditor(context)
             ];

modules/ui/section.js

@@ -9,19 +9,28 @@ import { utilFunctor } from '../util';
 // Can be labeled and collapsible.
 export function uiSection(id, context) {
 
-    var _title;
-    var _expandedByDefault = utilFunctor(true);
+    var _classes = utilFunctor('');
     var _shouldDisplay;
     var _content;
-    var _disclosureContent;
 
     var _disclosure;
+    var _title;
+    var _expandedByDefault = utilFunctor(true);
+    var _disclosureContent;
+    var _disclosureExpanded;
+
     var _containerSelection = d3_select(null);
 
     var section = {
         id: id
     };
 
+    section.classes = function(val) {
+        if (!arguments.length) return _classes;
+        _classes = utilFunctor(val);
+        return section;
+    };
+
     section.title = function(val) {
         if (!arguments.length) return _title;
         _title = utilFunctor(val);
@@ -52,6 +61,12 @@ export function uiSection(id, context) {
         return section;
     };
 
+    section.disclosureExpanded = function(val) {
+        if (!arguments.length) return _disclosureExpanded;
+        _disclosureExpanded = val;
+        return section;
+    };
+
     // may be called multiple times
     section.render = function(selection) {
 
@@ -62,7 +77,7 @@ export function uiSection(id, context) {
         var sectionEnter = _containerSelection
             .enter()
             .append('div')
-            .attr('class', 'section section-' + id);
+            .attr('class', 'section section-' + id + ' ' + (_classes && _classes() || ''));
 
         _containerSelection = sectionEnter
             .merge(_containerSelection);
@@ -104,6 +119,10 @@ export function uiSection(id, context) {
                     })*/
                     .content(_disclosureContent);
             }
+            if (_disclosureExpanded !== undefined) {
+                _disclosure.expanded(_disclosureExpanded);
+                _disclosureExpanded = undefined;
+            }
             selection
                 .call(_disclosure);
 

modules/ui/sections/raw_tag_editor.js

@@ -10,9 +10,10 @@ import { t } from '../../util/locale';
 import { utilArrayDifference, utilArrayIdentical } from '../../util/array';
 import { utilGetSetValue, utilNoAuto, utilRebind, utilTagDiff } from '../../util';
 
-export function uiSectionRawTagEditor(context) {
+export function uiSectionRawTagEditor(id, context) {
 
-    var section = uiSection('raw-tag-editor', context)
+    var section = uiSection(id, context)
+        .classes('raw-tag-editor')
         .title(function() {
             var count = Object.keys(_tags).filter(function(d) { return d; }).length;
             return t('inspector.tags_count', { count: count });
@@ -32,8 +33,6 @@ export function uiSectionRawTagEditor(context) {
     // the keys in the order we want them to display
     var _orderedKeys = [];
     var _showBlank = false;
-    var _updatePreference = true;
-    var _expanded = false;
     var _pendingChange = null;
     var _state;
     var _presets;
@@ -576,11 +575,9 @@ export function uiSectionRawTagEditor(context) {
         if (!arguments.length) return _presets;
         _presets = val;
         if (_presets && _presets.length && _presets[0].isFallback()) {
-            _expanded = true;
-            _updatePreference = false;
+            section.disclosureExpanded(true);
         } else {
-            _expanded = undefined;
-            _updatePreference = true;
+            section.disclosureExpanded(null);
         }
         return section;
     };
@@ -603,14 +600,6 @@ export function uiSectionRawTagEditor(context) {
     };
 
 
-    section.expanded = function(val) {
-        if (!arguments.length) return _expanded;
-        _expanded = val;
-        _updatePreference = false;
-        return section;
-    };
-
-
     // pass an array of regular expressions to test against the tag key
     section.readOnlyTags = function(val) {
         if (!arguments.length) return _readOnlyTags;

test/index.html

@@ -135,7 +135,8 @@
       'spec/ui/flash.js',
       'spec/ui/inspector.js',
       'spec/ui/modal.js',
-      'spec/ui/raw_tag_editor.js',
+
+      'spec/ui/sections/raw_tag_editor.js',
 
       'spec/ui/fields/access.js',
       'spec/ui/fields/localized.js',

test/spec/ui/sections/raw_tag_editor.js

@@ -2,11 +2,11 @@ describe('iD.uiSectionRawTagEditor', function() {
     var taglist, element, entity, context;
 
     function render(tags) {
-        taglist = iD.uiSectionRawTagEditor(context)
+        taglist = iD.uiSectionRawTagEditor('raw-tag-editor', context)
             .entityIDs([entity.id])
             .presets([{isFallback: function() { return false; }}])
             .tags(tags)
-            .expanded(true);
+            .expandedByDefault(true);
 
         element = d3.select('body')
             .append('div')