From 2e238b821862fb6e5f46a0827f09c2e0c3bb9feb Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Wed, 26 Feb 2020 14:51:32 -0800 Subject: [PATCH] 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 | 14 +++++----- modules/ui/commit.js | 12 +++------ modules/ui/data_editor.js | 6 ++--- modules/ui/disclosure.js | 11 ++++++-- modules/ui/entity_editor.js | 4 +-- modules/ui/section.js | 27 ++++++++++++++++--- modules/ui/sections/raw_tag_editor.js | 21 ++++----------- test/index.html | 3 ++- test/spec/ui/{ => sections}/raw_tag_editor.js | 4 +-- 9 files changed, 57 insertions(+), 45 deletions(-) rename test/spec/ui/{ => sections}/raw_tag_editor.js (96%) diff --git a/css/80_app.css b/css/80_app.css index 725ac2c6e..6a41048e8 100644 --- a/css/80_app.css +++ b/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%; } diff --git a/modules/ui/commit.js b/modules/ui/commit.js index 82ac500fa..0047ee422 100644 --- a/modules/ui/commit.js +++ b/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 ); } } diff --git a/modules/ui/data_editor.js b/modules/ui/data_editor.js index ed7640ce8..e78329f6b 100644 --- a/modules/ui/data_editor.js +++ b/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 diff --git a/modules/ui/disclosure.js b/modules/ui/disclosure.js index 83828379d..0d889663d 100644 --- a/modules/ui/disclosure.js +++ b/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]); diff --git a/modules/ui/entity_editor.js b/modules/ui/entity_editor.js index 9efffea13..ed09f7cf3 100644 --- a/modules/ui/entity_editor.js +++ b/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) ]; diff --git a/modules/ui/section.js b/modules/ui/section.js index b4567c52b..81482b4c0 100644 --- a/modules/ui/section.js +++ b/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); diff --git a/modules/ui/sections/raw_tag_editor.js b/modules/ui/sections/raw_tag_editor.js index e219b9627..d9c55c5ac 100644 --- a/modules/ui/sections/raw_tag_editor.js +++ b/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; diff --git a/test/index.html b/test/index.html index 35498a23e..b6596910f 100644 --- a/test/index.html +++ b/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', diff --git a/test/spec/ui/raw_tag_editor.js b/test/spec/ui/sections/raw_tag_editor.js similarity index 96% rename from test/spec/ui/raw_tag_editor.js rename to test/spec/ui/sections/raw_tag_editor.js index eaa84760b..d45b16efb 100644 --- a/test/spec/ui/raw_tag_editor.js +++ b/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')