Skip to content

Commit

Permalink
Suggest adding an entrance=* node to fix 'disconnected highway' errors
Browse files Browse the repository at this point in the history
  • Loading branch information
k-yle committed Feb 4, 2025
1 parent 51ee85f commit 38cc04e
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 3 deletions.
7 changes: 5 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,11 @@ _Breaking developer changes, which may affect downstream projects or sites that
* Remove rarely-used keyboard shortcut <kbd>L</kbd> to prevent accidental activation of the geolocate tool ([#9999], thanks [@k-yle])
#### :camera: Street-Level
#### :white_check_mark: Validation
* Suggest adding an `entrance=*` node to fix 'disconnected highway' errors ([#10729], thanks [@k-yle])
#### :bug: Bugfixes
* Fix unsolvable validator error triggered by regional presets ([#10459])
* Fix unsolvable validator error triggered by regional presets ([#10459], thanks [@k-yle])
* 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])
* Prevent edit menu from being covered up by street level imagery or other map overlay panels ([#10495], thanks [@fahnestd])
* 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])
* Allow tiles in minimap to be slightly underzoomed, preventing them from blacking out on low map zoom levels ([#10653])
Expand Down Expand Up @@ -85,9 +86,11 @@ _Breaking developer changes, which may affect downstream projects or sites that
[#10683]: https://github.com/openstreetmap/iD/issues/10683
[#10684]: https://github.com/openstreetmap/iD/pull/10684
[#10716]: https://github.com/openstreetmap/iD/pull/10716
[#10729]: https://github.com/openstreetmap/iD/pull/10729
[@winstonsung]: https://github.com/winstonsung/
[@Nekzuris]: https://github.com/Nekzuris
[@michaelabon]: https://github.com/michaelabon
[@fahnestd]: https://github.com/fahnestd


# 2.30.4
Expand Down
2 changes: 2 additions & 0 deletions data/core.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1971,6 +1971,8 @@ en:
title: Continue drawing from start
continue_from_end:
title: Continue drawing from end
tag_as_entrance:
title: Mark the endpoint as a {presetName}
convert_to_line:
title: Convert this to a line
annotation: Converted an area to a line.
Expand Down
51 changes: 50 additions & 1 deletion modules/validations/disconnected_way.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ import { t, localizer } from '../core/localizer';
import { modeDrawLine } from '../modes/draw_line';
import { operationDelete } from '../operations/delete';
import { utilDisplayLabel } from '../util/utilDisplayLabel';
import { osmRoutableHighwayTagValues } from '../osm/tags';
import { osmPathHighwayTagValues, osmRoutableHighwayTagValues } from '../osm/tags';
import { validationIssue, validationIssueFix } from '../core/validation';
import { services } from '../services';
import { presetManager } from '../presets';
import { actionChangeTags } from '../actions';

export function validationDisconnectedWay() {
var type = 'disconnected_way';
Expand Down Expand Up @@ -45,6 +47,8 @@ export function validationDisconnectedWay() {

var textDirection = localizer.textDirection();

fixes.push(...makeTagAsEntranceFixIfAllowed(singleEntity));

var startFix = makeContinueDrawingFixIfAllowed(textDirection, singleEntity.first(), 'start');
if (startFix) fixes.push(startFix);

Expand Down Expand Up @@ -207,6 +211,51 @@ export function validationDisconnectedWay() {
});
}

/** @param {osmWay} highway */
function makeTagAsEntranceFixIfAllowed(highway) {
const fixes = [];
for (const vertexId of [highway.first(), highway.last()]) {
const vertex = graph.hasEntity(vertexId);
if (!vertex) continue; // not downloaded

const parents = graph.parentWays(vertex);

const isConnectedToBuilding = parents.some(way =>
way.id !== highway.id &&
way.tags.building &&
way.tags.building !== 'no'
);

// ineligible for this fix
if (!isConnectedToBuilding) continue;

const entranceTags = highway.tags.highway in osmPathHighwayTagValues
? { entrance: 'yes' }
: { amenity: 'parking_entrance' };

const presetName = presetManager.matchTags(entranceTags, 'vertex').name();

fixes.push(
new validationIssueFix({
icon: 'iD-icon-wrench',
title: t.append('issues.fix.tag_as_entrance.title', { presetName }),
entityIds: [vertexId],
onClick: function(context) {
const newTags = {
...graph.entity(vertexId).tags,
...entranceTags,
};
context.perform(
actionChangeTags(vertexId, newTags),
t('operations.change_tags.annotation')
);
}
})
);
}

return fixes;
}
};

validation.type = type;
Expand Down
45 changes: 45 additions & 0 deletions test/spec/validations/disconnected_way.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ describe('iD.validations.disconnected_way', function () {
expect(issue.severity).to.eql('warning');
expect(issue.entityIds).to.have.lengthOf(1);
expect(issue.entityIds[0]).to.eql('w-1');

// dynamicFixes don't have IDs, but the icons can identify the dynamicFix
expect(issue.dynamicFixes(context).map(fix => fix.icon)).toStrictEqual([
'iD-operation-continue-left',
'iD-operation-continue',
'iD-operation-delete',
]);
});

it('flags highway connected only to service area', function() {
Expand Down Expand Up @@ -93,4 +100,42 @@ describe('iD.validations.disconnected_way', function () {
expect(issues).to.have.lengthOf(0);
});

describe.each([
['unclassified', { amenity: 'parking_entrance' }],
['path', { entrance: 'yes' }]
])('highway=%s -> %s', (highway, entranceTags) => {
it('suggests adding an entrance node if the isolated highway network touches a building outline', () => {
const n1 = iD.osmNode({ id: 'n-1', loc: [1, 1] });
const n2 = iD.osmNode({ id: 'n-2', loc: [2, 2] });
const n3 = iD.osmNode({ id: 'n-3', loc: [3, 3] });
const n4 = iD.osmNode({ id: 'n-4', loc: [4, 4] });
const w1 = iD.osmWay({ id: 'w-1', nodes: ['n-1', 'n-2'], tags: { highway } });
const w2 = iD.osmWay({ id: 'w-2', nodes: ['n-1', 'n-3', 'n-4', 'n-1'], tags: { building: 'yes'} });

context.perform(
iD.actionAddEntity(n1),
iD.actionAddEntity(n2),
iD.actionAddEntity(n3),
iD.actionAddEntity(n4),
iD.actionAddEntity(w1),
iD.actionAddEntity(w2),
);

const issues = validate();
expect(issues).toHaveLength(1);

// dynamicFixes don't have IDs, but the icons can identify the dynamicFix
expect(issues[0].dynamicFixes(context).map(fix => fix.icon)).toStrictEqual([
'iD-icon-wrench',
'iD-operation-continue-left',
'iD-operation-continue',
'iD-operation-delete',
]);

// run the 'Tag as Entrance' autofix
issues[0].dynamicFixes(context)[0].onClick(context);

expect(context.graph().entity('n-1').tags).toStrictEqual(entranceTags);
});
});
});

0 comments on commit 38cc04e

Please sign in to comment.