From 5751e80b93be5f0e4dc12161ad11b53851aa8d95 Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Thu, 24 Nov 2022 20:10:27 +0100 Subject: [PATCH 01/10] replace `parseFloat` with `Number` --- modules/renderer/background_source.js | 4 ++-- modules/ui/feature_list.js | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/renderer/background_source.js b/modules/renderer/background_source.js index 9d92416e6..3aff58fe2 100644 --- a/modules/renderer/background_source.js +++ b/modules/renderer/background_source.js @@ -503,8 +503,8 @@ rendererBackgroundSource.Esri = function(data) { vintage: vintage, source: clean(result.NICE_NAME), description: clean(result.NICE_DESC), - resolution: clean(+parseFloat(result.SRC_RES).toFixed(4)), - accuracy: clean(+parseFloat(result.SRC_ACC).toFixed(4)) + resolution: clean(+Number(result.SRC_RES).toFixed(4)), + accuracy: clean(+Number(result.SRC_ACC).toFixed(4)) }; // append units - meters diff --git a/modules/ui/feature_list.js b/modules/ui/feature_list.js index 34d2cf4ed..c0634f1eb 100644 --- a/modules/ui/feature_list.js +++ b/modules/ui/feature_list.js @@ -126,7 +126,7 @@ export function uiFeatureList(context) { var locationMatch = sexagesimal.pair(q.toUpperCase()) || q.match(/^(-?\d+\.?\d*)\s+(-?\d+\.?\d*)$/); if (locationMatch) { - var loc = [parseFloat(locationMatch[0]), parseFloat(locationMatch[1])]; + var loc = [Number(locationMatch[0]), Number(locationMatch[1])]; result.push({ id: -1, geometry: 'point', @@ -205,8 +205,8 @@ export function uiFeatureList(context) { type: type, name: d.display_name, extent: new geoExtent( - [parseFloat(d.boundingbox[3]), parseFloat(d.boundingbox[0])], - [parseFloat(d.boundingbox[2]), parseFloat(d.boundingbox[1])]) + [Number(d.boundingbox[3]), Number(d.boundingbox[0])], + [Number(d.boundingbox[2]), Number(d.boundingbox[1])]) }); } }); From 50919660569374d0c984dddfd8377ab38d9b84ec Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Fri, 25 Nov 2022 17:30:43 +0100 Subject: [PATCH 02/10] [WIP] add string length indicator and max-length message --- css/80_app.css | 30 ++++++++++++++++++ data/core.yaml | 1 + modules/core/context.js | 25 +++------------ modules/osm/entity.js | 2 +- modules/ui/fields/textarea.js | 30 +++++++++++------- modules/ui/index.js | 1 + modules/ui/length_indicator.js | 58 ++++++++++++++++++++++++++++++++++ modules/util/index.js | 1 + modules/util/util.js | 19 +++++++++++ 9 files changed, 134 insertions(+), 33 deletions(-) create mode 100644 modules/ui/length_indicator.js diff --git a/css/80_app.css b/css/80_app.css index f0d7659ca..83a6b051c 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -2324,6 +2324,36 @@ div.combobox { color: #333; } +.form-field-input-wrap { + position: relative; +} + +.form-field-input-wrap span.length-indicator-wrap { + visibility: hidden; + position: absolute; + top: -5px; + left: 0; + right: 0; +} + +.form-field-input-wrap > textarea:focus + span.length-indicator-wrap, +.form-field-input-wrap > textarea:focus + div.combobox-caret + span.length-indicator-wrap { + visibility: visible; +} + +.form-field-input-wrap span.length-indicator { + display: block; + left: 0; + right: 0; + height: 4px; + background-color: #7092ff; + border-right-style: solid; + border-right-color: lightgray; +} + +.form-field-input-wrap span.length-indicator.limit-reached { + border-right-color: red; +} /* Field Help ------------------------------------------------------- */ diff --git a/data/core.yaml b/data/core.yaml index f98a773c5..481f2b3cc 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -782,6 +782,7 @@ en: foot: ft # abbreviation of inches inch: in + max_length_reached: "Please keep in mind that texts cannot be longer than a maximum of {maxChars} characters. Anything exceeding that length will be truncated." background: title: Background description: Background Settings diff --git a/modules/core/context.js b/modules/core/context.js index ddce5cccc..e72f06a2b 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -19,7 +19,7 @@ import { presetManager } from '../presets'; import { rendererBackground, rendererFeatures, rendererMap, rendererPhotos } from '../renderer'; import { services } from '../services'; import { uiInit } from '../ui/init'; -import { utilKeybinding, utilRebind, utilStringQs, utilUnicodeCharsTruncated } from '../util'; +import { utilKeybinding, utilRebind, utilStringQs, utilCleanOsmString } from '../util'; export function coreContext() { @@ -220,26 +220,9 @@ export function coreContext() { context.maxCharsForTagValue = () => 255; context.maxCharsForRelationRole = () => 255; - function cleanOsmString(val, maxChars) { - // be lenient with input - if (val === undefined || val === null) { - val = ''; - } else { - val = val.toString(); - } - - // remove whitespace - val = val.trim(); - - // use the canonical form of the string - if (val.normalize) val = val.normalize('NFC'); - - // trim to the number of allowed characters - return utilUnicodeCharsTruncated(val, maxChars); - } - context.cleanTagKey = (val) => cleanOsmString(val, context.maxCharsForTagKey()); - context.cleanTagValue = (val) => cleanOsmString(val, context.maxCharsForTagValue()); - context.cleanRelationRole = (val) => cleanOsmString(val, context.maxCharsForRelationRole()); + context.cleanTagKey = (val) => utilCleanOsmString(val, context.maxCharsForTagKey()); + context.cleanTagValue = (val) => utilCleanOsmString(val, context.maxCharsForTagValue()); + context.cleanRelationRole = (val) => utilCleanOsmString(val, context.maxCharsForRelationRole()); /* History */ diff --git a/modules/osm/entity.js b/modules/osm/entity.js index 5636fdc78..155fe6438 100644 --- a/modules/osm/entity.js +++ b/modules/osm/entity.js @@ -156,7 +156,7 @@ osmEntity.prototype = { changed = true; merged[k] = utilUnicodeCharsTruncated( utilArrayUnion(t1.split(/;\s*/), t2.split(/;\s*/)).join(';'), - 255 // avoid exceeding character limit; see also services/osm.js -> maxCharsForTagValue() + 255 // avoid exceeding character limit; see also context.maxCharsForTagValue() ); } } diff --git a/modules/ui/fields/textarea.js b/modules/ui/fields/textarea.js index 31e792052..1824fac34 100644 --- a/modules/ui/fields/textarea.js +++ b/modules/ui/fields/textarea.js @@ -7,11 +7,13 @@ import { utilNoAuto, utilRebind } from '../../util'; +import { uiLengthIndicator } from '../length_indicator'; export function uiFieldTextarea(field, context) { var dispatch = d3_dispatch('change'); var input = d3_select(null); + var _lengthIndicator = uiLengthIndicator(context.maxCharsForTagValue()); var _tags; @@ -22,6 +24,7 @@ export function uiFieldTextarea(field, context) { wrap = wrap.enter() .append('div') .attr('class', 'form-field-input-wrap form-field-input-' + field.type) + .style('position', 'relative') .merge(wrap); input = wrap.selectAll('textarea') @@ -35,22 +38,23 @@ export function uiFieldTextarea(field, context) { .on('blur', change()) .on('change', change()) .merge(input); - } + wrap.call(_lengthIndicator); - function change(onInput) { - return function() { + function change(onInput) { + return function() { - var val = utilGetSetValue(input); - if (!onInput) val = context.cleanTagValue(val); + var val = utilGetSetValue(input); + if (!onInput) val = context.cleanTagValue(val); - // don't override multiple values with blank string - if (!val && Array.isArray(_tags[field.key])) return; + // don't override multiple values with blank string + if (!val && Array.isArray(_tags[field.key])) return; - var t = {}; - t[field.key] = val || undefined; - dispatch.call('change', this, t, onInput); - }; + var t = {}; + t[field.key] = val || undefined; + dispatch.call('change', this, t, onInput); + }; + } } @@ -63,6 +67,10 @@ export function uiFieldTextarea(field, context) { .attr('title', isMixed ? tags[field.key].filter(Boolean).join('\n') : undefined) .attr('placeholder', isMixed ? t('inspector.multiple_values') : (field.placeholder() || t('inspector.unknown'))) .classed('mixed', isMixed); + + if (!isMixed) { + _lengthIndicator.update(tags[field.key]); + } }; diff --git a/modules/ui/index.js b/modules/ui/index.js index c1e7abcd5..2ffbdd11b 100644 --- a/modules/ui/index.js +++ b/modules/ui/index.js @@ -33,6 +33,7 @@ export { uiIssuesInfo } from './issues_info'; export { uiKeepRightDetails } from './keepRight_details'; export { uiKeepRightEditor } from './keepRight_editor'; export { uiKeepRightHeader } from './keepRight_header'; +export { uiLengthIndicator } from './length_indicator'; export { uiLasso } from './lasso'; export { uiLoading } from './loading'; export { uiMapInMap } from './map_in_map'; diff --git a/modules/ui/length_indicator.js b/modules/ui/length_indicator.js new file mode 100644 index 000000000..765e24f28 --- /dev/null +++ b/modules/ui/length_indicator.js @@ -0,0 +1,58 @@ +import { select as d3_select } from 'd3-selection'; + +import { t } from '../core/localizer'; +import { + utilUnicodeCharsCount, + utilCleanOsmString +} from '../util'; +import { uiPopover } from './popover'; + + +export function uiLengthIndicator(maxChars) { + var _wrap = d3_select(null); + var _tooltip = uiPopover('tooltip') + .placement('bottom') + .hasArrow(true) + .content(() => selection => { + selection.text(''); + t.append('inspector.max_length_reached', { maxChars })(selection); + }); + + var lengthIndicator = function(selection) { + _wrap = selection.selectAll('span.length-indicator-wrap').data([0]); + _wrap = _wrap.enter() + .append('span') + .merge(_wrap) + .classed('length-indicator-wrap', true); + selection.call(_tooltip); + }; + + lengthIndicator.update = function(val) { + const strLen = utilUnicodeCharsCount(utilCleanOsmString(val, Number.POSITIVE_INFINITY)); + + var lengthIndicator = _wrap.selectAll('span.length-indicator') + .data([strLen]); + + lengthIndicator = lengthIndicator.enter() + .append('span') + .merge(lengthIndicator) + .classed('length-indicator', true) + .classed('limit-reached', d => d > maxChars) + .style('border-right-width', d => `${Math.abs(maxChars - d) * 2}px`) + .style('margin-right', d => d > maxChars + ? `${(maxChars - d) * 2}px` + : 0) + .style('opacity', d => d > maxChars * 0.8 + ? Math.min(1, (d / maxChars - 0.8) / (1 - 0.8)) + : 0) + .style('pointer-events', d => d > maxChars * 0.8 ? null: 'none'); + + if (strLen > maxChars) { + _tooltip.show(); + } else { + _tooltip.hide(); + } + } + + return lengthIndicator; +} diff --git a/modules/util/index.js b/modules/util/index.js index a1fd6c74f..1aebc3adb 100644 --- a/modules/util/index.js +++ b/modules/util/index.js @@ -54,3 +54,4 @@ export { utilUnicodeCharsCount } from './util'; export { utilUnicodeCharsTruncated } from './util'; export { utilUniqueDomId } from './util'; export { utilWrap } from './util'; +export { utilCleanOsmString } from './util'; diff --git a/modules/util/util.js b/modules/util/util.js index 13af49878..e370f3db5 100644 --- a/modules/util/util.js +++ b/modules/util/util.js @@ -629,3 +629,22 @@ export function utilOldestID(ids) { return ids[oldestIDIndex]; } + +// returns a normalized and truncated string to `maxChars` utf-8 characters +export function utilCleanOsmString(val, maxChars) { + // be lenient with input + if (val === undefined || val === null) { + val = ''; + } else { + val = val.toString(); + } + + // remove whitespace + val = val.trim(); + + // use the canonical form of the string + if (val.normalize) val = val.normalize('NFC'); + + // trim to the number of allowed characters + return utilUnicodeCharsTruncated(val, maxChars); + } From f931d447afd914da7492184a2055351a1b45adc6 Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Fri, 25 Nov 2022 18:05:16 +0100 Subject: [PATCH 03/10] add length indicator to regular text (and localized) fields --- css/80_app.css | 1 + modules/ui/fields/input.js | 7 +++++++ modules/ui/fields/localized.js | 8 ++++++++ modules/ui/fields/textarea.js | 2 +- 4 files changed, 17 insertions(+), 1 deletion(-) diff --git a/css/80_app.css b/css/80_app.css index 83a6b051c..998ed6bba 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -2336,6 +2336,7 @@ div.combobox { right: 0; } +.form-field-input-wrap > input:focus + span.length-indicator-wrap, .form-field-input-wrap > textarea:focus + span.length-indicator-wrap, .form-field-input-wrap > textarea:focus + div.combobox-caret + span.length-indicator-wrap { visibility: visible; diff --git a/modules/ui/fields/input.js b/modules/ui/fields/input.js index db55fa600..92de39beb 100644 --- a/modules/ui/fields/input.js +++ b/modules/ui/fields/input.js @@ -9,6 +9,7 @@ import { t, localizer } from '../../core/localizer'; import { utilGetSetValue, utilNoAuto, utilRebind, utilTotalExtent } from '../../util'; import { svgIcon } from '../../svg/icon'; import { cardinal } from '../../osm/node'; +import { uiLengthIndicator } from '..'; export { uiFieldText as uiFieldColour, @@ -25,6 +26,7 @@ export function uiFieldText(field, context) { var input = d3_select(null); var outlinkButton = d3_select(null); var wrap = d3_select(null); + var _lengthIndicator = uiLengthIndicator(context.maxCharsForTagValue()); var _entityIDs = []; var _tags; var _phoneFormats = {}; @@ -93,6 +95,7 @@ export function uiFieldText(field, context) { .on('blur', change()) .on('change', change()); + wrap.call(_lengthIndicator); if (field.type === 'tel') { updatePhonePlaceholder(); @@ -365,6 +368,10 @@ export function uiFieldText(field, context) { var disabled = !validIdentifierValueForLink(); outlinkButton.classed('disabled', disabled); } + + if (!isMixed) { + _lengthIndicator.update(tags[field.key]); + } }; diff --git a/modules/ui/fields/localized.js b/modules/ui/fields/localized.js index d86fc79c6..1a040f152 100644 --- a/modules/ui/fields/localized.js +++ b/modules/ui/fields/localized.js @@ -10,6 +10,7 @@ import { svgIcon } from '../../svg'; import { uiTooltip } from '../tooltip'; import { uiCombobox } from '../combobox'; import { utilArrayUniq, utilGetSetValue, utilNoAuto, utilRebind, utilTotalExtent, utilUniqueDomId } from '../../util'; +import { uiLengthIndicator } from '../length_indicator'; var _languagesArray = []; @@ -19,6 +20,7 @@ export function uiFieldLocalized(field, context) { var wikipedia = services.wikipedia; var input = d3_select(null); var localizedInputs = d3_select(null); + var _lengthIndicator = uiLengthIndicator(context.maxCharsForTagValue()); var _countryCode; var _tags; @@ -181,6 +183,8 @@ export function uiFieldLocalized(field, context) { .on('blur', change()) .on('change', change()); + wrap.call(_lengthIndicator); + var translateButton = wrap.selectAll('.localized-add') .data([0]); @@ -497,6 +501,10 @@ export function uiFieldLocalized(field, context) { _selection .call(localized); + + if (!isMixed) { + _lengthIndicator.update(tags[field.key]); + } }; diff --git a/modules/ui/fields/textarea.js b/modules/ui/fields/textarea.js index 1824fac34..59fe8b6a2 100644 --- a/modules/ui/fields/textarea.js +++ b/modules/ui/fields/textarea.js @@ -7,7 +7,7 @@ import { utilNoAuto, utilRebind } from '../../util'; -import { uiLengthIndicator } from '../length_indicator'; +import { uiLengthIndicator } from '..'; export function uiFieldTextarea(field, context) { From 2c0ca277bf36346c6ff571d71d12044f80c5006a Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Fri, 25 Nov 2022 18:18:51 +0100 Subject: [PATCH 04/10] add max-length-indicator to combo fields --- css/80_app.css | 1 + data/core.yaml | 2 +- modules/ui/fields/combo.js | 12 +++++++++++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/css/80_app.css b/css/80_app.css index 998ed6bba..49fba507d 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -2338,6 +2338,7 @@ div.combobox { .form-field-input-wrap > input:focus + span.length-indicator-wrap, .form-field-input-wrap > textarea:focus + span.length-indicator-wrap, +.form-field-input-wrap > input:focus + div.combobox-caret + span.length-indicator-wrap, .form-field-input-wrap > textarea:focus + div.combobox-caret + span.length-indicator-wrap { visibility: visible; } diff --git a/data/core.yaml b/data/core.yaml index 481f2b3cc..aaff253bf 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -782,7 +782,7 @@ en: foot: ft # abbreviation of inches inch: in - max_length_reached: "Please keep in mind that texts cannot be longer than a maximum of {maxChars} characters. Anything exceeding that length will be truncated." + max_length_reached: "This string is longer than the maximum length of {maxChars} characters. Anything exceeding that length will be truncated." background: title: Background description: Background Settings diff --git a/modules/ui/fields/combo.js b/modules/ui/fields/combo.js index d27198bdd..27dc5a965 100644 --- a/modules/ui/fields/combo.js +++ b/modules/ui/fields/combo.js @@ -12,6 +12,7 @@ import { svgIcon } from '../../svg/icon'; import { utilKeybinding } from '../../util/keybinding'; import { utilArrayUniq, utilGetSetValue, utilNoAuto, utilRebind, utilTotalExtent, utilUnicodeCharsCount } from '../../util'; +import { uiLengthIndicator } from '../length_indicator'; export { uiFieldCombo as uiFieldManyCombo, @@ -52,6 +53,7 @@ export function uiFieldCombo(field, context) { var _container = d3_select(null); var _inputWrap = d3_select(null); var _input = d3_select(null); + var _lengthIndicator = uiLengthIndicator(context.maxCharsForTagValue()); var _comboData = []; var _multiData = []; var _entityIDs = []; @@ -447,6 +449,8 @@ export function uiFieldCombo(field, context) { .call(initCombo, selection) .merge(_input); + _container.call(_lengthIndicator); + if (_isNetwork) { var extent = combinedEntityExtent(); var countryCode = extent && countryCoder.iso1A2Code(extent.center()); @@ -457,7 +461,9 @@ export function uiFieldCombo(field, context) { .on('change', change) .on('blur', change) .on('input', function() { - updateIcon(utilGetSetValue(_input)); + const val = utilGetSetValue(_input); + updateIcon(val); + _lengthIndicator.update(val); }); _input @@ -688,6 +694,10 @@ export function uiFieldCombo(field, context) { if (!Array.isArray(tags[field.key])) { updateIcon(tags[field.key]); } + + if (!isMixed) { + _lengthIndicator.update(tags[field.key]); + } } }; From 59b10b797458de27fbb1f141c3b0aef69c3ebf24 Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Fri, 25 Nov 2022 18:52:13 +0100 Subject: [PATCH 05/10] add length indicator for semicombo fields as well --- css/80_app.css | 14 +++++++++----- modules/ui/fields/combo.js | 16 ++++++++++++++-- modules/ui/length_indicator.js | 2 +- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/css/80_app.css b/css/80_app.css index 49fba507d..3c08bd04b 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -1717,7 +1717,7 @@ a.hide-toggle { .form-field-input-multicombo .input-wrap { border: 1px solid #ddd; - width: 100px; + width: 180px; } .form-field-input-multicombo input { border: none; @@ -2336,10 +2336,10 @@ div.combobox { right: 0; } -.form-field-input-wrap > input:focus + span.length-indicator-wrap, -.form-field-input-wrap > textarea:focus + span.length-indicator-wrap, -.form-field-input-wrap > input:focus + div.combobox-caret + span.length-indicator-wrap, -.form-field-input-wrap > textarea:focus + div.combobox-caret + span.length-indicator-wrap { +.form-field-input-wrap input:focus + span.length-indicator-wrap, +.form-field-input-wrap textarea:focus + span.length-indicator-wrap, +.form-field-input-wrap input:focus + div.combobox-caret + span.length-indicator-wrap, +.form-field-input-wrap textarea:focus + div.combobox-caret + span.length-indicator-wrap { visibility: visible; } @@ -2357,6 +2357,10 @@ div.combobox { border-right-color: red; } +.tooltip.max-length-warning { + z-index: 10; +} + /* Field Help ------------------------------------------------------- */ .field-help-body { diff --git a/modules/ui/fields/combo.js b/modules/ui/fields/combo.js index 27dc5a965..568cbb5d2 100644 --- a/modules/ui/fields/combo.js +++ b/modules/ui/fields/combo.js @@ -390,6 +390,8 @@ export function uiFieldCombo(field, context) { arr = utilArrayUniq(arr); t[field.key] = arr.length ? arr.join(';') : undefined; + + _lengthIndicator.update(t[field.key]); } dispatch.call('change', this, t); } @@ -449,7 +451,13 @@ export function uiFieldCombo(field, context) { .call(initCombo, selection) .merge(_input); - _container.call(_lengthIndicator); + if (_isSemi) { + _inputWrap.call(_lengthIndicator); + } else if (_isMulti) { + // todo: implement + } else { + _container.call(_lengthIndicator); + } if (_isNetwork) { var extent = combinedEntityExtent(); @@ -461,8 +469,12 @@ export function uiFieldCombo(field, context) { .on('change', change) .on('blur', change) .on('input', function() { - const val = utilGetSetValue(_input); + let val = utilGetSetValue(_input); updateIcon(val); + if (_isSemi && _tags[field.key]) { + // when adding a new value to existing ones + val += ';' + _tags[field.key]; + } _lengthIndicator.update(val); }); diff --git a/modules/ui/length_indicator.js b/modules/ui/length_indicator.js index 765e24f28..345134a9b 100644 --- a/modules/ui/length_indicator.js +++ b/modules/ui/length_indicator.js @@ -10,7 +10,7 @@ import { uiPopover } from './popover'; export function uiLengthIndicator(maxChars) { var _wrap = d3_select(null); - var _tooltip = uiPopover('tooltip') + var _tooltip = uiPopover('tooltip max-length-warning') .placement('bottom') .hasArrow(true) .content(() => selection => { From 6c9ffdc854ff3e9c9cefc5f76e72805b554842c3 Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Sat, 26 Nov 2022 10:28:20 +0100 Subject: [PATCH 06/10] lint --- modules/ui/fields/combo.js | 4 +--- modules/ui/length_indicator.js | 8 ++++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/modules/ui/fields/combo.js b/modules/ui/fields/combo.js index 568cbb5d2..7d3c6ac04 100644 --- a/modules/ui/fields/combo.js +++ b/modules/ui/fields/combo.js @@ -453,9 +453,7 @@ export function uiFieldCombo(field, context) { if (_isSemi) { _inputWrap.call(_lengthIndicator); - } else if (_isMulti) { - // todo: implement - } else { + } else if (!_isMulti) { _container.call(_lengthIndicator); } diff --git a/modules/ui/length_indicator.js b/modules/ui/length_indicator.js index 345134a9b..a7e84857d 100644 --- a/modules/ui/length_indicator.js +++ b/modules/ui/length_indicator.js @@ -30,12 +30,12 @@ export function uiLengthIndicator(maxChars) { lengthIndicator.update = function(val) { const strLen = utilUnicodeCharsCount(utilCleanOsmString(val, Number.POSITIVE_INFINITY)); - var lengthIndicator = _wrap.selectAll('span.length-indicator') + let indicator = _wrap.selectAll('span.length-indicator') .data([strLen]); - lengthIndicator = lengthIndicator.enter() + indicator.enter() .append('span') - .merge(lengthIndicator) + .merge(indicator) .classed('length-indicator', true) .classed('limit-reached', d => d > maxChars) .style('border-right-width', d => `${Math.abs(maxChars - d) * 2}px`) @@ -52,7 +52,7 @@ export function uiLengthIndicator(maxChars) { } else { _tooltip.hide(); } - } + }; return lengthIndicator; } From de23bd5c33e49e0d3488cc452e217cc9b5ae99d3 Mon Sep 17 00:00:00 2001 From: alanb43 Date: Sun, 27 Nov 2022 01:55:30 -0500 Subject: [PATCH 07/10] added warning when changeset comment length > 255 chars --- data/core.yaml | 1 + modules/ui/changeset_editor.js | 41 ++++++++++++++++++++-------------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index f98a773c5..041921828 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -615,6 +615,7 @@ en: comment_needed_message: Please add a changeset comment first. about_changeset_comments: About changeset comments about_changeset_comments_link: //wiki.openstreetmap.org/wiki/Good_changeset_comments + changeset_comment_length_warning: "Changeset comments can have a maximum of 255 characters" google_warning: "You mentioned Google in this comment: remember that copying from Google Maps is strictly forbidden." google_warning_link: https://www.openstreetmap.org/copyright contributors: diff --git a/modules/ui/changeset_editor.js b/modules/ui/changeset_editor.js index 8daa62bbd..1ff6580b2 100644 --- a/modules/ui/changeset_editor.js +++ b/modules/ui/changeset_editor.js @@ -85,10 +85,10 @@ export function uiChangesetEditor(context) { } } - // Add warning if comment mentions Google var hasGoogle = _tags.comment.match(/google/i); + var commentTooLong = _tags.comment.length > 255; var commentWarning = selection.select('.form-field-comment').selectAll('.comment-warning') - .data(hasGoogle ? [0] : []); + .data(hasGoogle || commentTooLong ? [0] : []); commentWarning.exit() .transition() @@ -96,23 +96,30 @@ export function uiChangesetEditor(context) { .style('opacity', 0) .remove(); - var commentEnter = commentWarning.enter() - .insert('div', '.tag-reference-body') - .attr('class', 'field-warning comment-warning') - .style('opacity', 0); + function displayWarningMessage(msg, link) { + var commentEnter = commentWarning.enter() + .insert('div', '.tag-reference-body') + .attr('class', 'field-warning comment-warning') + .style('opacity', 0); - commentEnter - .append('a') - .attr('target', '_blank') - .call(svgIcon('#iD-icon-alert', 'inline')) - .attr('href', t('commit.google_warning_link')) - .append('span') - .call(t.append('commit.google_warning')); + commentEnter + .append('a') + .attr('target', '_blank') + .call(svgIcon('#iD-icon-alert', 'inline')) + .attr('href', t(link)) + .append('span') + .call(t.append(msg)); - commentEnter - .transition() - .duration(200) - .style('opacity', 1); + commentEnter + .transition() + .duration(200) + .style('opacity', 1); + } + + // Add warning if comment mentions Google or comment length + // exceeds 255 chars + if (hasGoogle) displayWarningMessage('commit.google_warning', 'commit.google_warning_link'); + if (commentTooLong) displayWarningMessage('commit.changeset_comment_length_warning', 'commit.about_changeset_comments_link'); } From d5872cca737c265444b8a74efff291956bf1d8a3 Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Mon, 28 Nov 2022 19:08:37 +0100 Subject: [PATCH 08/10] show prettier maxChars warning for changeset comments from #9392 --- css/80_app.css | 5 ++- data/core.yaml | 2 +- modules/ui/changeset_editor.js | 68 +++++++++++++++++++++------------- modules/ui/fields/textarea.js | 3 +- modules/ui/length_indicator.js | 8 ++++ 5 files changed, 58 insertions(+), 28 deletions(-) diff --git a/css/80_app.css b/css/80_app.css index 3c08bd04b..4d5984c02 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -5201,13 +5201,16 @@ a.user-info { display: none; } -.field-warning, .changeset-info, .request-review, .commit-info { margin-bottom: 10px; } +.field-warning { + margin-top: 10px; +} + .request-review label { cursor: pointer; } diff --git a/data/core.yaml b/data/core.yaml index 72c4a8427..a67b88248 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -615,7 +615,7 @@ en: comment_needed_message: Please add a changeset comment first. about_changeset_comments: About changeset comments about_changeset_comments_link: //wiki.openstreetmap.org/wiki/Good_changeset_comments - changeset_comment_length_warning: "Changeset comments can have a maximum of 255 characters" + changeset_comment_length_warning: "Changeset comments can have a maximum of {maxChars} characters" google_warning: "You mentioned Google in this comment: remember that copying from Google Maps is strictly forbidden." google_warning_link: https://www.openstreetmap.org/copyright contributors: diff --git a/modules/ui/changeset_editor.js b/modules/ui/changeset_editor.js index 1ff6580b2..d929a72d9 100644 --- a/modules/ui/changeset_editor.js +++ b/modules/ui/changeset_editor.js @@ -1,4 +1,5 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; +import { select as d3_select } from 'd3-selection'; import { presetManager } from '../presets'; import { t } from '../core/localizer'; @@ -6,7 +7,7 @@ import { svgIcon } from '../svg/icon'; import { uiCombobox} from './combobox'; import { uiField } from './field'; import { uiFormFields } from './form_fields'; -import { utilArrayUniqBy, utilRebind, utilTriggerEvent } from '../util'; +import { utilArrayUniqBy, utilCleanOsmString, utilRebind, utilTriggerEvent, utilUnicodeCharsCount } from '../util'; export function uiChangesetEditor(context) { @@ -85,10 +86,26 @@ export function uiChangesetEditor(context) { } } - var hasGoogle = _tags.comment.match(/google/i); - var commentTooLong = _tags.comment.length > 255; + // Show warning(s) if comment mentions Google or comment length exceeds 255 chars + const warnings = []; + if (_tags.comment.match(/google/i)) { + warnings.push({ + id: 'contains "google"', + msg: t.append('commit.google_warning'), + link: t('commit.google_warning_link') + }); + } + const maxChars = context.maxCharsForTagValue(); + const strLen = utilUnicodeCharsCount(utilCleanOsmString(_tags.comment, Number.POSITIVE_INFINITY)); + if (strLen > maxChars || !true) { + warnings.push({ + id: 'message too long', + msg: t.append('commit.changeset_comment_length_warning', { maxChars: maxChars }), + }); + } + var commentWarning = selection.select('.form-field-comment').selectAll('.comment-warning') - .data(hasGoogle || commentTooLong ? [0] : []); + .data(warnings, d => d.id); commentWarning.exit() .transition() @@ -96,30 +113,31 @@ export function uiChangesetEditor(context) { .style('opacity', 0) .remove(); - function displayWarningMessage(msg, link) { - var commentEnter = commentWarning.enter() - .insert('div', '.tag-reference-body') - .attr('class', 'field-warning comment-warning') - .style('opacity', 0); + var commentEnter = commentWarning.enter() + .insert('div', '.comment-warning') + .attr('class', 'comment-warning field-warning') + .style('opacity', 0); - commentEnter - .append('a') - .attr('target', '_blank') - .call(svgIcon('#iD-icon-alert', 'inline')) - .attr('href', t(link)) - .append('span') - .call(t.append(msg)); + commentEnter + .call(svgIcon('#iD-icon-alert', 'inline')) + .append('span'); - commentEnter - .transition() - .duration(200) - .style('opacity', 1); - } + commentEnter + .transition() + .duration(200) + .style('opacity', 1); - // Add warning if comment mentions Google or comment length - // exceeds 255 chars - if (hasGoogle) displayWarningMessage('commit.google_warning', 'commit.google_warning_link'); - if (commentTooLong) displayWarningMessage('commit.changeset_comment_length_warning', 'commit.about_changeset_comments_link'); + commentWarning.merge(commentEnter).selectAll('div > span') + .text('') + .each(function(d) { + let selection = d3_select(this); + if (d.link) { + selection = selection.append('a') + .attr('target', '_blank') + .attr('href', d.link); + } + selection.call(d.msg); + }); } diff --git a/modules/ui/fields/textarea.js b/modules/ui/fields/textarea.js index 59fe8b6a2..fb56f7be1 100644 --- a/modules/ui/fields/textarea.js +++ b/modules/ui/fields/textarea.js @@ -13,7 +13,8 @@ import { uiLengthIndicator } from '..'; export function uiFieldTextarea(field, context) { var dispatch = d3_dispatch('change'); var input = d3_select(null); - var _lengthIndicator = uiLengthIndicator(context.maxCharsForTagValue()); + var _lengthIndicator = uiLengthIndicator(context.maxCharsForTagValue()) + .silent(field.usage === 'changeset' && field.key === 'comment'); var _tags; diff --git a/modules/ui/length_indicator.js b/modules/ui/length_indicator.js index a7e84857d..d6e034f88 100644 --- a/modules/ui/length_indicator.js +++ b/modules/ui/length_indicator.js @@ -17,6 +17,7 @@ export function uiLengthIndicator(maxChars) { selection.text(''); t.append('inspector.max_length_reached', { maxChars })(selection); }); + var _silent = false; var lengthIndicator = function(selection) { _wrap = selection.selectAll('span.length-indicator-wrap').data([0]); @@ -47,6 +48,7 @@ export function uiLengthIndicator(maxChars) { : 0) .style('pointer-events', d => d > maxChars * 0.8 ? null: 'none'); + if (_silent) return; if (strLen > maxChars) { _tooltip.show(); } else { @@ -54,5 +56,11 @@ export function uiLengthIndicator(maxChars) { } }; + lengthIndicator.silent = function(val) { + if (!arguments.length) return _silent; + _silent = val; + return lengthIndicator; + }; + return lengthIndicator; } From e5810acb6514204017473d22b00093b62f606c69 Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Mon, 28 Nov 2022 19:16:33 +0100 Subject: [PATCH 09/10] add to changelog --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebebe5634..6f0b4a4f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,8 @@ _Breaking developer changes, which may affect downstream projects or sites that # Unreleased +#### :tada: New Features +* Show a _remaining input length_ indicator and a warning if the maximum for OSM tags (typically, 255 characters) is exceeded ([#9390], [#9392] thanks [@alanb43], [#7943], [#9374]) #### :sparkles: Usability & Accessibility #### :white_check_mark: Validation #### :bug: Bugfixes @@ -48,9 +50,14 @@ _Breaking developer changes, which may affect downstream projects or sites that #### :hammer: Development * Upgrade to Transifex API v3 ([#9375]) +[#7943]: https://github.com/openstreetmap/iD/issues/7943 [#9372]: https://github.com/openstreetmap/iD/issues/9372 +[#9374]: https://github.com/openstreetmap/iD/issues/9374 [#9375]: https://github.com/openstreetmap/iD/pull/9375 [#9386]: https://github.com/openstreetmap/iD/issues/9386 +[#9390]: https://github.com/openstreetmap/iD/pull/9390 +[#9392]: https://github.com/openstreetmap/iD/pull/9392 +[@alanb43]: https://github.com/alanb43 # 2.23.2 From 827608ad2388055454e4f3052518ffc2f254a69a Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Mon, 28 Nov 2022 19:18:35 +0100 Subject: [PATCH 10/10] add warning icon also to "regular" max length indicator warning --- data/core.yaml | 2 +- modules/ui/length_indicator.js | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index a67b88248..d8c68b709 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -615,7 +615,7 @@ en: comment_needed_message: Please add a changeset comment first. about_changeset_comments: About changeset comments about_changeset_comments_link: //wiki.openstreetmap.org/wiki/Good_changeset_comments - changeset_comment_length_warning: "Changeset comments can have a maximum of {maxChars} characters" + changeset_comment_length_warning: "Changeset comments can have a maximum of {maxChars} characters." google_warning: "You mentioned Google in this comment: remember that copying from Google Maps is strictly forbidden." google_warning_link: https://www.openstreetmap.org/copyright contributors: diff --git a/modules/ui/length_indicator.js b/modules/ui/length_indicator.js index d6e034f88..ae11e7922 100644 --- a/modules/ui/length_indicator.js +++ b/modules/ui/length_indicator.js @@ -1,6 +1,7 @@ import { select as d3_select } from 'd3-selection'; import { t } from '../core/localizer'; +import { svgIcon } from '../svg'; import { utilUnicodeCharsCount, utilCleanOsmString @@ -15,7 +16,8 @@ export function uiLengthIndicator(maxChars) { .hasArrow(true) .content(() => selection => { selection.text(''); - t.append('inspector.max_length_reached', { maxChars })(selection); + selection.call(svgIcon('#iD-icon-alert', 'inline')); + selection.call(t.append('inspector.max_length_reached', { maxChars })); }); var _silent = false;