-
Notifications
You must be signed in to change notification settings - Fork 4
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
Implemented the Bus Route, Draw polyline feat, and hidden unrelated stops #31
Conversation
The feature is working correctly but there's one issue left. When we try to choose a new stop, the old bus route is not removed. So, working on the leftover issue 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.
I've noted a few places where code can be simplified or deleted but this is looking good!
src/components/map/GoogleMap.svelte
Outdated
@@ -51,20 +56,54 @@ | |||
async function loadStopsAndAddMarkers(lat, lng) { | |||
const json = await loadStopsForLocation(lat, lng); | |||
const stops = json.data.list; | |||
allStops = [...allStops, ...stops]; |
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.
be sure to de-dupe these lists as you merge them.
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.
sure, done.
src/components/map/GoogleMap.svelte
Outdated
function clearAllMarkers() { | ||
markers.forEach(({ marker, overlay, element }) => { | ||
if (marker) { | ||
marker.setMap(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.
this can be simplified to marker?.setMap(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.
sure, done the optional chaining.
if (marker) { | ||
marker.setMap(null); | ||
} | ||
if (overlay) { |
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.
same here
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.
done
src/components/map/GoogleMap.svelte
Outdated
overlay.onRemove(); | ||
} | ||
} | ||
if (element && element.parentNode) { |
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.
same here
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.
done
src/components/map/GoogleMap.svelte
Outdated
allStops.forEach((s) => addMarker(s)); | ||
} | ||
|
||
/* function markerExists(s) { |
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.
delete me
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.
deleted the component
src/components/map/GoogleMap.svelte
Outdated
@@ -107,6 +146,11 @@ | |||
container.style.position = 'absolute'; | |||
this.getPanes().overlayMouseTarget.appendChild(container); | |||
}; | |||
overlay.onRemove = function () { | |||
if (container.parentNode) { |
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.
use optional chaining here too to get rid of the if
statement
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.
removed the if
statement in place of optional chaining.
7018abc
to
6b2161f
Compare
Fixes #24 - Trip Details List and Map Overlay Fixes #29 - The bus's route should also be projected onto the map Fixes #30 - Hide unrelated stops on the map while a route line is being displayed Also, checks to see if `window.google` exists, and only loads the Google Maps library if it does not. This resolves a warning in the console. Co-authored-by: tarunsinghofficial <[email protected]>
6b2161f
to
d32218b
Compare
Fixes #30 #29 #24 issues.