From 1c5b3b40517b987845e758f9b55143ec90f42e0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ky=E2=84=93e=20Hensel?= Date: Wed, 15 Jan 2025 21:38:36 +1100 Subject: [PATCH] use a more visible colour for oneway arrows on dark lines (#9143) --- modules/svg/defs.js | 41 +++++++++++++++++++++++++---------------- modules/svg/lines.js | 16 +++++++++++++++- test/spec/svg/lines.js | 10 +++++----- 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/modules/svg/defs.js b/modules/svg/defs.js index 0e85e52e4..a1857035c 100644 --- a/modules/svg/defs.js +++ b/modules/svg/defs.js @@ -20,28 +20,37 @@ export function svgDefs(context) { _defsSelection = selection.append('defs'); // add markers - _defsSelection - .append('marker') - .attr('id', 'ideditor-oneway-marker') - .attr('viewBox', '0 0 10 5') - .attr('refX', 2.5) - .attr('refY', 2.5) - .attr('markerWidth', 2) - .attr('markerHeight', 2) - .attr('markerUnits', 'strokeWidth') - .attr('orient', 'auto') - .append('path') - .attr('class', 'oneway-marker-path') - .attr('d', 'M 5,3 L 0,3 L 0,2 L 5,2 L 5,0 L 10,2.5 L 5,5 z') - .attr('stroke', 'none') - .attr('fill', '#000') - .attr('opacity', '0.75'); // SVG markers have to be given a colour where they're defined // (they can't inherit it from the line they're attached to), // so we need to manually define markers for each color of tag // (also, it's slightly nicer if we can control the // positioning for different tags) + + /** @param {string} name @param {string} colour */ + function addOnewayMarker(name, colour) { + _defsSelection + .append('marker') + .attr('id', `ideditor-oneway-marker-${name}`) + .attr('viewBox', '0 0 10 5') + .attr('refX', 2.5) + .attr('refY', 2.5) + .attr('markerWidth', 2) + .attr('markerHeight', 2) + .attr('markerUnits', 'strokeWidth') + .attr('orient', 'auto') + .append('path') + .attr('class', 'oneway-marker-path') + .attr('d', 'M 5,3 L 0,3 L 0,2 L 5,2 L 5,0 L 10,2.5 L 5,5 z') + .attr('stroke', 'none') + .attr('fill', colour) + .attr('opacity', '0.75'); + } + addOnewayMarker('black', '#000'); // default + addOnewayMarker('white', '#fff'); // for dark lines (bridges under construction, railways, etc.) + addOnewayMarker('pink', '#eaf'); // for dark lines where white arrows don't work + + function addSidedMarker(name, color, offset) { _defsSelection .append('marker') diff --git a/modules/svg/lines.js b/modules/svg/lines.js index 42e3a80cd..3e31f91b3 100644 --- a/modules/svg/lines.js +++ b/modules/svg/lines.js @@ -10,6 +10,17 @@ import { osmEntity } from '../osm'; import { utilArrayFlatten, utilArrayGroupBy } from '../util'; import { utilDetect } from '../util/detect'; +/** @param {{ [key: string ]: string }} tags */ +function onewayArrowColour(tags) { + // the return value must be defined in ./defs.js + if (tags.highway === 'construction' && tags.bridge) return 'white'; + if (tags.highway === 'pedestrian' && tags.bridge) return 'pink'; + if (tags.railway) return 'black'; // TODO: use a better colour + if (tags.aeroway === 'runway') return 'pink'; + + return 'black'; +} + export function svgLines(projection, context) { var detected = utilDetect(); @@ -317,7 +328,10 @@ export function svgLines(projection, context) { layergroup.selectAll('g.line-stroke-highlighted') .call(drawLineGroup, 'stroke', true); - addMarkers(layergroup, 'oneway', 'onewaygroup', onewaydata, 'url(#ideditor-oneway-marker)'); + addMarkers(layergroup, 'oneway', 'onewaygroup', onewaydata, (d) => { + const category = onewayArrowColour(graph.entity(d.id).tags); + return `url(#ideditor-oneway-marker-${category})`; + }); addMarkers(layergroup, 'sided', 'sidedgroup', sideddata, function marker(d) { var category = graph.entity(d.id).sidednessIdentifier(); diff --git a/test/spec/svg/lines.js b/test/spec/svg/lines.js index 5c11342c1..f0666c165 100644 --- a/test/spec/svg/lines.js +++ b/test/spec/svg/lines.js @@ -145,9 +145,9 @@ describe('iD.svgLines', function () { var selection = surface.selectAll('g.onewaygroup > path'); expect(selection.size()).to.eql(3); - expect(selection.nodes()[0].attributes['marker-mid'].nodeValue).to.eql('url(#ideditor-oneway-marker)'); - expect(selection.nodes()[1].attributes['marker-mid'].nodeValue).to.eql('url(#ideditor-oneway-marker)'); - expect(selection.nodes()[2].attributes['marker-mid'].nodeValue).to.eql('url(#ideditor-oneway-marker)'); + expect(selection.nodes()[0].attributes['marker-mid'].nodeValue).to.eql('url(#ideditor-oneway-marker-black)'); + expect(selection.nodes()[1].attributes['marker-mid'].nodeValue).to.eql('url(#ideditor-oneway-marker-black)'); + expect(selection.nodes()[2].attributes['marker-mid'].nodeValue).to.eql('url(#ideditor-oneway-marker-black)'); }); it('has two marker layers for alternating oneway ways', function() { @@ -162,8 +162,8 @@ describe('iD.svgLines', function () { var selection = surface.selectAll('g.onewaygroup > path'); expect(selection.size()).to.eql(2); - expect(selection.nodes()[0].attributes['marker-mid'].nodeValue).to.eql('url(#ideditor-oneway-marker)'); - expect(selection.nodes()[1].attributes['marker-mid'].nodeValue).to.eql('url(#ideditor-oneway-marker)'); + expect(selection.nodes()[0].attributes['marker-mid'].nodeValue).to.eql('url(#ideditor-oneway-marker-black)'); + expect(selection.nodes()[1].attributes['marker-mid'].nodeValue).to.eql('url(#ideditor-oneway-marker-black)'); }); it('has no marker layer for oneway=no ways', function() {