Conversation
josh-willis-arcadis
added
the
BLOCKED
josh-willis-arcadis
requested review from
binh-dam-ibigroup and
miles-grant-ibigroup
miles-grant-ibigroup
left a comment
miles-grant-ibigroup
left a comment
There was a problem hiding this comment.
I see quite a bit of potential to reduce the number of files touched. I'd start by leaving followStreets untouched. That will also make the edits to valhalla less dangerous. We can leave that file mostly untouched and merely add a short check after the alternates section to switch to rail if followRail is true.
That way we can also configure rail routing within alternates. Just have to add a rail flag on each graphhopper alternate!
| graphHopperKey = KEY | ||
| } else { // no URL or need default value so use graphhopper test environment | ||
| graphHopperUrl = 'https://graphhopper.com/api/1/' | ||
| // include key here??? this will throw 401 i believe |
There was a problem hiding this comment.
You deleted the code that makes it required to have a graphhopper key! That's a required param
There was a problem hiding this comment.
the rail routing server doesn't require a key. only the street routing server.
miles-grant-ibigroup
removed
the
BLOCKED
miles-grant-ibigroup
left a comment
miles-grant-ibigroup
left a comment
There was a problem hiding this comment.
I'm noticing that the default OpenRailServer server is missing, but maybe that's ok? A few small nitpicks and cleanups but once those are resolved this is good to go!
binh-dam-ibigroup
left a comment
binh-dam-ibigroup
left a comment
There was a problem hiding this comment.
I think you are missing a change to fetch a polyline with the followRail option (see comment). See also other suggested refactors.
binh-dam-ibigroup
left a comment
binh-dam-ibigroup
left a comment
There was a problem hiding this comment.
Much better now, and all modes work. Thanks for the all the work.
| componentClass='select' | ||
| value={currentSelection} | ||
| name='followOption' | ||
| onChange={this._onFollowOptionChange}> |
There was a problem hiding this comment.
Nit - Sort props and put closing > on new line.
binh-dam-ibigroup
left a comment
binh-dam-ibigroup
left a comment
There was a problem hiding this comment.
Apologies, one requirement we overlooked. Could you "enable “Snap To Rail” function when editing routes with rail GTFS route_type (0 - Light Rail/Streetcar, 1 - Subway/Metro , 2 - Rail) and not enable it for all other route_type values", please. Thank you!
| render () { | ||
| // Edit Settings passed in are present | ||
| const {editSettings, patternSegment, updateEditSetting} = this.props | ||
| const {editSettings, patternSegment, updateEditSetting, activeEntity} = this.props |
3 participants
Add support for a snap to rail feature when editing trip patterns. Uses OpenRailRouter, a fork of GraphHopper.