-
Notifications
You must be signed in to change notification settings - Fork 10
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
Same road mode #46
base: main
Are you sure you want to change the base?
Same road mode #46
Conversation
Also closes #45 |
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.
(Sorry I've been slow to review; I'm on holiday for a while.)
Hmm, there's a logic bug. Even when the preview doesn't show a route continuing on a differently named street, I can click and create a disconnected point:
screencast.mp4
My bad, i'll try to fix the problems asap |
bez.tytuluf8.mp4this should work now |
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.
Apologies again for the slow review; I have too many things going on at work to keep up. I only partly reviewed and found a few confusing things. I'll pick back up when I can make more time.
@@ -174,6 +174,8 @@ | |||
}); | |||
let idCounter = 0; | |||
let snapTool = document.getElementById("snap-tool"); | |||
let lastRouteName = null; |
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.
Unused
@@ -384,6 +402,9 @@ export class RouteSnapper { | |||
#redraw() { | |||
if (this.loaded) { | |||
let gj = JSON.parse(this.inner.renderGeojson()); | |||
if (gj.length == 0) { |
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.
Fairly sure this is a bug. We might have been drawing something previously, then the user makes a change like deleting a waypoint, and there's no longer anything valid to draw. In that case, we still want to update MapLibre and stop drawing things.
}; | ||
|
||
if !is_connected(node1, current, &self.router) { | ||
return String::from("[]"); |
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.
In this case maybe we don't want to draw the current extension to the route (this big if block), but we still want to draw any confirmed waypoints and parts of the route
Implements #44
Performance-wise, I don't really see a difference. All tested.