handle cycleway:both=* (#9587)

this works for any `directionalCombo` field, including `sidewalk:both`

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

eslint.config.mjs

@@ -115,6 +115,7 @@ export default [
         'after': 'readonly',
         'd3': 'readonly',
         'iD': 'readonly',
+        'vi': 'readonly',
         'sinon': 'readonly',
         'happen': 'readonly',
         'fetchMock': 'readonly',

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;
     }

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] });

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
+            });
+        });
+    });
+});