New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/waypoint editing #526
Feature/waypoint editing #526
Conversation
build again |
Not necessarily specific to this, but related to the previous comment: is it intended behavior that switching to Discover Places and back re-runs your directions and gets rid of your waypoints? |
@@ -425,7 +439,7 @@ CAC.Map.Control = (function ($, Handlebars, cartodb, L, turf, _, UserPreferences | |||
}).on('dragend', function(e) { | |||
dragging = false; | |||
var coords = e.target.getLatLng(); | |||
addWaypoint(itinerary, [coords.lng, coords.lat], | |||
addWaypoint(itinerary, [coords.lat, coords.lng], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a note in the comment on addWaypoint
making it clear that the waypoint argument is [lat, lon] and the startDragPoint argument is [lon, lat].
Though the draggable markers no longer show up when there are multiple itineraries, the cursor still changes when it passes over the line. It would be extra cool if in multi-itinerary mode clicking the line selected the itinerary. Next best would be remaining unchanged to make it clear that the line is not currently interactive. |
👍 aside from the minor notes above. I took the branch name in the initial comment (flibbertigibbet:feature/waypoint-markers) to be a typo. To test this I rebased onto flibbertigibbet:feature/waypoint-urls, which is the branch for PR #525. |
0952923
to
eae59b5
Compare
For ease of use in both adding as layer to map, and for adding new waypoints.
Only allow editing itinerary if it is the only one or is the selected one.
Turn itinerary layer hover listener off when directions back button clicked.
Show markers with tooltips for existing waypoints.
Click an existing waypoint to remove it.
Drag existing waypoint marker to move.
eae59b5
to
10661b1
Compare
build again |
Prevent waypoint layer from being added multiple times when going into/back out of directions list.
Sorted issue with waypoint marker layer not always going away; it would sometimes get added multiple times if user repeatedly goes into and out of the directions detail list. |
Regarding cursor hover on the line: Leaflet does not currently allow toggling interactivity on and off for a layer (issue here). |
Display draggable markers for waypoints. Delete waypoint on marker click; move waypoint when marker dragged.
Closes #501, #521, and #520. Re-uses destination marker icon for now, until #522 can be completed.
Adds tooltips to the markers that may be sufficient for #524, unless we don't want to make the user wait for the tooltip hover delay to find out what to do.
Also fixes issue where all itineraries would be editable if multiple shown; limits itinerary editing to when only one itinerary is shown, either because only option one was returned from the trip planner, or because the user has selected the itinerary from the list. Note that if a plan query includes waypoints, only one itinerary will be returned.
To test, try adding, moving, and deleting waypoints, and clicking in and out of the directions list to verify expected behavior.
Developed this on a branch based on #525, flibbertigibbet:feature/waypoint-markers.