Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"cycleway" field: fix multiselections tag corruption + make field reusable #9423

Merged
merged 7 commits into from
Dec 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,13 @@ _Breaking developer changes, which may affect downstream projects or sites that
#### :white_check_mark: Validation
#### :bug: Bugfixes
* Fix bug which made it impossible to change an object's preset from a sub-preset to the respective parents preset (e.g. from Driveway to Service Road) ([#9372])
* Fix corruption of (directional) `cycleway` tags when editing a multi-selection ([#9423])
#### :hourglass: Performance
#### :rocket: Presets
* Clamp degree values in `direction` fields between 0 and 359 degrees ([#9386])
* Disable increment/decrement buttons on number fields if the input value is not numeric or when there is a multi-selection with conflicting values
* Filter out misspelled taginfo suggestions in combo field ([#9397])
* Rename `cycleway` field type to `directionalCombo` and make it reusable for arbitrary directional tags ([#9423])
#### :hammer: Development
* Upgrade to Transifex API v3 ([#9375])
* Upgrade dependencies: `d3` to v7.7
Expand All @@ -60,6 +62,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
[#9390]: https://github.com/openstreetmap/iD/pull/9390
[#9392]: https://github.com/openstreetmap/iD/pull/9392
[#9397]: https://github.com/openstreetmap/iD/issues/9397
[#9423]: https://github.com/openstreetmap/iD/pull/9423
[@alanb43]: https://github.com/alanb43


Expand Down
4 changes: 2 additions & 2 deletions css/80_app.css
Original file line number Diff line number Diff line change
Expand Up @@ -1551,10 +1551,10 @@ a.hide-toggle {
}


/* Field - Access, Cycleway
/* Field - Access, DirectionalCombo
------------------------------------------------------- */
.form-field-input-access,
.form-field-input-cycleway {
.form-field-input-directionalcombo {
flex: 1 1 auto;
display: flex;
flex-flow: row wrap;
Expand Down
21 changes: 13 additions & 8 deletions modules/ui/entity_editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,19 @@ export function uiEntityEditor(context) {

var tags = Object.assign({}, entity.tags); // shallow copy

for (var k in changed) {
if (!k) continue;
var v = changed[k];
if (typeof v === 'object') {
// a "key only" tag change
tags[k] = tags[v.oldKey];
} else if (v !== undefined || tags.hasOwnProperty(k)) {
tags[k] = v;
if (typeof changed === 'function') {
// a complex callback tag change
tags = changed(tags);
} else {
for (var k in changed) {
if (!k) continue;
var v = changed[k];
if (typeof v === 'object') {
// a "key only" tag change
tags[k] = tags[v.oldKey];
} else if (v !== undefined || tags.hasOwnProperty(k)) {
tags[k] = v;
}
}
}

Expand Down
45 changes: 29 additions & 16 deletions modules/ui/fields/combo.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,22 @@ export function uiFieldCombo(field, context) {
}


function getLabelId(field, v) {
return field.hasTextForStringId(`options.${v}.title`)
? `options.${v}.title`
: `options.${v}`;
}


// returns the display value for a tag value
// (for multiCombo, tval should be the key suffix, not the entire key)
function displayValue(tval) {
tval = tval || '';

var stringsField = field.resolveReference('stringsCrossReference');
if (stringsField.hasTextForStringId('options.' + tval)) {
return stringsField.t('options.' + tval, { default: tval });
const labelId = getLabelId(stringsField, tval);
if (stringsField.hasTextForStringId(labelId)) {
return stringsField.t(labelId, { default: tval });
}

if (field.type === 'typeCombo' && tval.toLowerCase() === 'yes') {
Expand All @@ -139,8 +147,9 @@ export function uiFieldCombo(field, context) {
tval = tval || '';

var stringsField = field.resolveReference('stringsCrossReference');
if (stringsField.hasTextForStringId('options.' + tval)) {
return stringsField.t.append('options.' + tval, { default: tval });
const labelId = getLabelId(stringsField, tval);
if (stringsField.hasTextForStringId(labelId)) {
return stringsField.t(labelId, { default: tval });
}

if (field.type === 'typeCombo' && tval.toLowerCase() === 'yes') {
Expand Down Expand Up @@ -184,12 +193,13 @@ export function uiFieldCombo(field, context) {
if (!(field.options || stringsField.options)) return [];

return (field.options || stringsField.options).map(function(v) {
const labelId = getLabelId(stringsField, v);
return {
key: v,
value: stringsField.t('options.' + v, { default: v }),
title: v,
display: addComboboxIcons(stringsField.t.append('options.' + v, { default: v }), v),
klass: stringsField.hasTextForStringId('options.' + v) ? '' : 'raw-option'
value: stringsField.t(labelId, { default: v }),
title: stringsField.t(`options.${v}.description`, { default: v }),
display: addComboboxIcons(stringsField.t.append(labelId, { default: v }), v),
klass: stringsField.hasTextForStringId(labelId) ? '' : 'raw-option'
};
});
}
Expand Down Expand Up @@ -266,15 +276,17 @@ export function uiFieldCombo(field, context) {
_container.classed('empty-combobox', data.length === 0);

_comboData = data.concat(additionalOptions).map(function(d) {
var k = d.value;
if (_isMulti) k = k.replace(field.key, '');
var isLocalizable = stringsField.hasTextForStringId('options.' + k);
var label = stringsField.t('options.' + k, { default: k });
var v = d.value;
if (_isMulti) v = v.replace(field.key, '');
const labelId = getLabelId(stringsField, v);
var isLocalizable = stringsField.hasTextForStringId(labelId);
var label = stringsField.t(labelId, { default: v });
return {
key: k,
key: v,
value: label,
display: addComboboxIcons(stringsField.t.append('options.' + k, { default: k }), k),
title: isLocalizable ? k : (d.title !== label ? d.title : ''),
title: stringsField.t(`options.${v}.description`, { default:
isLocalizable ? v : (d.title !== label ? d.title : '') }),
display: addComboboxIcons(stringsField.t.append(labelId, { default: v }), v),
klass: isLocalizable ? '' : 'raw-option'
};
});
Expand Down Expand Up @@ -684,7 +696,8 @@ export function uiFieldCombo(field, context) {
}).filter(Boolean);

var showsValue = !isMixed && tags[field.key] && !(field.type === 'typeCombo' && tags[field.key] === 'yes');
var isRawValue = showsValue && !stringsField.hasTextForStringId('options.' + tags[field.key]);
var isRawValue = showsValue && !stringsField.hasTextForStringId(`options.${tags[field.key]}`)
&& !stringsField.hasTextForStringId(`options.${tags[field.key]}.title`);
var isKnownValue = showsValue && !isRawValue;

var isReadOnly = !_allowCustomValues || isKnownValue;
Expand Down
170 changes: 0 additions & 170 deletions modules/ui/fields/cycleway.js

This file was deleted.

Loading