Fixes issue #69 (adds path and next waypoint tracking on map) #155
Conversation
There was a problem hiding this comment.
Nice work! I've noticed an error that happens after a few minutes of use (seems like an edge case). Here it is:
Finally, you should try not to include formatting changes in this PR. Useful tools for this are VSCode's commit diffs and git add -p on command line.
Keep it up!
| firstPoint: firstPoint | ||
| firstPoint: firstPoint, | ||
| planePoints: planePoints, | ||
| Awaypoint: Awaypoint |
There was a problem hiding this comment.
Better name? What's the purpose?
There was a problem hiding this comment.
Nice work with these getters and setters. However, I'd call Awaypoint something different - maybe toNextWaypoint?
client/src/components/FlightMap.js
Outdated
| @@ -508,6 +520,22 @@ const FlightPlanMap = props => { | |||
| })} | |||
| </LayerGroup> | |||
| </LayersControl.Overlay> | |||
| <LayersControl.Overlay checked name="test"> | |||
There was a problem hiding this comment.
Use a name that isn't test, so it's easier to identify layers in the overlay
| <LayerGroup> | ||
| <Polyline positions={props.getters.planePoints.slice(-20)} color="#8a2be2" weight={3} /> | ||
| {props.getters.planePoints.map((marker, index) => { | ||
| return popup(marker, index, "uavPath") |
There was a problem hiding this comment.
You should make and use custom datatypes - see how airdropBoundary is defined above. Also, note the name={props.display.airdropBoundary[1]} which solves the previous comment.
There was a problem hiding this comment.
props.display gets you the display name of a certain property. With this comment, I didn't want you to use the airdrop boundary display name for the uav path - I wanted you to create a custom property. You did this, but never gave it a display name and never used it in the name= field. So close!
| @@ -107,6 +113,7 @@ const FlightData = () => { | |||
| time: ["Time Loiter", "Time Loiter"], | |||
| jump: ["Jump", "Jump"], | |||
| uav: ["UAV", "UAV Location"], | |||
| uavPath: ["UAV Path", "UAV Path"], | |||
There was a problem hiding this comment.
Again, you should use custom datatypes to populate this
There was a problem hiding this comment.
You did in fact implement custom datatypes above (great job!), you just never added display attributes for them. This is what I'm looking for in FlightMap.js.
krishnans2006
added
type/feat
| <LayerGroup> | ||
| <Polyline positions={props.getters.planePoints.slice(-20)} color="#8a2be2" weight={3} /> | ||
| {props.getters.planePoints.map((marker, index) => { | ||
| return popup(marker, index, "uavPath") |
There was a problem hiding this comment.
props.display gets you the display name of a certain property. With this comment, I didn't want you to use the airdrop boundary display name for the uav path - I wanted you to create a custom property. You did this, but never gave it a display name and never used it in the name= field. So close!
| firstPoint: firstPoint | ||
| firstPoint: firstPoint, | ||
| planePoints: planePoints, | ||
| Awaypoint: Awaypoint |
There was a problem hiding this comment.
Nice work with these getters and setters. However, I'd call Awaypoint something different - maybe toNextWaypoint?
| @@ -107,6 +113,7 @@ const FlightData = () => { | |||
| time: ["Time Loiter", "Time Loiter"], | |||
| jump: ["Jump", "Jump"], | |||
| uav: ["UAV", "UAV Location"], | |||
| uavPath: ["UAV Path", "UAV Path"], | |||
There was a problem hiding this comment.
You did in fact implement custom datatypes above (great job!), you just never added display attributes for them. This is what I'm looking for in FlightMap.js.
Previous path is purple and target waypoint is white.