CalvinBackup/iD
1
0
Fork 0
mirror of https://github.com/FoggedLens/iD.git synced 2026-05-02 00:15:12 +02:00
Code Issues Packages Projects Releases Wiki Activity

simplify raw tag editor ui and logic (#10889)

Browse Source 
* always show empty line at bottom of the tag list to allow adding more tags (replaces `+` button and tab logic)
* disable delete and info buttons on empty tag
* replace deferring of tag change events and re-rendering with a simpler logic (to fix issues like #10871):
  only set input field values if the rendering event was triggered by an external event, or by a change of the respective field in the raw tag editor itself
* skip delete and tag info buttons while navigating using tab: makes it more quick to get to where one typically needs to, deleting a key using the keyboard only is possible by emptying the tag's key, and advanced users are not typically using the `i` button anyway
This commit is contained in:
Martin Raifer
2025-05-12 18:21:55 +02:00
committed by GitHub
parent 7f8e676af3
commit ac59197632
5 changed files with 55 additions and 115 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG.md
+2
View File
@@ -39,6 +39,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
#### :sparkles: Usability & Accessibility
* Render housenumbers (or housenames) of address points or buildings as dedicated labels on the map ([#10970])
* Simplify raw tag editor and make it easier to use with keyboard-only input ([#10889])
#### :scissors: Operations
#### :camera: Street-Level
#### :white_check_mark: Validation
@@ -49,6 +50,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
#### :mortar_board: Walkthrough / Help
#### :hammer: Development
[#10889]: https://github.com/openstreetmap/iD/pull/10889
[#10970]: https://github.com/openstreetmap/iD/pull/10970
[#11027]: https://github.com/openstreetmap/iD/pull/11027
css/80_app.css
+6 -3
View File
@@ -359,6 +359,9 @@ button.disabled {
color: rgba(0,0,0,.4);
cursor: not-allowed;
}
button.disabled .icon {
fill: rgba(0,0,0,.4);
}
.joined > * {
border-radius: 0;
@@ -4250,7 +4253,8 @@ li.issue-fix-item button:not(.actionable) .fix-icon {
.inspector-hover .form-field-input-radio label,
.inspector-hover .form-field-input-radio label span,
.inspector-hover .form-field-input-radio label.remove .icon,
.inspector-hover .add-row,
.inspector-hover .tag-row button,
.inspector-hover .tag-row.add-tag,
.inspector-hover .section-entity-issues .issue-container .issue-fix-list,
.inspector-hover .section-entity-issues .issue-container .issue-info-button {
display: none;
@@ -4264,9 +4268,8 @@ li.issue-fix-item button:not(.actionable) .fix-icon {
.inspector-hover .hide-toggle:before,
.inspector-hover .more-fields,
.inspector-hover .field-label button,
.inspector-hover .tag-row button,
.inspector-hover .footer * {
opacity: 0;
visibility: hidden;
}
/* Unstyle the active entity issue on hover */
data/core.yaml
-1
View File
@@ -713,7 +713,6 @@ en:
features: Features
title_count: "{title} ({count})"
add_to_relation: Add to a relation
add_to_tag: Add a tag
new_relation: New relation...
choose_relation: Choose a parent relation
role: Role
modules/ui/sections/raw_tag_editor.js
+39 -83
View File
@@ -1,5 +1,6 @@
import { dispatch as d3_dispatch } from 'd3-dispatch';
import { select as d3_select } from 'd3-selection';
import { isEmpty } from 'lodash-es';
import { services } from '../../services';
import { svgIcon } from '../../svg/icon';
@@ -7,10 +8,9 @@ import { uiCombobox } from '../combobox';
import { uiSection } from '../section';
import { uiTagReference } from '../tag_reference';
import { prefs } from '../../core/preferences';
import { localizer, t } from '../../core/localizer';
import { t } from '../../core/localizer';
import { utilArrayDifference, utilArrayIdentical } from '../../util/array';
import { utilGetSetValue, utilNoAuto, utilRebind, utilTagDiff } from '../../util';
import { uiTooltip } from '..';
import { allowUpperCaseTagValues } from '../../osm/tags';
import { fileFetcher } from '../../core';
@@ -42,7 +42,6 @@ export function uiSectionRawTagEditor(id, context) {
var _readOnlyTags = [];
// the keys in the order we want them to display
var _orderedKeys = [];
var _showBlank = false;
var _pendingChange = null;
var _state;
var _presets;
@@ -76,11 +75,8 @@ export function uiSectionRawTagEditor(id, context) {
return { index: i, key: key, value: _tags[key] };
});
// append blank row last, if necessary
if (!rowData.length || _showBlank) {
_showBlank = false;
rowData.push({ index: rowData.length, key: '', value: '' });
}
// append blank row last
rowData.push({ index: rowData.length, key: '', value: '' });
// View Options
@@ -160,32 +156,6 @@ export function uiSectionRawTagEditor(id, context) {
.merge(list);
// Container for the Add button
var addRowEnter = wrap.selectAll('.add-row')
.data([0])
.enter()
.append('div')
.attr('class', 'add-row' + (_tagView !== 'list' ? ' hide' : ''));
addRowEnter
.append('button')
.attr('class', 'add-tag')
.attr('aria-label', t('inspector.add_to_tag'))
.call(svgIcon('#iD-icon-plus', 'light'))
.call(uiTooltip()
.title(() => t.append('inspector.add_to_tag'))
.placement(localizer.textDirection() === 'ltr' ? 'right' : 'left'))
.on('click', addTag);
addRowEnter
.append('div')
.attr('class', 'space-value'); // preserve space
addRowEnter
.append('div')
.attr('class', 'space-buttons'); // preserve space
// Tag list items
var items = list.selectAll('.tag-row')
.data(rowData, function(d) { return d.key; });
@@ -224,11 +194,11 @@ export function uiSectionRawTagEditor(id, context) {
.call(utilNoAuto)
.on('focus', interacted)
.on('blur', valueChange)
.on('change', valueChange)
.on('keydown.push-more', pushMore);
.on('change', valueChange);
innerWrap
.append('button')
.attr('tabindex', -1)
.attr('class', 'form-field-button remove')
.attr('title', t('icons.remove'))
.call(svgIcon('#iD-operation-delete'));
@@ -240,6 +210,7 @@ export function uiSectionRawTagEditor(id, context) {
.sort(function(a, b) { return a.index - b.index; });
items
.classed('add-tag', d => d.key === '')
.each(function(d) {
var row = d3_select(this);
var key = row.select('input.key'); // propagate bound data
@@ -260,7 +231,10 @@ export function uiSectionRawTagEditor(id, context) {
}
row.select('.inner-wrap') // propagate bound data
.call(reference.button);
.call(reference.button)
.select('.tag-reference-button')
.attr('tabindex', -1)
.classed('disabled', d => d.key === ''); // disabled for blank tag line
row.call(reference.body);
@@ -269,10 +243,13 @@ export function uiSectionRawTagEditor(id, context) {
items.selectAll('input.key')
.attr('title', function(d) { return d.key; })
.call(utilGetSetValue, function(d) { return d.key; })
.attr('readonly', function(d) {
return isReadOnly(d) || null;
});
})
.call(utilGetSetValue,
d => d.key,
(_, newKey) => _pendingChange === null || isEmpty(_pendingChange) || _pendingChange[newKey] // if there are pending changes: skip untouched keys
);
items.selectAll('input.value')
.attr('title', function(d) {
@@ -284,14 +261,21 @@ export function uiSectionRawTagEditor(id, context) {
.attr('placeholder', function(d) {
return typeof d.value === 'string' ? null : t('inspector.multiple_values');
})
.call(utilGetSetValue, function(d) {
return typeof d.value === 'string' ? d.value : '';
})
.attr('readonly', function(d) {
return isReadOnly(d) || null;
});
})
.call(utilGetSetValue,
d => {
if (_pendingChange !== null && !isEmpty(_pendingChange) && !_pendingChange[d.value]) {
// if there are pending changes: skip untouched values
return null;
}
return typeof d.value === 'string' ? d.value : '';
}, (_, newValue) => newValue !== null
);
items.selectAll('button.remove')
.classed('disabled', d => d.key === '') // disabled for blank tag line
.on(('PointerEvent' in window ? 'pointer' : 'mouse') + 'down', // 'click' fires too late - #5878
(d3_event, d) => {
if (d3_event.button !== 0) return;
@@ -388,7 +372,7 @@ export function uiSectionRawTagEditor(id, context) {
}
});
if (Object.keys(_pendingChange).length === 0) {
if (isEmpty(_pendingChange)) {
_pendingChange = null;
section.reRender();
return;
@@ -397,15 +381,6 @@ export function uiSectionRawTagEditor(id, context) {
scheduleChange();
}
function pushMore(d3_event) {
// if pressing Tab on the last value field with content, add a blank row
if (d3_event.keyCode === 9 && !d3_event.shiftKey &&
section.selection().selectAll('.tag-list li:last-child input.value').node() === this &&
utilGetSetValue(d3_select(this))) {
addTag();
}
}
function bindTypeahead(key, value) {
if (isReadOnly(key.datum())) return;
@@ -574,41 +549,22 @@ export function uiSectionRawTagEditor(id, context) {
function removeTag(d3_event, d) {
if (isReadOnly(d)) return;
if (d.key === '') { // removing the blank row
_showBlank = false;
section.reRender();
// remove the key from the ordered key index
_orderedKeys = _orderedKeys.filter(function(key) { return key !== d.key; });
} else {
// remove the key from the ordered key index
_orderedKeys = _orderedKeys.filter(function(key) { return key !== d.key; });
_pendingChange = _pendingChange || {};
_pendingChange[d.key] = undefined;
scheduleChange();
}
}
function addTag() {
// Delay render in case this click is blurring an edited combo.
// Without the setTimeout, the `content` render would wipe out the pending tag change.
window.setTimeout(function() {
_showBlank = true;
section.reRender();
section.selection().selectAll('.tag-list li:last-child input.key').node().focus();
}, 20);
_pendingChange = _pendingChange || {};
_pendingChange[d.key] = undefined;
scheduleChange();
}
function scheduleChange() {
// Cache IDs in case the editor is reloaded before the change event is called. - #6028
var entityIDs = _entityIDs;
if (!_pendingChange) return;
// Delay change in case this change is blurring an edited combo. - #5878
window.setTimeout(function() {
if (!_pendingChange) return;
dispatch.call('change', this, entityIDs, _pendingChange);
_pendingChange = null;
}, 10);
for (const key in _pendingChange) {
_tags[key] = _pendingChange[key];
}
dispatch.call('change', this, _entityIDs, _pendingChange);
_pendingChange = null;
}
test/spec/ui/sections/raw_tag_editor.js
+8 -28
View File
@@ -1,5 +1,3 @@
import { setTimeout } from 'node:timers/promises';
describe('iD.uiSectionRawTagEditor', function() {
var taglist, element, entity, context;
@@ -37,40 +35,22 @@ describe('iD.uiSectionRawTagEditor', function() {
it('creates a pair of empty input elements if the entity has no tags', function () {
element.remove();
render({});
expect(element.selectAll('.tag-list li').nodes().length).to.eql(1);
expect(element.select('.tag-list').selectAll('input.value').property('value')).to.be.empty;
expect(element.select('.tag-list').selectAll('input.key').property('value')).to.be.empty;
});
it('adds tags when clicking the add button', async () => {
happen.click(element.selectAll('button.add-tag').node());
await setTimeout(20);
expect(element.select('.tag-list').selectAll('input').nodes()[2].value).to.be.empty;
expect(element.select('.tag-list').selectAll('input').nodes()[3].value).to.be.empty;
});
it('removes tags when clicking the remove button', async () => {
iD.utilTriggerEvent(element.selectAll('button.remove'), 'mousedown', { button: 0 });
const tags = await new Promise(cb => {
taglist.on('change', (_, tags) => cb(tags));
});
expect(tags).to.eql({highway: undefined});
});
it('adds tags when pressing the TAB key on last input.value', async () => {
expect(element.selectAll('.tag-list li').nodes().length).to.eql(1);
var input = d3.select('.tag-list li:last-child input.value').nodes()[0];
happen.keydown(d3.select(input).node(), {keyCode: 9});
await setTimeout(20);
it('adds pair of empty input elements at end of list', () => {
expect(element.selectAll('.tag-list li').nodes().length).to.eql(2);
expect(element.select('.tag-list').selectAll('input').nodes()[2].value).to.be.empty;
expect(element.select('.tag-list').selectAll('input').nodes()[3].value).to.be.empty;
});
it('does not add a tag when pressing TAB while shift is pressed', async () => {
expect(element.selectAll('.tag-list li').nodes().length).to.eql(1);
var input = d3.select('.tag-list li:last-child input.value').nodes()[0];
happen.keydown(d3.select(input).node(), {keyCode: 9, shiftKey: true});
await setTimeout(20);
expect(element.selectAll('.tag-list li').nodes().length).to.eql(1);
it('removes tags when clicking the remove button', async () => {
const tags = new Promise(cb => {
taglist.on('change', (_, tags) => cb(tags));
});
iD.utilTriggerEvent(element.selectAll('button.remove'), 'mousedown', { button: 0 });
expect(await tags).to.eql({highway: undefined});
});
});