From 39b3f1df6830a69419b05eab829f8c656a623b90 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Tue, 11 Dec 2018 16:07:00 -0500 Subject: [PATCH] Avoid creating comboboxes (closes #5568) Several strategies in here: - Move uiCombobox() from inside the render function to class variable - Don't render stuff like the raw tag editor when it's collapsed - Don't show as many fields/combos on hover - Don't instantiate fields (like universal/more) until they're actually shown - Bind the combo on enter selection not on update selection --- modules/ui/changeset_editor.js | 4 +-- modules/ui/combobox.js | 6 ++-- modules/ui/entity_editor.js | 12 +++---- modules/ui/field.js | 39 +++++++++++++--------- modules/ui/fields/access.js | 2 +- modules/ui/fields/address.js | 46 +++++++++++++------------- modules/ui/fields/combo.js | 2 +- modules/ui/fields/cycleway.js | 2 +- modules/ui/fields/localized.js | 8 ++--- modules/ui/fields/maxspeed.js | 15 ++++----- modules/ui/fields/wikipedia.js | 51 ++++++++++++++--------------- modules/ui/form_fields.js | 13 ++++++-- modules/ui/preset_editor.js | 5 ++- modules/ui/raw_member_editor.js | 2 +- modules/ui/raw_membership_editor.js | 13 ++++---- modules/ui/raw_tag_editor.js | 30 ++++++++--------- 16 files changed, 135 insertions(+), 115 deletions(-) diff --git a/modules/ui/changeset_editor.js b/modules/ui/changeset_editor.js index 5bdf46e7b..643094a94 100644 --- a/modules/ui/changeset_editor.js +++ b/modules/ui/changeset_editor.js @@ -11,6 +11,7 @@ import { utilRebind, utilTriggerEvent } from '../util'; export function uiChangesetEditor(context) { var dispatch = d3_dispatch('change'); var formFields = uiFormFields(context); + var commentCombo = uiCombobox(context, 'comment').caseSensitive(true); var _fieldsArr; var _tags; var _changesetID; @@ -78,8 +79,7 @@ export function uiChangesetEditor(context) { }); commentField - .call(uiCombobox(context) - .caseSensitive(true) + .call(commentCombo .data(_uniqBy(comments, 'title')) ); }); diff --git a/modules/ui/combobox.js b/modules/ui/combobox.js index 8173021bc..fff2b5443 100644 --- a/modules/ui/combobox.js +++ b/modules/ui/combobox.js @@ -20,7 +20,7 @@ import { utilRebind, utilTriggerEvent } from '../util'; var _comboTimerID; -export function uiCombobox(context) { +export function uiCombobox(context, klass) { var dispatch = d3_dispatch('accept', 'cancel'); var container = context.container(); var _suggestions = []; @@ -40,6 +40,8 @@ export function uiCombobox(context) { }; var combobox = function(input, attachTo) { + if (!input || input.empty()) return; + input .classed('combobox-input', true) .on('focus.typeahead', focus) @@ -98,7 +100,7 @@ export function uiCombobox(context) { container .insert('div', ':first-child') .datum(input.node()) - .attr('class', 'combobox') + .attr('class', 'combobox' + (klass ? ' combobox-' + klass : '')) .style('position', 'absolute') .style('display', 'block') .style('left', '0px') diff --git a/modules/ui/entity_editor.js b/modules/ui/entity_editor.js index ddbb5f071..e55c9b054 100644 --- a/modules/ui/entity_editor.js +++ b/modules/ui/entity_editor.js @@ -33,10 +33,10 @@ export function uiEntityEditor(context) { var _activePreset; var _tagReference; - var presetEditor = uiPresetEditor(context) - .on('change', changeTags); - var rawTagEditor = uiRawTagEditor(context) - .on('change', changeTags); + var presetEditor = uiPresetEditor(context).on('change', changeTags); + var rawTagEditor = uiRawTagEditor(context).on('change', changeTags); + var rawMemberEditor = uiRawMemberEditor(context); + var rawMembershipEditor = uiRawMembershipEditor(context); function entityEditor(selection) { @@ -178,7 +178,7 @@ export function uiEntityEditor(context) { if (entity.type === 'relation') { body.select('.raw-member-editor') .style('display', 'block') - .call(uiRawMemberEditor(context) + .call(rawMemberEditor .entityID(_entityID) ); } else { @@ -187,7 +187,7 @@ export function uiEntityEditor(context) { } body.select('.raw-membership-editor') - .call(uiRawMembershipEditor(context) + .call(rawMembershipEditor .entityID(_entityID) ); diff --git a/modules/ui/field.js b/modules/ui/field.js index 86ae3f9d0..e4859ec58 100644 --- a/modules/ui/field.js +++ b/modules/ui/field.js @@ -33,20 +33,26 @@ export function uiField(context, presetField, entity, options) { var _state = ''; var _tags = {}; + field.keys = field.keys || [field.key]; - // field implementation - field.impl = uiFields[field.type](field, context) - .on('change', function(t, onInput) { - dispatch.call('change', field, t, onInput); - }); - - // if this field cares about the entity, pass it along - if (entity && field.impl.entity) { - field.entityID = entity.id; - field.impl.entity(entity); + // only create the fields that are actually being shown + if (_show && !field.impl) { + createField(); } - field.keys = field.keys || [field.key]; + // field implementation + function createField() { + field.impl = uiFields[field.type](field, context) + .on('change', function(t, onInput) { + dispatch.call('change', field, t, onInput); + }); + + // if this field cares about the entity, pass it along + if (entity && field.impl.entity) { + field.entityID = entity.id; + field.impl.entity(entity); + } + } function isModified() { @@ -192,22 +198,25 @@ export function uiField(context, presetField, entity, options) { }; - field.state = function(_) { + field.state = function(val) { if (!arguments.length) return _state; - _state = _; + _state = val; return field; }; - field.tags = function(_) { + field.tags = function(val) { if (!arguments.length) return _tags; - _tags = _; + _tags = val; return field; }; field.show = function() { _show = true; + if (!field.impl) { + createField(); + } if (field.default && field.key && _tags[field.key] !== field.default) { var t = {}; t[field.key] = field.default; diff --git a/modules/ui/fields/access.js b/modules/ui/fields/access.js index 429754674..f3ef8c33d 100644 --- a/modules/ui/fields/access.js +++ b/modules/ui/fields/access.js @@ -51,7 +51,7 @@ export function uiFieldAccess(field, context) { .call(utilNoAuto) .each(function(d) { d3_select(this) - .call(uiCombobox(context) + .call(uiCombobox(context, 'access-' + d) .data(access.options(d)) ); }); diff --git a/modules/ui/fields/address.js b/modules/ui/fields/address.js index 6f81ced0e..7388a9567 100644 --- a/modules/ui/fields/address.js +++ b/modules/ui/fields/address.js @@ -112,13 +112,19 @@ export function uiFieldAddress(field, context) { } - function initCallback(err, countryCode) { + function countryCallback(err, countryCode) { if (err) return; var addressFormat = _find(dataAddressFormats, function (a) { return a && a.countryCodes && _includes(a.countryCodes, countryCode.toLowerCase()); }) || dataAddressFormats[0]; + var dropdowns = addressFormat.dropdowns || [ + 'city', 'county', 'country', 'district', 'hamlet', + 'neighbourhood', 'place', 'postcode', 'province', + 'quarter', 'state', 'street', 'subdistrict', 'suburb' + ]; + var widths = addressFormat.widths || { housenumber: 1/3, street: 2/3, city: 2/3, state: 1/4, postcode: 1/3 @@ -126,14 +132,14 @@ export function uiFieldAddress(field, context) { function row(r) { // Normalize widths. - var total = _reduce(r, function(sum, field) { - return sum + (widths[field] || 0.5); + var total = _reduce(r, function(sum, key) { + return sum + (widths[key] || 0.5); }, 0); - return r.map(function (field) { + return r.map(function(key) { return { - id: field, - width: (widths[field] || 0.5) / total + id: key, + width: (widths[key] || 0.5) / total }; }); } @@ -155,31 +161,25 @@ export function uiFieldAddress(field, context) { }) .attr('class', function (d) { return 'addr-' + d.id; }) .call(utilNoAuto) + .each(addDropdown) .style('width', function (d) { return d.width * 100 + '%'; }); - // Update - // setup dropdowns for common address tags - var dropdowns = addressFormat.dropdowns || [ - 'city', 'county', 'country', 'district', 'hamlet', - 'neighbourhood', 'place', 'postcode', 'province', - 'quarter', 'state', 'street', 'subdistrict', 'suburb' - ]; + function addDropdown(d) { + if (dropdowns.indexOf(d.id) === -1) return; // not a dropdown - // If fields exist for any of these tags, create dropdowns to pick nearby values.. - dropdowns.forEach(function(tag) { - var nearValues = (tag === 'street') ? getNearStreets - : (tag === 'city') ? getNearCities - : getNearValues; + var nearValues = (d.id === 'street') ? getNearStreets + : (d.id === 'city') ? getNearCities + : getNearValues; - wrap.selectAll('input.addr-' + tag) - .call(uiCombobox(context) + d3_select(this) + .call(uiCombobox(context, 'address-' + d.id) .minItems(1) .fetcher(function(value, callback) { - callback(nearValues('addr:' + tag)); + callback(nearValues('addr:' + d.id)); }) ); - }); + } wrap.selectAll('input') .on('blur', change()) @@ -206,7 +206,7 @@ export function uiFieldAddress(field, context) { if (nominatim && _entity) { var center = _entity.extent(context.graph()).center(); - nominatim.countryCode(center, initCallback); + nominatim.countryCode(center, countryCallback); } } diff --git a/modules/ui/fields/combo.js b/modules/ui/fields/combo.js index fcad59049..869c35516 100644 --- a/modules/ui/fields/combo.js +++ b/modules/ui/fields/combo.js @@ -38,7 +38,7 @@ export function uiFieldCombo(field, context) { var optarray = field.options; var snake_case = (field.snake_case || (field.snake_case === undefined)); var caseSensitive = field.caseSensitive; - var combobox = uiCombobox(context) + var combobox = uiCombobox(context, 'combo-' + field.safeid) .caseSensitive(caseSensitive) .minItems(isMulti || isSemi ? 1 : 2); var container = d3_select(null); diff --git a/modules/ui/fields/cycleway.js b/modules/ui/fields/cycleway.js index 3c117d00b..5aaf9f4b3 100644 --- a/modules/ui/fields/cycleway.js +++ b/modules/ui/fields/cycleway.js @@ -57,7 +57,7 @@ export function uiFieldCycleway(field, context) { .call(utilNoAuto) .each(function(d) { d3_select(this) - .call(uiCombobox(context) + .call(uiCombobox(context, 'cycleway-' + stripcolon(d)) .data(cycleway.options(d)) ); }); diff --git a/modules/ui/fields/localized.js b/modules/ui/fields/localized.js index 227035dc9..7b1b24fc6 100644 --- a/modules/ui/fields/localized.js +++ b/modules/ui/fields/localized.js @@ -29,11 +29,11 @@ export function uiFieldLocalized(field, context) { }); // reuse these combos - var langcombo = uiCombobox(context) + var langCombo = uiCombobox(context, 'localized-lang') .fetcher(fetchLanguages) .minItems(0); - var brandcombo = uiCombobox(context) + var brandCombo = uiCombobox(context, 'localized-brand') .canAutocomplete(false) .minItems(1); @@ -145,7 +145,7 @@ export function uiFieldLocalized(field, context) { // Show the suggestions.. If the user picks one, change the tags.. if (allSuggestions.length && goodSuggestions.length) { input - .call(brandcombo + .call(brandCombo .fetcher(fetchBrandNames(preset, allSuggestions)) .on('accept', function(d) { var entity = context.entity(_entity.id); // get latest @@ -391,7 +391,7 @@ export function uiFieldLocalized(field, context) { .attr('placeholder', t('translate.localized_translation_language')) .on('blur', changeLang) .on('change', changeLang) - .call(langcombo); + .call(langCombo); wrap .append('input') diff --git a/modules/ui/fields/maxspeed.js b/modules/ui/fields/maxspeed.js index 0b990a674..db6017dd7 100644 --- a/modules/ui/fields/maxspeed.js +++ b/modules/ui/fields/maxspeed.js @@ -13,19 +13,18 @@ export function uiFieldMaxspeed(field, context) { var dispatch = d3_dispatch('change'); var unitInput = d3_select(null); var input = d3_select(null); - var combobox; var _entity; var _isImperial; + var speedCombo = uiCombobox(context, 'maxspeed'); + var unitCombo = uiCombobox(context, 'maxspeed-unit') + .data(['km/h', 'mph'].map(comboValues)); + var metricValues = [20, 30, 40, 50, 60, 70, 80, 90, 100, 110, 120]; var imperialValues = [5, 10, 15, 20, 25, 30, 35, 40, 45, 50, 55, 60, 65, 70, 75, 80]; function maxspeed(selection) { - combobox = uiCombobox(context); - - var unitCombobox = uiCombobox(context) - .data(['km/h', 'mph'].map(comboValues)); var wrap = selection.selectAll('.form-field-input-wrap') .data([0]); @@ -45,7 +44,7 @@ export function uiFieldMaxspeed(field, context) { .attr('id', 'preset-input-' + field.safeid) .attr('placeholder', field.placeholder()) .call(utilNoAuto) - .call(combobox) + .call(speedCombo) .merge(input); input @@ -73,7 +72,7 @@ export function uiFieldMaxspeed(field, context) { .append('input') .attr('type', 'text') .attr('class', 'maxspeed-unit') - .call(unitCombobox) + .call(unitCombo) .merge(unitInput); unitInput @@ -91,7 +90,7 @@ export function uiFieldMaxspeed(field, context) { function setSuggestions() { - combobox.data((_isImperial ? imperialValues : metricValues).map(comboValues)); + speedCombo.data((_isImperial ? imperialValues : metricValues).map(comboValues)); utilGetSetValue(unitInput, _isImperial ? 'mph' : 'km/h'); } diff --git a/modules/ui/fields/wikipedia.js b/modules/ui/fields/wikipedia.js index 9688c068b..1b1653107 100644 --- a/modules/ui/fields/wikipedia.js +++ b/modules/ui/fields/wikipedia.js @@ -27,36 +27,35 @@ export function uiFieldWikipedia(field, context) { var _wikiURL = ''; var _entity; + var langCombo = uiCombobox(context, 'wikipedia-lang') + .fetcher(function(value, cb) { + var v = value.toLowerCase(); - function wiki(selection) { - var langcombo = uiCombobox(context) - .fetcher(function(value, cb) { - var v = value.toLowerCase(); + cb(dataWikipedia.filter(function(d) { + return d[0].toLowerCase().indexOf(v) >= 0 || + d[1].toLowerCase().indexOf(v) >= 0 || + d[2].toLowerCase().indexOf(v) >= 0; + }).map(function(d) { + return { value: d[1] }; + })); + }); - cb(dataWikipedia.filter(function(d) { - return d[0].toLowerCase().indexOf(v) >= 0 || - d[1].toLowerCase().indexOf(v) >= 0 || - d[2].toLowerCase().indexOf(v) >= 0; - }).map(function(d) { - return { value: d[1] }; + var titleCombo = uiCombobox(context, 'wikipedia-title') + .fetcher(function(value, cb) { + if (!value && _entity) { + value = context.entity(_entity.id).tags.name || ''; + } + + var searchfn = value.length > 7 ? wikipedia.search : wikipedia.suggestions; + searchfn(language()[2], value, function(query, data) { + cb(data.map(function(d) { + return { value: d }; })); }); - - var titlecombo = uiCombobox(context) - .fetcher(function(value, cb) { - if (!value) { - value = context.entity(_entity.id).tags.name || ''; - } - - var searchfn = value.length > 7 ? wikipedia.search : wikipedia.suggestions; - searchfn(language()[2], value, function(query, data) { - cb(data.map(function(d) { - return { value: d }; - })); - }); - }); + }); + function wiki(selection) { var wrap = selection.selectAll('.form-field-input-wrap') .data([0]); @@ -84,12 +83,12 @@ export function uiFieldWikipedia(field, context) { .attr('class', 'wiki-lang') .attr('placeholder', t('translate.localized_translation_language')) .call(utilNoAuto) + .call(langCombo) .merge(lang); utilGetSetValue(lang, language()[1]); lang - .call(langcombo) .on('blur', changeLang) .on('change', changeLang); @@ -111,10 +110,10 @@ export function uiFieldWikipedia(field, context) { .attr('class', 'wiki-title') .attr('id', 'preset-input-' + field.safeid) .call(utilNoAuto) + .call(titleCombo) .merge(title); title - .call(titlecombo) .on('blur', blur) .on('change', change); diff --git a/modules/ui/form_fields.js b/modules/ui/form_fields.js index eea70fbe9..51984aac8 100644 --- a/modules/ui/form_fields.js +++ b/modules/ui/form_fields.js @@ -6,6 +6,8 @@ import { utilGetSetValue, utilNoAuto } from '../util'; export function uiFormFields(context) { + var moreCombo = uiCombobox(context, 'more-fields').minItems(1); + var _state = ''; var _fieldsArr; @@ -60,7 +62,7 @@ export function uiFormFields(context) { var more = selection.selectAll('.more-fields') - .data((notShown.length > 0) ? [0] : []); + .data((_state === 'hover' || notShown.length === 0) ? [] : [0]); more.exit() .remove(); @@ -95,9 +97,8 @@ export function uiFormFields(context) { } return placeholder.slice(0,3).join(', ') + ((placeholder.length > 3) ? '…' : ''); }) - .call(uiCombobox(context) + .call(moreCombo .data(notShown) - .minItems(1) .on('accept', function (d) { var field = d.field; field.show(); @@ -116,6 +117,12 @@ export function uiFormFields(context) { return formFields; }; + formFields.state = function(val) { + if (!arguments.length) return _state; + _state = val; + return formFields; + }; + return formFields; } diff --git a/modules/ui/preset_editor.js b/modules/ui/preset_editor.js index 9ccd9612a..e688962db 100644 --- a/modules/ui/preset_editor.js +++ b/modules/ui/preset_editor.js @@ -85,7 +85,10 @@ export function uiPresetEditor(context) { selection - .call(formFields.fieldsArr(_fieldsArr), 'inspector-inner fillL3'); + .call(formFields + .fieldsArr(_fieldsArr) + .state(_state), + 'inspector-inner fillL3'); selection.selectAll('.wrap-form-field input') diff --git a/modules/ui/raw_member_editor.js b/modules/ui/raw_member_editor.js index a3b7276d1..4575c0b86 100644 --- a/modules/ui/raw_member_editor.js +++ b/modules/ui/raw_member_editor.js @@ -249,7 +249,7 @@ export function uiRawMemberEditor(context) { return sameletter.concat(other); } - role.call(uiCombobox(context) + role.call(uiCombobox(context, 'member-role') .fetcher(function(role, callback) { var rtype = entity.tags.type; taginfo.roles({ diff --git a/modules/ui/raw_membership_editor.js b/modules/ui/raw_membership_editor.js index 6756dff44..55f8ffd6e 100644 --- a/modules/ui/raw_membership_editor.js +++ b/modules/ui/raw_membership_editor.js @@ -26,6 +26,9 @@ import { utilDisplayName, utilNoAuto, utilHighlightEntity } from '../util'; export function uiRawMembershipEditor(context) { var taginfo = services.taginfo; + var nearbyCombo = uiCombobox(context, 'parent-relation') + .minItems(1) + .fetcher(fetchNearbyRelations); var _entityID; var _showBlank; @@ -81,7 +84,7 @@ export function uiRawMembershipEditor(context) { } - function relations(q) { + function fetchNearbyRelations(q, callback) { var newRelation = { relation: null, value: t('inspector.new_relation') }; var result = []; var graph = context.graph(); @@ -120,7 +123,7 @@ export function uiRawMembershipEditor(context) { }); result.unshift(newRelation); - return result; + callback(result); } @@ -266,9 +269,7 @@ export function uiRawMembershipEditor(context) { .merge(enter); newrow.selectAll('.member-entity-input') - .call(uiCombobox(context) - .minItems(1) - .fetcher(function(value, callback) { callback(relations(value)); }) + .call(nearbyCombo .on('accept', onAccept) ); @@ -313,7 +314,7 @@ export function uiRawMembershipEditor(context) { return sameletter.concat(other); } - role.call(uiCombobox(context) + role.call(uiCombobox(context, 'member-role') .fetcher(function(role, callback) { var rtype = d.relation.tags.type; taginfo.roles({ diff --git a/modules/ui/raw_tag_editor.js b/modules/ui/raw_tag_editor.js index 7954d8f33..0194382af 100644 --- a/modules/ui/raw_tag_editor.js +++ b/modules/ui/raw_tag_editor.js @@ -148,7 +148,7 @@ export function uiRawTagEditor(context) { var key = row.select('input.key'); // propagate bound data to child var value = row.select('input.value'); // propagate bound data to child - if (_entityID && taginfo) { + if (_entityID && taginfo && _state !== 'hover') { bindTypeahead(key, value); } @@ -209,7 +209,7 @@ export function uiRawTagEditor(context) { var geometry = context.geometry(_entityID); - key.call(uiCombobox(context) + key.call(uiCombobox(context, 'tag-key') .fetcher(function(value, callback) { taginfo.keys({ debounce: true, @@ -220,7 +220,7 @@ export function uiRawTagEditor(context) { }); })); - value.call(uiCombobox(context) + value.call(uiCombobox(context, 'tag-value') .fetcher(function(value, callback) { taginfo.values({ debounce: true, @@ -335,16 +335,16 @@ export function uiRawTagEditor(context) { } - rawTagEditor.state = function(_) { + rawTagEditor.state = function(val) { if (!arguments.length) return _state; - _state = _; + _state = val; return rawTagEditor; }; - rawTagEditor.preset = function(_) { + rawTagEditor.preset = function(val) { if (!arguments.length) return _preset; - _preset = _; + _preset = val; if (_preset.isFallback()) { _expanded = true; _updatePreference = false; @@ -356,31 +356,31 @@ export function uiRawTagEditor(context) { }; - rawTagEditor.tags = function(_) { + rawTagEditor.tags = function(val) { if (!arguments.length) return _tags; - _tags = _; + _tags = val; return rawTagEditor; }; - rawTagEditor.entityID = function(_) { + rawTagEditor.entityID = function(val) { if (!arguments.length) return _entityID; - _entityID = _; + _entityID = val; return rawTagEditor; }; - rawTagEditor.expanded = function(_) { + rawTagEditor.expanded = function(val) { if (!arguments.length) return _expanded; - _expanded = _; + _expanded = val; _updatePreference = false; return rawTagEditor; }; - rawTagEditor.readOnlyTags = function(_) { + rawTagEditor.readOnlyTags = function(val) { if (!arguments.length) return _readOnlyTags; - _readOnlyTags = _; + _readOnlyTags = val; return rawTagEditor; };