From a6be05966b3a88f49322f163451406b2ec2568a5 Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Tue, 8 Nov 2022 12:09:06 +0100 Subject: [PATCH] improve comboboxes: (#9344) * pressing return/enter doesn't deselect the entity anymore * predefined ("static") field options are always listed in the combobox dropdown, even if taginfo doesn't include them (because of low usage) * (raw) tag values can also be entered for localized strings * static localized strings can be used before taginfo response is received (useful when taginfo is slow or unavailable) * fixes some bugs which can be triggered when taginfo is slow * fixes a bug where the autocomplete dropdown doesn't work properly when tags have "few" values * multiCombo fields can be case-sensitive now --- CHANGELOG.md | 4 ++ modules/services/taginfo.js | 2 +- modules/ui/combobox.js | 34 +++++++++---- modules/ui/fields/combo.js | 75 +++++++++++++++++++--------- modules/ui/sections/preset_fields.js | 12 ----- 5 files changed, 80 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 069a69fcc..b266e171c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,9 @@ _Breaking developer changes, which may affect downstream projects or sites that #### :tada: New Features #### :sparkles: Usability & Accessibility * Trigger context menu by long-presses of non-mouse inputs (touch or stylus) ([#8105]) +* Improve comboboxes ([#9344]): + * (raw) tag values can be also entered (and are autocompleted) when localized strings are available for the respective options + * autocomplete now also works when taginfo service is slow or unavailable #### :white_check_mark: Validation #### :bug: Bugfixes * Fix selection of best background source when starting on a zoomed-out view ([#9325]) @@ -71,6 +74,7 @@ _Breaking developer changes, which may affect downstream projects or sites that [#9337]: https://github.com/openstreetmap/iD/issues/9337 [#9341]: https://github.com/openstreetmap/iD/issues/9341 [#9342]: https://github.com/openstreetmap/iD/issues/9342 +[#9344]: https://github.com/openstreetmap/iD/pull/9344 [#9345]: https://github.com/openstreetmap/iD/issues/9345 [#9347]: https://github.com/openstreetmap/iD/pull/9347 [schema-builder#38]: https://github.com/ideditor/schema-builder/pull/38 diff --git a/modules/services/taginfo.js b/modules/services/taginfo.js index 11eed11dd..170ef14c2 100644 --- a/modules/services/taginfo.js +++ b/modules/services/taginfo.js @@ -79,7 +79,7 @@ function filterKeys(type) { function filterMultikeys(prefix) { return function(d) { // d.key begins with prefix, and d.key contains no additional ':'s - var re = new RegExp('^' + prefix + '(.*)$'); + var re = new RegExp('^' + prefix + '(.*)$', 'i'); var matches = d.key.match(re) || []; return (matches.length === 2 && matches[1].indexOf(':') === -1); }; diff --git a/modules/ui/combobox.js b/modules/ui/combobox.js index b331212f0..956f6beb1 100644 --- a/modules/ui/combobox.js +++ b/modules/ui/combobox.js @@ -34,6 +34,9 @@ export function uiCombobox(context, klass) { cb(_data.filter(function(d) { var terms = d.terms || []; terms.push(d.value); + if (d.key) { + terms.push(d.key); + } return terms.some(function(term) { return term .toString() @@ -179,7 +182,8 @@ export function uiCombobox(context, klass) { input.on('input.combo-input', function() { var start = input.property('selectionStart'); input.node().setSelectionRange(start, start); - input.on('input.combo-input', change); + input.on('input.combo-input', change); // reset event handler + change(false); }); break; @@ -190,6 +194,7 @@ export function uiCombobox(context, klass) { case 13: // ↩ Return d3_event.preventDefault(); d3_event.stopPropagation(); + accept(d3_event); break; case 38: // ↑ Up arrow @@ -218,22 +223,19 @@ export function uiCombobox(context, klass) { case 27: // ⎋ Escape cancel(); break; - - case 13: // ↩ Return - accept(d3_event); - break; } } // Called whenever the input value is changed (e.g. on typing) - function change() { + function change(doAutoComplete) { + if (doAutoComplete === undefined) doAutoComplete = true; fetchComboData(value(), function() { _selected = null; var val = input.property('value'); if (_suggestions.length) { - if (input.property('selectionEnd') === val.length) { + if (doAutoComplete && input.property('selectionEnd') === val.length) { _selected = tryAutocomplete(); } @@ -339,9 +341,19 @@ export function uiCombobox(context, klass) { // Don't autocomplete if user is typing a number - #4935 if (!isNaN(parseFloat(val)) && isFinite(val)) return; + const suggestionValues = []; + _suggestions.forEach(s => { + suggestionValues.push(s.value); + if (s.key && s.key !== s.value) { + suggestionValues.push(s.key); + } + }); + //_suggestions.map(s => s.value) + // .concat(_suggestions.filter(s => s.key !== s.value).map(s => s.key)); + var bestIndex = -1; - for (var i = 0; i < _suggestions.length; i++) { - var suggestion = _suggestions[i].value; + for (var i = 0; i < suggestionValues.length; i++) { + var suggestion = suggestionValues[i]; var compare = _caseSensitive ? suggestion : suggestion.toLowerCase(); // if search string matches suggestion exactly, pick it.. @@ -356,7 +368,7 @@ export function uiCombobox(context, klass) { } if (bestIndex !== -1) { - var bestVal = _suggestions[bestIndex].value; + var bestVal = suggestionValues[bestIndex]; input.property('value', bestVal); input.node().setSelectionRange(val.length, bestVal.length); return bestVal; @@ -397,7 +409,7 @@ export function uiCombobox(context, klass) { .on('mouseenter', _mouseEnterHandler) .on('mouseleave', _mouseLeaveHandler) .merge(options) - .classed('selected', function(d) { return d.value === _selected; }) + .classed('selected', function(d) { return d.value === _selected || d.key === _selected; }) .on('click.combo-option', accept) .order(); diff --git a/modules/ui/fields/combo.js b/modules/ui/fields/combo.js index 4bb9dc175..df9e2a465 100644 --- a/modules/ui/fields/combo.js +++ b/modules/ui/fields/combo.js @@ -31,7 +31,7 @@ export function uiFieldCombo(field, context) { var _snake_case = (field.snake_case || (field.snake_case === undefined)); var _combobox = uiCombobox(context, 'combo-' + field.safeid) .caseSensitive(field.caseSensitive) - .minItems(_isMulti || _isSemi ? 1 : 2); + .minItems(1); var _container = d3_select(null); var _inputWrap = d3_select(null); var _input = d3_select(null); @@ -56,7 +56,7 @@ export function uiFieldCombo(field, context) { function snake(s) { - return s.replace(/\s+/g, '_').toLowerCase(); + return s.replace(/\s+/g, '_'); } function clean(s) { @@ -71,7 +71,7 @@ export function uiFieldCombo(field, context) { function tagValue(dval) { dval = clean(dval || ''); - var found = _comboData.find(function(o) { + var found = getOptions().find(function(o) { return o.key && clean(o.value) === dval; }); if (found) return found.key; @@ -80,7 +80,15 @@ export function uiFieldCombo(field, context) { return 'yes'; } - return (_snake_case ? snake(dval) : dval) || undefined; + if (_snake_case) { + dval = snake(dval); + } + + if (!field.caseSensitive) { + dval = dval.toLowerCase(); + } + + return dval || undefined; } @@ -141,7 +149,6 @@ export function uiFieldCombo(field, context) { if (_showTagInfoSuggestions && services.taginfo) { selection.call(_combobox.fetcher(setTaginfoValues), attachTo); - setStaticValues(); // pre-populate _combobox.data with static values setTaginfoValues('', setPlaceholder); } else { selection.call(_combobox, attachTo); @@ -149,12 +156,11 @@ export function uiFieldCombo(field, context) { } } - - function setStaticValues(callback) { + function getOptions() { var stringsField = field.resolveReference('stringsCrossReference'); - if (!(field.options || stringsField.options)) return; + if (!(field.options || stringsField.options)) return []; - _comboData = (field.options || stringsField.options).map(function(v) { + return (field.options || stringsField.options).map(function(v) { return { key: v, value: stringsField.t('options.' + v, { default: v }), @@ -163,13 +169,26 @@ export function uiFieldCombo(field, context) { klass: stringsField.hasTextForStringId('options.' + v) ? '' : 'raw-option' }; }); + } - _combobox.data(objectDifference(_comboData, _multiData)); + + function setStaticValues(callback, filter) { + _comboData = getOptions(); + + if (filter !== undefined) { + _comboData = _comboData.filter(filter); + } + + _comboData = objectDifference(_comboData, _multiData); + _combobox.data(_comboData); if (callback) callback(_comboData); } function setTaginfoValues(q, callback) { + var queryFilter = d => d.value.toLowerCase().includes(q.toLowerCase()) || d.key.toLowerCase().includes(q.toLowerCase()); + setStaticValues(callback, queryFilter); + var stringsField = field.resolveReference('stringsCrossReference'); var fn = _isMulti ? 'multikeys' : 'values'; var query = (_isMulti ? field.key : '') + q; @@ -189,11 +208,7 @@ export function uiFieldCombo(field, context) { } services.taginfo[fn](params, function(err, data) { - if (err) { - // if service is unavailable: use static values (if any) - setStaticValues(callback); - return; - } + if (err) return; data = data.filter(function(d) { // don't show the fallback value @@ -214,22 +229,29 @@ export function uiFieldCombo(field, context) { }); } + const additionalOptions = (field.options || stringsField.options || []) + .filter(v => !data.some(dv => dv.value === (_isMulti ? field.key + v : v))) + .map(v => ({ value: v })); + // hide the caret if there are no suggestions _container.classed('empty-combobox', data.length === 0); - _comboData = data.map(function(d) { + _comboData = data.concat(additionalOptions).map(function(d) { var k = d.value; if (_isMulti) k = k.replace(field.key, ''); + var isLocalizable = stringsField.hasTextForStringId('options.' + k); var label = stringsField.t('options.' + k, { default: k }); return { key: k, - value: _isMulti ? k : label, + value: label, display: stringsField.t.append('options.' + k, { default: k }), - title: d.title || label, - klass: stringsField.hasTextForStringId('options.' + k) ? '' : 'raw-option' + title: isLocalizable ? k : (d.title !== label ? d.title : ''), + klass: isLocalizable ? '' : 'raw-option' }; }); + _comboData = _comboData.filter(queryFilter); + _comboData = objectDifference(_comboData, _multiData); if (callback) callback(_comboData); }); @@ -270,13 +292,20 @@ export function uiFieldCombo(field, context) { var val; if (_isMulti || _isSemi) { - val = tagValue(utilGetSetValue(_input).replace(/,/g, ';')) || ''; + var vals; + if (_isMulti) { + vals = [tagValue(utilGetSetValue(_input))]; + } else if (_isSemi) { + val = tagValue(utilGetSetValue(_input).replace(/,/g, ';')) || ''; + vals = val.split(';'); + } + vals = vals.filter(Boolean); + + if (!vals.length) return; + _container.classed('active', false); utilGetSetValue(_input, ''); - var vals = val.split(';').filter(Boolean); - if (!vals.length) return; - if (_isMulti) { utilArrayUniq(vals).forEach(function(v) { var key = (field.key || '') + v; diff --git a/modules/ui/sections/preset_fields.js b/modules/ui/sections/preset_fields.js index 34a1b9b1a..c694c4b58 100644 --- a/modules/ui/sections/preset_fields.js +++ b/modules/ui/sections/preset_fields.js @@ -4,7 +4,6 @@ import { presetManager } from '../../presets'; import { t, localizer } from '../../core/localizer'; import { utilArrayIdentical } from '../../util/array'; import { utilArrayUnion, utilRebind } from '../../util'; -import { modeBrowse } from '../../modes/browse'; import { uiField } from '../field'; import { uiFormFields } from '../form_fields'; import { uiSection } from '../section'; @@ -117,17 +116,6 @@ export function uiSectionPresetFields(context) { .state(_state) .klass('grouped-items-area') ); - - - selection.selectAll('.wrap-form-field input') - .on('keydown', function(d3_event) { - // if user presses enter, and combobox is not active, accept edits.. - if (d3_event.keyCode === 13 && // ↩ Return - context.container().select('.combobox').empty()) { - - context.enter(modeBrowse(context)); - } - }); } section.presets = function(val) {