diff --git a/CHANGELOG.md b/CHANGELOG.md index 01cc149ec..43b609ba4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,10 +64,12 @@ _Breaking developer changes, which may affect downstream projects or sites that #### :mortar_board: Walkthrough / Help * Fix walkthrough from showing tooltips on wrong location under certain circumstances ([#10650], [#10624], [#10634]) #### :rocket: Presets +* Updated the [`cycleway`](https://osm.wiki/Key:cycleway) & [`sidewalk`](https://osm.wiki/Key:sidewalk) fields to recognise the `:both` suffix, for example [`cycleway:both`](https://osm.wiki/Key:cycleway:both) ([#9587], thanks [@k-yle]) #### :hammer: Development * Migrate unit tests from karma to vitest ([#10452]) [#9013]: https://github.com/openstreetmap/iD/issues/9013 +[#9587]: https://github.com/openstreetmap/iD/issues/9587 [#9816]: https://github.com/openstreetmap/iD/issues/9816 [#9999]: https://github.com/openstreetmap/iD/issues/9999 [#10452]: https://github.com/openstreetmap/iD/pull/10452 diff --git a/eslint.config.mjs b/eslint.config.mjs index a9772ec1a..e188d6dd8 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -115,6 +115,7 @@ export default [ 'after': 'readonly', 'd3': 'readonly', 'iD': 'readonly', + 'vi': 'readonly', 'sinon': 'readonly', 'happen': 'readonly', 'fetchMock': 'readonly', diff --git a/modules/ui/field.js b/modules/ui/field.js index ca6932a8f..9bf967e01 100644 --- a/modules/ui/field.js +++ b/modules/ui/field.js @@ -70,7 +70,9 @@ export function uiField(context, presetField, entityIDs, options) { if (field.type === 'directionalCombo' && field.key) { // directionalCombo fields can have an additional key describing the for // cases where both directions share a "common" value. - keys = keys.concat(field.key); + // The field also support *:both. The preset decides which field to write to. + const baseKey = field.key.replace(/:both$/, ''); + keys = keys.concat(baseKey, `${baseKey}:both`); } return keys; } diff --git a/modules/ui/fields/directional_combo.js b/modules/ui/fields/directional_combo.js index 9fea3b5d5..22157ec4e 100644 --- a/modules/ui/fields/directional_combo.js +++ b/modules/ui/fields/directional_combo.js @@ -85,19 +85,26 @@ export function uiFieldDirectionalCombo(field, context) { function change(key, newValue) { const commonKey = field.key; + /** if commonKey ends with :both, this is the key without :both. and vice-verca */ + const otherCommonKey = field.key.endsWith(':both') + ? field.key.replace(/:both$/, '') + : `${field.key}:both`; + const otherKey = key === field.keys[0] ? field.keys[1] : field.keys[0]; dispatch.call('change', this, tags => { - const otherValue = tags[otherKey] || tags[commonKey]; + const otherValue = tags[otherKey] || tags[commonKey] || tags[otherCommonKey]; if (newValue === otherValue) { // both tags match, use the common tag to tag both sides the same way tags[commonKey] = newValue; delete tags[key]; delete tags[otherKey]; + delete tags[otherCommonKey]; } else { // Always set both left and right as changing one can affect the other tags[key] = newValue; delete tags[commonKey]; + delete tags[otherCommonKey]; tags[otherKey] = otherValue; } return tags; @@ -108,10 +115,11 @@ export function uiFieldDirectionalCombo(field, context) { directionalCombo.tags = function(tags) { _tags = tags; - const commonKey = field.key; + const commonKey = field.key.replace(/:both$/, ''); for (let key in _combos) { const uniqueValues = [... new Set([] .concat(_tags[commonKey]) + .concat(_tags[`${commonKey}:both`]) .concat(_tags[key]) .filter(Boolean))]; _combos[key].tags({ [key]: uniqueValues.length > 1 ? uniqueValues : uniqueValues[0] }); diff --git a/test/spec/ui/fields/directional_combo.js b/test/spec/ui/fields/directional_combo.js new file mode 100644 index 000000000..88f2e9f1e --- /dev/null +++ b/test/spec/ui/fields/directional_combo.js @@ -0,0 +1,111 @@ +describe('iD.uiFieldDirectionalCombo', () => { + /** @type {iD.Context} */ + let context; + /** @type {import("d3-selection").Selection} */ + let selection; + + beforeEach(() => { + context = iD.coreContext().assetPath('../dist/').init(); + selection = d3.select(document.createElement('div')); + }); + + describe.each(['cycleway', 'cycleway:both'])('preset uses %s', (commonKey) => { + /** if commonKey ends with :both, this is the key without :both. and vice-verca */ + const otherCommonKey = commonKey.endsWith(':both') + ? commonKey.replace(/:both$/, '') + : `${commonKey}:both`; + + const field = iD.presetField('name', { + key: commonKey, + keys: ['cycleway:left', 'cycleway:right'], + }); + + it('populates the left/right fields using :left & :right', () => { + const instance = iD.uiFieldDirectionalCombo(field, context); + selection.call(instance); + instance.tags({ 'cycleway:left': 'lane' }); + + expect(selection.selectAll('input').nodes()).toHaveLength(2); + const [left, right] = selection.selectAll('input').nodes(); + expect(left.value).toBe('lane'); + expect(right.value).toBe(''); + }); + + it('populates the left/right fields using :both', () => { + const instance = iD.uiFieldDirectionalCombo(field, context); + selection.call(instance); + instance.tags({ 'cycleway:both': 'lane' }); + + expect(selection.selectAll('input').nodes()).toHaveLength(2); + const [left, right] = selection.selectAll('input').nodes(); + expect(left.value).toBe('lane'); + expect(right.value).toBe('lane'); + }); + + it('populates the left/right fields using the unprefixed tag', () => { + const instance = iD.uiFieldDirectionalCombo(field, context); + selection.call(instance); + instance.tags({ cycleway: 'lane' }); + + expect(selection.selectAll('input').nodes()).toHaveLength(2); + const [left, right] = selection.selectAll('input').nodes(); + expect(left.value).toBe('lane'); + expect(right.value).toBe('lane'); + }); + + it(`setting left & right to the same value will use the ${commonKey}`, () => { + const instance = iD.uiFieldDirectionalCombo(field, context); + selection.call(instance); + const tags = { 'cycleway:left': 'lane', 'cycleway:right': 'shoulder' }; + instance.tags(tags); + + const onChange = vi.fn(); + instance.on('change', v => onChange(v(tags))); + + expect(selection.selectAll('input').nodes()).toHaveLength(2); + const [left, right] = selection.selectAll('input').nodes(); + expect(left.value).toBe('lane'); + expect(right.value).toBe('shoulder'); + + + left.value = 'shoulder'; + d3.select(left).dispatch('change'); + + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledWith({ [commonKey]: 'shoulder' }); + }); + + it(`can read the value from ${otherCommonKey}, but writes to ${commonKey}`, () => { + const instance = iD.uiFieldDirectionalCombo(field, context); + selection.call(instance); + let tags = { [otherCommonKey]: 'lane' }; + instance.tags(tags); + + const onChange = vi.fn(); + instance.on('change', v => onChange(tags = v(tags))); + + expect(selection.selectAll('input').nodes()).toHaveLength(2); + const [left, right] = selection.selectAll('input').nodes(); + expect(left.value).toBe('lane'); + expect(right.value).toBe('lane'); + + + left.value = 'shoulder'; + d3.select(left).dispatch('change'); + + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenNthCalledWith(1, { + 'cycleway:left': 'shoulder', // left was updated + 'cycleway:right': 'lane', + }); + + right.value = 'shoulder'; + d3.select(right).dispatch('change'); + + expect(onChange).toHaveBeenCalledTimes(2); + expect(onChange).toHaveBeenNthCalledWith(2, { + [commonKey]: 'shoulder', // now left & right have been updated + }); + }); + }); +});