Merge branch 'maxlength-indicator' into develop

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
@@ -49,10 +51,15 @@ _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
 [#9397]: https://github.com/openstreetmap/iD/issues/9397
+[@alanb43]: https://github.com/alanb43
 
 
 # 2.23.2

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;
@@ -2324,6 +2324,42 @@ 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 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;
+}
+
+.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;
+}
+
+.tooltip.max-length-warning {
+    z-index: 10;
+}
 
 /* Field Help
 ------------------------------------------------------- */
@@ -5165,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;
 }

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 {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:
@@ -782,6 +783,7 @@ en:
       foot: ft
       # abbreviation of inches
       inch: in
+    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

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 */

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()
                 );
             }
         }

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) {
             }
         }
 
-        // Add warning if comment mentions Google
-        var hasGoogle = _tags.comment.match(/google/i);
+        // 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 ? [0] : []);
+            .data(warnings, d => d.id);
 
         commentWarning.exit()
             .transition()
@@ -97,22 +114,30 @@ export function uiChangesetEditor(context) {
             .remove();
 
         var commentEnter = commentWarning.enter()
-            .insert('div', '.tag-reference-body')
-            .attr('class', 'field-warning comment-warning')
+            .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('commit.google_warning_link'))
-            .append('span')
-            .call(t.append('commit.google_warning'));
+            .append('span');
 
         commentEnter
             .transition()
             .duration(200)
             .style('opacity', 1);
+
+        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);
+            });
     }
 
 

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 = [];
@@ -398,6 +400,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);
     }
@@ -457,6 +461,12 @@ export function uiFieldCombo(field, context) {
             .call(initCombo, selection)
             .merge(_input);
 
+        if (_isSemi) {
+            _inputWrap.call(_lengthIndicator);
+        } else if (!_isMulti) {
+            _container.call(_lengthIndicator);
+        }
+
         if (_isNetwork) {
             var extent = combinedEntityExtent();
             var countryCode = extent && countryCoder.iso1A2Code(extent.center());
@@ -467,7 +477,13 @@ export function uiFieldCombo(field, context) {
             .on('change', change)
             .on('blur', change)
             .on('input', function() {
-                updateIcon(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);
             });
 
         _input
@@ -698,6 +714,10 @@ export function uiFieldCombo(field, context) {
             if (!Array.isArray(tags[field.key])) {
                 updateIcon(tags[field.key]);
             }
+
+            if (!isMixed) {
+                _lengthIndicator.update(tags[field.key]);
+            }
         }
     };
 

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]);
+        }
     };
 
 

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]);
+        }
     };
 
 

modules/ui/fields/textarea.js

@@ -7,11 +7,14 @@ import {
     utilNoAuto,
     utilRebind
 } from '../../util';
+import { uiLengthIndicator } from '..';
 
 
 export function uiFieldTextarea(field, context) {
     var dispatch = d3_dispatch('change');
     var input = d3_select(null);
+    var _lengthIndicator = uiLengthIndicator(context.maxCharsForTagValue())
+        .silent(field.usage === 'changeset' && field.key === 'comment');
     var _tags;
 
 
@@ -22,6 +25,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 +39,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 +68,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]);
+        }
     };
 
 

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';

modules/ui/length_indicator.js

@@ -0,0 +1,68 @@
+import { select as d3_select } from 'd3-selection';
+
+import { t } from '../core/localizer';
+import { svgIcon } from '../svg';
+import {
+    utilUnicodeCharsCount,
+    utilCleanOsmString
+} from '../util';
+import { uiPopover } from './popover';
+
+
+export function uiLengthIndicator(maxChars) {
+    var _wrap = d3_select(null);
+    var _tooltip = uiPopover('tooltip max-length-warning')
+        .placement('bottom')
+        .hasArrow(true)
+        .content(() => selection => {
+            selection.text('');
+            selection.call(svgIcon('#iD-icon-alert', 'inline'));
+            selection.call(t.append('inspector.max_length_reached', { maxChars }));
+        });
+    var _silent = false;
+
+    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));
+
+        let indicator = _wrap.selectAll('span.length-indicator')
+            .data([strLen]);
+
+        indicator.enter()
+            .append('span')
+            .merge(indicator)
+            .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 (_silent) return;
+        if (strLen > maxChars) {
+            _tooltip.show();
+        } else {
+            _tooltip.hide();
+        }
+    };
+
+    lengthIndicator.silent = function(val) {
+        if (!arguments.length) return _silent;
+        _silent = val;
+        return lengthIndicator;
+    };
+
+    return lengthIndicator;
+}

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';

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);
+  }