Feature Request: Road surface as quality color #621
Conversation
Required fork of leaflet-hotline with a new option to disable the gradient display. This is was done so that short stretches of very bad surface adjacent to very good ones are visible. Also note the eslint-disable-line for this compat warning: URLSearchParams is not supported in Safari 7, op_mini all, IE 10, android 4.1 don't seem relevant today because those are EOL for a long time now.
There was a problem hiding this comment.
Thanks for the PR!
(The original author does not maintain it anymore so no harm in forking.)
Maybe a PR would be nice anyway to let other users know of this improvement.
URLSearchParams is not supported in Safari 7, op_mini all, IE 10, android 4.1
URLSearchParams should be polyfilled by babel/core-js. I can take care of eslint config, probably adding it to polyfills.
Supported browsers could indeed be updated, created issue #628.
Some thoughts:
- What I also still have in mind is separate color codings of highway, surface and smoothness tag values, corresponding to the Analysis tab, with distinct colors (e.g. aspalt=gray, grass=green, ground=brown, gravel=yellow, ...). Then we would have two types of surface codings and the name in code and UI would need to be more specific, e.g. surface-rating or -index. But don't know if worth changing that now in foresight while unclear if feasible or ever implemented, so I don't really mind.
- I personally don't like the simple red-green color palette, in-between colors look muddy, what about either using the default palette with yellow in the middle or the incline palette (#313)?
See also inline comments.
js/plugin/RoutingPathQuality.js
Outdated
| surface = 0.9; | ||
| } /* else { | ||
| don't change | ||
| } */ |
| case null: | ||
| break; | ||
| default: | ||
| console.warn('unhandled surface type', data.get('surface')); |
There was a problem hiding this comment.
Please comment out console warning, as with a selected choice of values there will always be warnings without any actionable implication.
| } | ||
|
|
||
| // limit normal values 0-0.9 so 1.0 can be unknown | ||
| const final = surface === null ? 1.0 : surface * 0.9; |
There was a problem hiding this comment.
surface=asphalt + smoothness=excellent looks more like unknown, probably because final = ~0.99 (1 * 1.1 * 0.9), which is closer to gray for unknown than green, example:
995c322
to
f2d034c
Compare
|
To solve this "what color is what" a bit better I added a more usable way to find the message for a certain part of the route.
Yeah sounds nice. But I feel there are to many different surface values ... so finding colors that are somewhat self-explanatory could be very hard. For example is light-grey concrete or cobblestone?
Ok. I did put much thought into the colors. |
There was a problem hiding this comment.
To solve this "what color is what" a bit better I added a more usable way to find the message for a certain part of the route.
Nice, but just finding the closest node will sometimes return a node from another road nearby. See this example, when hovering along the "Europastraße" (highway=primary) it will also highlight the two other segments in some parts:
The height marker jumps to the other segments as well, it looks like you've actually taken the code from Leaflet.Heightgraph.
A better solution might be to use Turf.nearestPointOnLine, then maybe the index or location return values can help finding the next/previous node along the line (again not closest). There are also Turf.pointToLineDistance, L.LineUtil.closestPointOnSegment and L.LineUtil.pointToSegmentDistance.
You could also rename it
Yes, my comment was just about the naming, but that can be addressed once there is actually a conflict. Both approaches are valid and not exclusive.
More accurate on routes that are shaped like an 'U' for whatever reason.
f102036
to
aae9587
Compare
|
Added the proposed changes. Less code & more reuse nice! Here the most extreme example for Heightgraph I could find (which also led me to another bug in my code ...): |
There was a problem hiding this comment.
Yes, I've used the code from Heightgraph which of course means that this has the same bug. But probably less relevant there.
Yes, I would also consider it undesired behavior there, but as the marker and info is shown at the selected point it's less of an issue.
Thanks for your contribution!
This is similar to #310
Of course the actual numbers used are highly subjective.
Notes:
This is was done so that short stretches of very bad surface adjacent to very good ones are visible.
(The original author does not maintain it anymore so no harm in forking.)
URLSearchParams is not supported in Safari 7, op_mini all, IE 10, android 4.1
don't seem relevant today. Those are all EOL maybe eslint configuration should be updated?
And supporting opera-mini seems pointless.