From ea40634ac6788ce62536ce930fa7de8ad16a72f5 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Wed, 1 May 2019 21:53:43 -0400 Subject: [PATCH 01/12] Add list<->text toggle for raw tag editor --- css/80_app.css | 22 +++++++++++++++++- modules/ui/raw_tag_editor.js | 45 ++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/css/80_app.css b/css/80_app.css index a99b480a3..d4828d11f 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -259,10 +259,12 @@ table.tags, table.tags td, table.tags th { .ar { right: 0; } input.hide, +textarea.hide, div.hide, form.hide, button.hide, a.hide, +ul.hide, li.hide { display: none; } @@ -2400,8 +2402,26 @@ div.combobox { /* Raw Tag Editor ------------------------------------------------------- */ +.raw-tag-options { + display: flex; + flex-flow: row nowrap; + flex-direction: row-reverse; + margin-top: -25px; +} +.raw-tag-option { + padding: 5px; +} + +.tag-text { + width: 100%; + height: 100%; + font-family: monospace; + white-space: pre; +} + +.tag-text, .tag-list { - padding-top: 10px; + margin-top: 10px; } .tag-row { width: 100%; diff --git a/modules/ui/raw_tag_editor.js b/modules/ui/raw_tag_editor.js index 72c48745f..ee5d97ac2 100644 --- a/modules/ui/raw_tag_editor.js +++ b/modules/ui/raw_tag_editor.js @@ -75,6 +75,51 @@ export function uiRawTagEditor(context) { rowData.push({ index: _indexedKeys.length, key: '', value: '' }); } + // --------- EXPERIMENT! + var options = wrap.selectAll('.raw-tag-options') + .data([0]) + + var optionsEnter = options.enter() + .append('div') + .attr('class', 'raw-tag-options'); + + var optionEnter = optionsEnter.selectAll('.raw-tag-option') + .data(['text', 'list']) + .enter(); + + optionEnter + .append('a') + .attr('class', 'raw-tag-option') + .attr('href', '#') + .text(function(d) { return d; }) + .on('click', function(d) { + wrap.selectAll('.tag-text') + .classed('hide', (d === 'list')); + wrap.selectAll('.tag-list, .add-row') + .classed('hide', (d === 'text')); + }); + + + var textarea = wrap.selectAll('.tag-text') + .data([0]); + + textarea = textarea.enter() + .append('textarea') + .attr('class', 'tag-text hide') + .merge(textarea); + + textarea + .attr('rows', rowData.length + 1) + .text(asText(rowData)); + + function asText(rows) { + return rows + .filter(function(row) { return row.key && row.key.trim() !== ''; }) + .map(function(row) { return row.key + '=' + row.value; }) + .join('\n') + '\n'; + } + // --------- END + // List of tags var list = wrap.selectAll('.tag-list') .data([0]); From af97f091505f4d792e60ad10c7e7299cb77448a6 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 2 May 2019 10:22:37 -0400 Subject: [PATCH 02/12] Remove "key=value" pasting into raw tag editor key (Reverts: #6211, #5070, #5024) The old code allows users to circumvent the readonly tag protection see https://github.com/openstreetmap/iD/issues/6185#issuecomment-488692351 We'll offer a text-mode instead, with stricter checks on which keys can be modified. --- modules/ui/raw_tag_editor.js | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/modules/ui/raw_tag_editor.js b/modules/ui/raw_tag_editor.js index ee5d97ac2..8fc8daacd 100644 --- a/modules/ui/raw_tag_editor.js +++ b/modules/ui/raw_tag_editor.js @@ -77,7 +77,7 @@ export function uiRawTagEditor(context) { // --------- EXPERIMENT! var options = wrap.selectAll('.raw-tag-options') - .data([0]) + .data([0]); var optionsEnter = options.enter() .append('div') @@ -362,25 +362,7 @@ export function uiRawTagEditor(context) { _pendingChange[kOld] = undefined; } - // if the key looks like "key=value key2=value2", split them up - #5024 - var keys = (kNew.match(/[\w_]+=/g) || []).map(function (key) { return key.slice(0, -1); }); - var vals = keys.length === 0 - ? [] - : kNew - .split(new RegExp(keys.map(function (key) { return key.replace('_', '\\_'); }).join('|'))) - .splice(1) - .map(function (val) { return val.slice(1).trim(); }); - - if (keys.length > 0) { - kNew = keys[0]; - vNew = vals[0]; - - keys.forEach(function (key, i) { - _pendingChange[key] = vals[i]; - }); - } else { - _pendingChange[kNew] = vNew; - } + _pendingChange[kNew] = vNew; d.key = kNew; // update datum to avoid exit/enter on tag update d.value = vNew; From ed9a436318172f5dbb0836c56053505ce5cddf09 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 2 May 2019 11:16:15 -0400 Subject: [PATCH 03/12] Move common tag diffing code from validators into `utilTagDiff` --- modules/util/index.js | 1 + modules/util/util.js | 24 +++++++++- modules/validations/outdated_tags.js | 17 +------- modules/validations/private_data.js | 11 ++--- test/spec/util/util.js | 65 ++++++++++++++++------------ 5 files changed, 67 insertions(+), 51 deletions(-) diff --git a/modules/util/index.js b/modules/util/index.js index b7d51e668..dde2a5614 100644 --- a/modules/util/index.js +++ b/modules/util/index.js @@ -37,6 +37,7 @@ export { utilRebind } from './rebind'; export { utilSetTransform } from './util'; export { utilSessionMutex } from './session_mutex'; export { utilStringQs } from './util'; +export { utilTagDiff } from './util'; export { utilTagText } from './util'; export { utilTiler } from './tiler'; export { utilTriggerEvent } from './trigger_event'; diff --git a/modules/util/util.js b/modules/util/util.js index 2dc9c9b03..ea8d477da 100644 --- a/modules/util/util.js +++ b/modules/util/util.js @@ -1,8 +1,10 @@ -import { t, textDirection } from './locale'; -import { utilDetect } from './detect'; import { remove as removeDiacritics } from 'diacritics'; import { fixRTLTextForSvg, rtlRegex } from './svg_paths_rtl_fix'; +import { t, textDirection } from './locale'; +import { utilArrayUnion } from './array'; +import { utilDetect } from './detect'; + export function utilTagText(entity) { var obj = (entity && entity.tags) || {}; @@ -12,6 +14,24 @@ export function utilTagText(entity) { } +export function utilTagDiff(oldTags, newTags) { + var tagDiff = []; + var keys = utilArrayUnion(Object.keys(oldTags), Object.keys(newTags)).sort(); + keys.forEach(function(k) { + var oldVal = oldTags[k]; + var newVal = newTags[k]; + + if (oldVal && (!newVal || newVal !== oldVal)) { + tagDiff.push('- ' + k + '=' + oldVal); + } + if (newVal && (!oldVal || newVal !== oldVal)) { + tagDiff.push('+ ' + k + '=' + newVal); + } + }); + return tagDiff; +} + + export function utilEntitySelector(ids) { return ids.length ? '.' + ids.join(',.') : 'nothing'; } diff --git a/modules/validations/outdated_tags.js b/modules/validations/outdated_tags.js index a17156a79..d73ed0b77 100644 --- a/modules/validations/outdated_tags.js +++ b/modules/validations/outdated_tags.js @@ -3,7 +3,7 @@ import { actionChangePreset } from '../actions/change_preset'; import { actionChangeTags } from '../actions/change_tags'; import { actionUpgradeTags } from '../actions/upgrade_tags'; import { osmIsOldMultipolygonOuterMember, osmOldMultipolygonOuterMemberOfRelation } from '../osm/multipolygon'; -import { utilArrayUnion, utilDisplayLabel } from '../util'; +import { utilDisplayLabel, utilTagDiff } from '../util'; import { validationIssue, validationIssueFix } from '../core/validation'; @@ -48,20 +48,7 @@ export function validationOutdatedTags() { } // determine diff - var keys = utilArrayUnion(Object.keys(oldTags), Object.keys(newTags)).sort(); - var tagDiff = []; - keys.forEach(function(k) { - var oldVal = oldTags[k]; - var newVal = newTags[k]; - - if (oldVal && (!newVal || newVal !== oldVal)) { - tagDiff.push('- ' + k + '=' + oldVal); - } - if (newVal && (!oldVal || newVal !== oldVal)) { - tagDiff.push('+ ' + k + '=' + newVal); - } - }); - + var tagDiff = utilTagDiff(oldTags, newTags); if (!tagDiff.length) return []; return [new validationIssue({ diff --git a/modules/validations/private_data.js b/modules/validations/private_data.js index 53adef44c..972e77dec 100644 --- a/modules/validations/private_data.js +++ b/modules/validations/private_data.js @@ -1,6 +1,6 @@ import { actionChangeTags } from '../actions/change_tags'; import { t } from '../util/locale'; -import { utilDisplayLabel } from '../util'; +import { utilDisplayLabel, utilTagDiff } from '../util'; import { validationIssue, validationIssueFix } from '../core/validation'; @@ -40,20 +40,17 @@ export function validationPrivateData() { var validation = function checkPrivateData(entity, context) { var tags = entity.tags; - var keepTags = {}; - var tagDiff = []; if (!tags.building || !privateBuildingValues[tags.building]) return []; + var keepTags = {}; for (var k in tags) { if (publicKeys[k]) return []; // probably a public feature - - if (personalTags[k]) { - tagDiff.push('- ' + k + '=' + tags[k]); - } else { + if (!personalTags[k]) { keepTags[k] = tags[k]; } } + var tagDiff = utilTagDiff(tags, keepTags); if (!tagDiff.length) return []; var fixID = tagDiff.length === 1 ? 'remove_tag' : 'remove_tags'; diff --git a/test/spec/util/util.js b/test/spec/util/util.js index be330efaf..3811273be 100644 --- a/test/spec/util/util.js +++ b/test/spec/util/util.js @@ -2,57 +2,68 @@ describe('iD.util', function() { describe('utilGetAllNodes', function() { it('gets all descendant nodes of a way', function() { - var a = iD.osmNode({ id: 'a' }), - b = iD.osmNode({ id: 'b' }), - w = iD.osmWay({ id: 'w', nodes: ['a','b','a'] }), - graph = iD.coreGraph([a, b, w]), - result = iD.utilGetAllNodes(['w'], graph); + var a = iD.osmNode({ id: 'a' }); + var b = iD.osmNode({ id: 'b' }); + var w = iD.osmWay({ id: 'w', nodes: ['a','b','a'] }); + var graph = iD.coreGraph([a, b, w]); + var result = iD.utilGetAllNodes(['w'], graph); expect(result).to.have.members([a, b]); expect(result).to.have.lengthOf(2); }); it('gets all descendant nodes of a relation', function() { - var a = iD.osmNode({ id: 'a' }), - b = iD.osmNode({ id: 'b' }), - c = iD.osmNode({ id: 'c' }), - w = iD.osmWay({ id: 'w', nodes: ['a','b','a'] }), - r = iD.osmRelation({ id: 'r', members: [{id: 'w'}, {id: 'c'}] }), - graph = iD.coreGraph([a, b, c, w, r]), - result = iD.utilGetAllNodes(['r'], graph); + var a = iD.osmNode({ id: 'a' }); + var b = iD.osmNode({ id: 'b' }); + var c = iD.osmNode({ id: 'c' }); + var w = iD.osmWay({ id: 'w', nodes: ['a','b','a'] }); + var r = iD.osmRelation({ id: 'r', members: [{id: 'w'}, {id: 'c'}] }); + var graph = iD.coreGraph([a, b, c, w, r]); + var result = iD.utilGetAllNodes(['r'], graph); expect(result).to.have.members([a, b, c]); expect(result).to.have.lengthOf(3); }); it('gets all descendant nodes of multiple ids', function() { - var a = iD.osmNode({ id: 'a' }), - b = iD.osmNode({ id: 'b' }), - c = iD.osmNode({ id: 'c' }), - d = iD.osmNode({ id: 'd' }), - e = iD.osmNode({ id: 'e' }), - w1 = iD.osmWay({ id: 'w1', nodes: ['a','b','a'] }), - w2 = iD.osmWay({ id: 'w2', nodes: ['c','b','a','c'] }), - r = iD.osmRelation({ id: 'r', members: [{id: 'w1'}, {id: 'd'}] }), - graph = iD.coreGraph([a, b, c, d, e, w1, w2, r]), - result = iD.utilGetAllNodes(['r', 'w2', 'e'], graph); + var a = iD.osmNode({ id: 'a' }); + var b = iD.osmNode({ id: 'b' }); + var c = iD.osmNode({ id: 'c' }); + var d = iD.osmNode({ id: 'd' }); + var e = iD.osmNode({ id: 'e' }); + var w1 = iD.osmWay({ id: 'w1', nodes: ['a','b','a'] }); + var w2 = iD.osmWay({ id: 'w2', nodes: ['c','b','a','c'] }); + var r = iD.osmRelation({ id: 'r', members: [{id: 'w1'}, {id: 'd'}] }); + var graph = iD.coreGraph([a, b, c, d, e, w1, w2, r]); + var result = iD.utilGetAllNodes(['r', 'w2', 'e'], graph); expect(result).to.have.members([a, b, c, d, e]); expect(result).to.have.lengthOf(5); }); it('handles recursive relations', function() { - var a = iD.osmNode({ id: 'a' }), - r1 = iD.osmRelation({ id: 'r1', members: [{id: 'r2'}] }), - r2 = iD.osmRelation({ id: 'r2', members: [{id: 'r1'}, {id: 'a'}] }), - graph = iD.coreGraph([a, r1, r2]), - result = iD.utilGetAllNodes(['r1'], graph); + var a = iD.osmNode({ id: 'a' }); + var r1 = iD.osmRelation({ id: 'r1', members: [{id: 'r2'}] }); + var r2 = iD.osmRelation({ id: 'r2', members: [{id: 'r1'}, {id: 'a'}] }); + var graph = iD.coreGraph([a, r1, r2]); + var result = iD.utilGetAllNodes(['r1'], graph); expect(result).to.have.members([a]); expect(result).to.have.lengthOf(1); }); }); + it('utilTagDiff', function() { + var oldTags = { a: 'one', b: 'two', c: 'three' }; + var newTags = { a: 'one', b: 'three', d: 'four' }; + var diff = iD.utilTagDiff(oldTags, newTags); + expect(diff).to.have.length(4); + expect(diff[0]).to.eql('- b=two'); // delete-modify + expect(diff[1]).to.eql('+ b=three'); // insert-modify + expect(diff[2]).to.eql('- c=three'); // delete + expect(diff[3]).to.eql('+ d=four'); // insert + }); + it('utilTagText', function() { expect(iD.utilTagText({})).to.eql(''); expect(iD.utilTagText({tags:{foo:'bar'}})).to.eql('foo=bar'); From cebe4ced8ae88786db3598000bf9b85063005a6c Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 2 May 2019 11:17:06 -0400 Subject: [PATCH 04/12] Preserve user's preference for 'list'/'text' view for the raw tag editor --- modules/ui/raw_tag_editor.js | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/modules/ui/raw_tag_editor.js b/modules/ui/raw_tag_editor.js index 8fc8daacd..3d83424e5 100644 --- a/modules/ui/raw_tag_editor.js +++ b/modules/ui/raw_tag_editor.js @@ -13,6 +13,12 @@ import { utilArrayDifference, utilGetSetValue, utilNoAuto, utilRebind } from '.. export function uiRawTagEditor(context) { var taginfo = services.taginfo; var dispatch = d3_dispatch('change'); + var availableViews = [ + { id: 'text' }, + { id: 'list' } + ]; + + var _tagView = (context.storage('raw-tag-editor-view') || 'list'); // 'list, 'text' var _readOnlyTags = []; var _indexedKeys = []; var _showBlank = false; @@ -84,19 +90,20 @@ export function uiRawTagEditor(context) { .attr('class', 'raw-tag-options'); var optionEnter = optionsEnter.selectAll('.raw-tag-option') - .data(['text', 'list']) + .data(availableViews, function(d) { return d.id; }) .enter(); optionEnter .append('a') .attr('class', 'raw-tag-option') .attr('href', '#') - .text(function(d) { return d; }) + .text(function(d) { return d.id; }) .on('click', function(d) { + context.storage('raw-tag-editor-view', d.id); wrap.selectAll('.tag-text') - .classed('hide', (d === 'list')); + .classed('hide', (d.id !== 'text')); wrap.selectAll('.tag-list, .add-row') - .classed('hide', (d === 'text')); + .classed('hide', (d.id !== 'list')); }); @@ -105,7 +112,7 @@ export function uiRawTagEditor(context) { textarea = textarea.enter() .append('textarea') - .attr('class', 'tag-text hide') + .attr('class', 'tag-text' + (_tagView !== 'text' ? ' hide' : '')) .merge(textarea); textarea @@ -126,7 +133,7 @@ export function uiRawTagEditor(context) { list = list.enter() .append('ul') - .attr('class', 'tag-list') + .attr('class', 'tag-list' + (_tagView !== 'list' ? ' hide' : '')) .merge(list); @@ -135,7 +142,7 @@ export function uiRawTagEditor(context) { .data([0]) .enter() .append('div') - .attr('class', 'add-row'); + .attr('class', 'add-row' + (_tagView !== 'list' ? ' hide' : '')); addRowEnter .append('button') From b17e4e4f1d8d9e0b7ec0996684fc250be4cc0bef Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 2 May 2019 13:49:26 -0400 Subject: [PATCH 05/12] Add 'list'/'text' toggle buttons --- build_data.js | 5 ++++- css/80_app.css | 34 ++++++++++++++++++++++++++++++-- modules/ui/raw_tag_editor.js | 21 ++++++++++++++------ svg/fontawesome/fas-i-cursor.svg | 1 + svg/fontawesome/fas-th-list.svg | 1 + 5 files changed, 53 insertions(+), 9 deletions(-) create mode 100644 svg/fontawesome/fas-i-cursor.svg create mode 100644 svg/fontawesome/fas-th-list.svg diff --git a/build_data.js b/build_data.js index 1f2daac3a..50d282f7a 100644 --- a/build_data.js +++ b/build_data.js @@ -55,9 +55,12 @@ module.exports = function buildData() { // Font Awesome icons used var faIcons = { - 'fas-long-arrow-alt-right': {} + 'fas-i-cursor': {}, + 'fas-long-arrow-alt-right': {}, + 'fas-th-list': {} }; + // The Noun Project icons used var tnpIcons = {}; // Start clean diff --git a/css/80_app.css b/css/80_app.css index d4828d11f..29398a406 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -2407,10 +2407,40 @@ div.combobox { flex-flow: row nowrap; flex-direction: row-reverse; margin-top: -25px; + padding: 0 3px; } -.raw-tag-option { - padding: 5px; +button.raw-tag-option { + flex: 0 0 20px; + height: 20px; + width: 20px; + background: #aaa; + color: #eee; + margin: 0 3px; } +button.raw-tag-option:focus, +button.raw-tag-option:hover, +button.raw-tag-option.active { + color: #fff; + background: #597be7; +} +button.raw-tag-option.selected { + color: #fff; + background: #7092ff; +} +button.raw-tag-option svg.icon { + width: 14px; + height: 14px; + vertical-align: text-bottom; +} +[dir='ltr'] button.raw-tag-option-list { + -moz-transform: scaleX(-1); + -o-transform: scaleX(-1); + -webkit-transform: scaleX(-1); + transform: scaleX(-1); + filter: FlipH; + -ms-filter: "FlipH"; +} + .tag-text { width: 100%; diff --git a/modules/ui/raw_tag_editor.js b/modules/ui/raw_tag_editor.js index 3d83424e5..bb28cc875 100644 --- a/modules/ui/raw_tag_editor.js +++ b/modules/ui/raw_tag_editor.js @@ -14,8 +14,8 @@ export function uiRawTagEditor(context) { var taginfo = services.taginfo; var dispatch = d3_dispatch('change'); var availableViews = [ - { id: 'text' }, - { id: 'list' } + { id: 'text', icon: '#fas-i-cursor' }, + { id: 'list', icon: '#fas-th-list' } ]; var _tagView = (context.storage('raw-tag-editor-view') || 'list'); // 'list, 'text' @@ -94,16 +94,25 @@ export function uiRawTagEditor(context) { .enter(); optionEnter - .append('a') - .attr('class', 'raw-tag-option') - .attr('href', '#') - .text(function(d) { return d.id; }) + .append('button') + .attr('class', function(d) { + return 'raw-tag-option raw-tag-option-' + d.id + (_tagView === d.id ? ' selected' : ''); + }) + .attr('title', function(d) { return d.id; }) .on('click', function(d) { + _tagView = d.id; context.storage('raw-tag-editor-view', d.id); + + wrap.selectAll('.raw-tag-option') + .classed('selected', function(datum) { return datum === d; }); wrap.selectAll('.tag-text') .classed('hide', (d.id !== 'text')); wrap.selectAll('.tag-list, .add-row') .classed('hide', (d.id !== 'list')); + }) + .each(function(d) { + d3_select(this) + .call(svgIcon(d.icon)); }); diff --git a/svg/fontawesome/fas-i-cursor.svg b/svg/fontawesome/fas-i-cursor.svg new file mode 100644 index 000000000..8670a48aa --- /dev/null +++ b/svg/fontawesome/fas-i-cursor.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/svg/fontawesome/fas-th-list.svg b/svg/fontawesome/fas-th-list.svg new file mode 100644 index 000000000..108272386 --- /dev/null +++ b/svg/fontawesome/fas-th-list.svg @@ -0,0 +1 @@ + \ No newline at end of file From 61819058d3b1fbcaf37f9c058530673caaa5b724 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 2 May 2019 14:21:57 -0400 Subject: [PATCH 06/12] Avoid frequent reflow from setting scrollTop (possibly re: #6289) --- modules/ui/entity_editor.js | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/modules/ui/entity_editor.js b/modules/ui/entity_editor.js index 9b3ae46e1..55204e475 100644 --- a/modules/ui/entity_editor.js +++ b/modules/ui/entity_editor.js @@ -17,7 +17,7 @@ import { uiTagReference } from './tag_reference'; import { uiPresetEditor } from './preset_editor'; import { uiEntityIssues } from './entity_issues'; import { uiTooltipHtml } from './tooltipHtml'; -import { utilCleanTags, utilRebind } from '../util'; +import { utilCallWhenIdle, utilCleanTags, utilRebind } from '../util'; export function uiEntityEditor(context) { @@ -25,6 +25,7 @@ export function uiEntityEditor(context) { var _state = 'select'; var _coalesceChanges = false; var _modified = false; + var _scrolled = false; var _base; var _entityID; var _activePreset; @@ -83,7 +84,8 @@ export function uiEntityEditor(context) { // Enter var bodyEnter = body.enter() .append('div') - .attr('class', 'inspector-body'); + .attr('class', 'inspector-body') + .on('scroll.entity-editor', function() { _scrolled = true; }); bodyEnter .append('div') @@ -327,9 +329,14 @@ export function uiEntityEditor(context) { _coalesceChanges = false; // reset the scroll to the top of the inspector (warning: triggers reflow) - var body = d3_selectAll('.entity-editor-pane .inspector-body'); - if (!body.empty()) { - body.node().scrollTop = 0; + if (_scrolled) { + utilCallWhenIdle(function() { + var body = d3_selectAll('.entity-editor-pane .inspector-body'); + if (!body.empty()) { + _scrolled = false; + body.node().scrollTop = 0; + } + })(); } var presetMatch = context.presets().match(context.entity(_entityID), _base); From 7b4a9a43b0cec4889bc12375a5f030a22885c735 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 2 May 2019 22:59:41 -0400 Subject: [PATCH 07/12] Change `utilTagDiff` to return an object with details --- modules/util/util.js | 16 ++++++++++++++-- modules/validations/outdated_tags.js | 4 ++-- modules/validations/private_data.js | 4 ++-- test/spec/util/util.js | 16 ++++++++++++---- 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/modules/util/util.js b/modules/util/util.js index ea8d477da..a5bfc2534 100644 --- a/modules/util/util.js +++ b/modules/util/util.js @@ -22,10 +22,22 @@ export function utilTagDiff(oldTags, newTags) { var newVal = newTags[k]; if (oldVal && (!newVal || newVal !== oldVal)) { - tagDiff.push('- ' + k + '=' + oldVal); + tagDiff.push({ + type: '-', + key: k, + oldVal: oldVal, + newVal: newVal, + display: '- ' + k + '=' + oldVal + }); } if (newVal && (!oldVal || newVal !== oldVal)) { - tagDiff.push('+ ' + k + '=' + newVal); + tagDiff.push({ + type: '+', + key: k, + oldVal: oldVal, + newVal: newVal, + display: '+ ' + k + '=' + newVal + }); } }); return tagDiff; diff --git a/modules/validations/outdated_tags.js b/modules/validations/outdated_tags.js index d73ed0b77..e3336e68d 100644 --- a/modules/validations/outdated_tags.js +++ b/modules/validations/outdated_tags.js @@ -99,10 +99,10 @@ export function validationOutdatedTags() { .attr('class', 'tagDiff-row') .append('td') .attr('class', function(d) { - var klass = d.charAt(0) === '+' ? 'add' : 'remove'; + var klass = d.type === '+' ? 'add' : 'remove'; return 'tagDiff-cell tagDiff-cell-' + klass; }) - .text(function(d) { return d; }); + .text(function(d) { return d.display; }); } } diff --git a/modules/validations/private_data.js b/modules/validations/private_data.js index 972e77dec..15c6ae5b7 100644 --- a/modules/validations/private_data.js +++ b/modules/validations/private_data.js @@ -107,10 +107,10 @@ export function validationPrivateData() { .attr('class', 'tagDiff-row') .append('td') .attr('class', function(d) { - var klass = d.charAt(0) === '+' ? 'add' : 'remove'; + var klass = d.type === '+' ? 'add' : 'remove'; return 'tagDiff-cell tagDiff-cell-' + klass; }) - .text(function(d) { return d; }); + .text(function(d) { return d.display; }); } }; diff --git a/test/spec/util/util.js b/test/spec/util/util.js index 3811273be..63b999955 100644 --- a/test/spec/util/util.js +++ b/test/spec/util/util.js @@ -58,10 +58,18 @@ describe('iD.util', function() { var newTags = { a: 'one', b: 'three', d: 'four' }; var diff = iD.utilTagDiff(oldTags, newTags); expect(diff).to.have.length(4); - expect(diff[0]).to.eql('- b=two'); // delete-modify - expect(diff[1]).to.eql('+ b=three'); // insert-modify - expect(diff[2]).to.eql('- c=three'); // delete - expect(diff[3]).to.eql('+ d=four'); // insert + expect(diff[0]).to.eql({ + type: '-', key: 'b', oldVal: 'two', newVal: 'three', display: '- b=two' // delete-modify + }); + expect(diff[1]).to.eql({ + type: '+', key: 'b', oldVal: 'two', newVal: 'three', display: '+ b=three' // insert-modify + }); + expect(diff[2]).to.eql({ + type: '-', key: 'c', oldVal: 'three', newVal: undefined, display: '- c=three' // delete + }); + expect(diff[3]).to.eql({ + type: '+', key: 'd', oldVal: undefined, newVal: 'four', display: '+ d=four' // insert + }); }); it('utilTagText', function() { From c95860f36d4747e12a920a6f3fcc463cdfce38c2 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 2 May 2019 23:01:33 -0400 Subject: [PATCH 08/12] Convert changes in the textarea back to tag changes --- modules/ui/raw_tag_editor.js | 62 ++++++++++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 10 deletions(-) diff --git a/modules/ui/raw_tag_editor.js b/modules/ui/raw_tag_editor.js index bb28cc875..649dff137 100644 --- a/modules/ui/raw_tag_editor.js +++ b/modules/ui/raw_tag_editor.js @@ -7,7 +7,7 @@ import { svgIcon } from '../svg/icon'; import { uiCombobox } from './combobox'; import { uiDisclosure } from './disclosure'; import { uiTagReference } from './tag_reference'; -import { utilArrayDifference, utilGetSetValue, utilNoAuto, utilRebind } from '../util'; +import { utilArrayDifference, utilGetSetValue, utilNoAuto, utilRebind, utilTagDiff } from '../util'; export function uiRawTagEditor(context) { @@ -81,7 +81,8 @@ export function uiRawTagEditor(context) { rowData.push({ index: _indexedKeys.length, key: '', value: '' }); } - // --------- EXPERIMENT! + + // View Options var options = wrap.selectAll('.raw-tag-options') .data([0]); @@ -116,27 +117,29 @@ export function uiRawTagEditor(context) { }); + // View as Text var textarea = wrap.selectAll('.tag-text') .data([0]); textarea = textarea.enter() .append('textarea') .attr('class', 'tag-text' + (_tagView !== 'text' ? ' hide' : '')) + .call(utilNoAuto) .merge(textarea); textarea .attr('rows', rowData.length + 1) - .text(asText(rowData)); + .call(utilGetSetValue, rowsToText(rowData)) + .on('blur', textChanged) + .on('change', textChanged); - function asText(rows) { - return rows - .filter(function(row) { return row.key && row.key.trim() !== ''; }) - .map(function(row) { return row.key + '=' + row.value; }) - .join('\n') + '\n'; + var fieldsExpanded = d3_select('.hide-toggle-preset_fields.expanded').size(); + if (_tagView === 'text' && !fieldsExpanded) { + textarea.node().focus(); } - // --------- END - // List of tags + + // View as List var list = wrap.selectAll('.tag-list') .data([0]); @@ -278,6 +281,45 @@ export function uiRawTagEditor(context) { } + function rowsToText(rows) { + return rows + .filter(function(row) { return row.key && row.key.trim() !== ''; }) + .map(function(row) { return row.key + '=' + row.value; }) + .join('\n') + '\n'; + } + + + function textChanged() { + var newText = this.value.trim(); + var newTags = {}; + newText.split('\n').forEach(function(row) { + var m = row.match(/^\s*([^=]+)=(.*)$/); + if (m !== null) { + var k = m[1].trim(); + var v = m[2].trim(); + newTags[k] = v; + } + }); + + var tagDiff = utilTagDiff(_tags, newTags); + if (!tagDiff.length) return; + + _pendingChange = _pendingChange || {}; + + tagDiff.forEach(function(change) { + if (isReadOnly({ key: change.key })) return; + + if (change.type === '-') { + _pendingChange[change.key] = undefined; + } else if (change.type === '+') { + _pendingChange[change.key] = change.newVal || ''; + } + }); + + scheduleChange(); + } + + function pushMore() { if (d3_event.keyCode === 9 && !d3_event.shiftKey && list.selectAll('li:last-child input.value').node() === this) { From fbe5389ecb05226c17d87de44939e3d2de044f0e Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 3 May 2019 10:14:53 -0400 Subject: [PATCH 09/12] Set textarea height to fit content --- modules/ui/raw_tag_editor.js | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/modules/ui/raw_tag_editor.js b/modules/ui/raw_tag_editor.js index 649dff137..a3e91582d 100644 --- a/modules/ui/raw_tag_editor.js +++ b/modules/ui/raw_tag_editor.js @@ -106,8 +106,11 @@ export function uiRawTagEditor(context) { wrap.selectAll('.raw-tag-option') .classed('selected', function(datum) { return datum === d; }); + wrap.selectAll('.tag-text') - .classed('hide', (d.id !== 'text')); + .classed('hide', (d.id !== 'text')) + .each(setTextareaHeight); + wrap.selectAll('.tag-list, .add-row') .classed('hide', (d.id !== 'list')); }) @@ -128,8 +131,9 @@ export function uiRawTagEditor(context) { .merge(textarea); textarea - .attr('rows', rowData.length + 1) .call(utilGetSetValue, rowsToText(rowData)) + .each(setTextareaHeight) + .on('input', setTextareaHeight) .on('blur', textChanged) .on('change', textChanged); @@ -281,6 +285,15 @@ export function uiRawTagEditor(context) { } + function setTextareaHeight() { + if (_tagView !== 'text') return; + + var selection = d3_select(this); + selection.style('height', null); + selection.style('height', selection.node().scrollHeight + 5 + 'px'); + } + + function rowsToText(rows) { return rows .filter(function(row) { return row.key && row.key.trim() !== ''; }) From 8a4e822fd7737c18190ee0722cb116465cae893f Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 3 May 2019 10:33:50 -0400 Subject: [PATCH 10/12] If All Fields section is hidden, focus textarea and put cursor at end --- modules/ui/raw_tag_editor.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/modules/ui/raw_tag_editor.js b/modules/ui/raw_tag_editor.js index a3e91582d..b8b318153 100644 --- a/modules/ui/raw_tag_editor.js +++ b/modules/ui/raw_tag_editor.js @@ -121,6 +121,7 @@ export function uiRawTagEditor(context) { // View as Text + var textData = rowsToText(rowData); var textarea = wrap.selectAll('.tag-text') .data([0]); @@ -131,15 +132,18 @@ export function uiRawTagEditor(context) { .merge(textarea); textarea - .call(utilGetSetValue, rowsToText(rowData)) + .call(utilGetSetValue, textData) .each(setTextareaHeight) .on('input', setTextareaHeight) .on('blur', textChanged) .on('change', textChanged); + // If All Fields section is hidden, focus textarea and put cursor at end.. var fieldsExpanded = d3_select('.hide-toggle-preset_fields.expanded').size(); - if (_tagView === 'text' && !fieldsExpanded) { - textarea.node().focus(); + if (_state !== 'hover' && _tagView === 'text' && !fieldsExpanded) { + var element = textarea.node(); + element.focus(); + element.setSelectionRange(textData.length, textData.length); } @@ -295,10 +299,12 @@ export function uiRawTagEditor(context) { function rowsToText(rows) { - return rows + var str = rows .filter(function(row) { return row.key && row.key.trim() !== ''; }) .map(function(row) { return row.key + '=' + row.value; }) - .join('\n') + '\n'; + .join('\n'); + + return _state === 'hover' ? str : str + '\n'; } From 22b7e03d3b1c623e3d3e0d44dc89e1356bde893d Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 3 May 2019 10:39:38 -0400 Subject: [PATCH 11/12] Disable spellcheck in the tag textarea --- modules/ui/raw_tag_editor.js | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/ui/raw_tag_editor.js b/modules/ui/raw_tag_editor.js index b8b318153..77d9243c8 100644 --- a/modules/ui/raw_tag_editor.js +++ b/modules/ui/raw_tag_editor.js @@ -129,6 +129,7 @@ export function uiRawTagEditor(context) { .append('textarea') .attr('class', 'tag-text' + (_tagView !== 'text' ? ' hide' : '')) .call(utilNoAuto) + .attr('spellcheck', 'false') .merge(textarea); textarea From 16ec25753d093ea67dfff74b54371b10be3f5313 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 3 May 2019 14:42:35 -0400 Subject: [PATCH 12/12] Support escaping of tricky characters, support quoted strings keys or values can be quoted or unquoted: - leisure=park - leisure="park" - "leisure"=park - "leisure"="park" --- modules/ui/raw_tag_editor.js | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/modules/ui/raw_tag_editor.js b/modules/ui/raw_tag_editor.js index 77d9243c8..135369496 100644 --- a/modules/ui/raw_tag_editor.js +++ b/modules/ui/raw_tag_editor.js @@ -299,10 +299,29 @@ export function uiRawTagEditor(context) { } + function stringify(s) { + return JSON.stringify(s).slice(1, -1); // without leading/trailing " + } + + function unstringify(s) { + var leading = ''; + var trailing = ''; + if (s.length < 1 || s.charAt(0) !== '"') { + leading = '"'; + } + if (s.length < 2 || s.charAt(s.length - 1) !== '"' || + (s.charAt(s.length - 1) === '"' && s.charAt(s.length - 2) === '\\') + ) { + trailing = '"'; + } + return JSON.parse(leading + s + trailing); + } + + function rowsToText(rows) { var str = rows .filter(function(row) { return row.key && row.key.trim() !== ''; }) - .map(function(row) { return row.key + '=' + row.value; }) + .map(function(row) { return stringify(row.key) + '=' + stringify(row.value); }) .join('\n'); return _state === 'hover' ? str : str + '\n'; @@ -315,8 +334,8 @@ export function uiRawTagEditor(context) { newText.split('\n').forEach(function(row) { var m = row.match(/^\s*([^=]+)=(.*)$/); if (m !== null) { - var k = m[1].trim(); - var v = m[2].trim(); + var k = unstringify(m[1].trim()); + var v = unstringify(m[2].trim()); newTags[k] = v; } });