diff --git a/CHANGELOG.md b/CHANGELOG.md index e268fb85e..46a6a70c6 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 @@ -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 diff --git a/css/80_app.css b/css/80_app.css index f0d7659ca..4d5984c02 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; @@ -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; } diff --git a/data/core.yaml b/data/core.yaml index 2fe80a29c..4795f4f58 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 {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 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/changeset_editor.js b/modules/ui/changeset_editor.js index 8daa62bbd..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) { } } - // 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); + }); } diff --git a/modules/ui/fields/combo.js b/modules/ui/fields/combo.js index 589324c31..7cdfa43e7 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 = []; @@ -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]); + } } }; 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 31e792052..fb56f7be1 100644 --- a/modules/ui/fields/textarea.js +++ b/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]); + } }; 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..ae11e7922 --- /dev/null +++ b/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; +} 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); + }