Skip to content

Commit

Permalink
handle cycleway:both=*
Browse files Browse the repository at this point in the history
this works for any `directionalCombo` field, including `sidewalk:both`
  • Loading branch information
k-yle committed Jan 16, 2025
1 parent 13d28b1 commit af25225
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 4 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
* Fix unsolvable validator error triggered by regional presets ([#10459])
* Render highway direction cones only on matching parent ways ([#9013])
* Prevent edit menu from being covered up by street level imagery or other map overlay panels ([#10495])
* Fix grid lines from showing up on background map tiles in certain situations (semi-transparent tiles or fractional browser zoom level) ([#10594], thanks [@Nekzuris])
* Fix grid lines from showing up on background map tiles in certain situations (semi-transparent tiles or fractional browser zoom level) ([#10594], thanks [@Nekzuris])
* Prevent search results from sometimes getting stuck in the highlighted state when mouse-hovering the list of search results while typing ([#10661])
#### :earth_asia: Localization
* Update Sinitic languages in the Multilingual Names field ([#10488], thanks [@winstonsung])
Expand All @@ -57,10 +57,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
[#10452]: https://github.com/openstreetmap/iD/pull/10452
[#10459]: https://github.com/openstreetmap/iD/pull/10459
[#10488]: https://github.com/openstreetmap/iD/pull/10488
Expand Down
1 change: 1 addition & 0 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ export default [
'after': 'readonly',
'd3': 'readonly',
'iD': 'readonly',
'vi': 'readonly',
'sinon': 'readonly',
'happen': 'readonly',
'fetchMock': 'readonly',
Expand Down
4 changes: 3 additions & 1 deletion modules/ui/field.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
12 changes: 10 additions & 2 deletions modules/ui/fields/directional_combo.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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] });
Expand Down
111 changes: 111 additions & 0 deletions test/spec/ui/fields/directional_combo.js
Original file line number Diff line number Diff line change
@@ -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
});
});
});
});

0 comments on commit af25225

Please sign in to comment.