Skip to content
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

Fix #192, #232 and #233. Refactor some comments. Don't disable interactivity for when there's an ongoing routing request. Properly cancel prev. requests #235

Merged
merged 43 commits into from
Aug 2, 2024
Merged
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
31b0558
chore(deps-dev): bump svelte-spa-router from 3.3.0 to 4.0.1
dependabot[bot] Feb 1, 2024
5dacd41
chore(deps-dev): bump eslint from 8.56.0 to 8.57.0
dependabot[bot] Mar 1, 2024
964da1e
chore(deps): bump nanoid from 5.0.4 to 5.0.6
dependabot[bot] Mar 1, 2024
5328bf0
chore(deps-dev): bump postcss-load-config from 4.0.2 to 5.0.3
dependabot[bot] Mar 1, 2024
3e01182
chore(deps-dev): bump prettier-plugin-svelte from 3.1.2 to 3.2.2
dependabot[bot] Mar 1, 2024
0e0bb8e
chore(deps-dev): bump lint-staged from 15.2.0 to 15.2.2
dependabot[bot] Mar 1, 2024
b61fec1
chore(deps-dev): bump svelte from 4.2.8 to 4.2.12
dependabot[bot] Mar 1, 2024
09595fe
chore(deps-dev): bump prettier from 3.1.1 to 3.2.5
dependabot[bot] Mar 1, 2024
1dc6481
chore(deps-dev): bump husky from 8.0.3 to 9.0.11
dependabot[bot] Mar 1, 2024
33718fc
chore(deps-dev): bump svelte-check from 3.6.2 to 3.6.8
dependabot[bot] Apr 1, 2024
ef1d11a
chore(deps-dev): bump postcss from 8.4.33 to 8.4.38
dependabot[bot] Apr 1, 2024
6763d61
chore(deps-dev): bump @tsconfig/svelte from 5.0.2 to 5.0.4
dependabot[bot] Apr 1, 2024
4bcc6f3
chore(deps-dev): bump @types/lodash from 4.14.202 to 4.17.0
dependabot[bot] Apr 1, 2024
ef13ab3
chore(deps-dev): bump autoprefixer from 10.4.16 to 10.4.19
dependabot[bot] Apr 1, 2024
1bfc31b
Merge pull request #311 from smellyshovel/dependabot/npm_and_yarn/sta…
smellyshovel Apr 20, 2024
3e8254c
Merge pull request #310 from smellyshovel/dependabot/npm_and_yarn/sta…
smellyshovel Apr 20, 2024
dc5ac33
Merge pull request #309 from smellyshovel/dependabot/npm_and_yarn/sta…
smellyshovel Apr 20, 2024
24ba592
Merge pull request #308 from smellyshovel/dependabot/npm_and_yarn/sta…
smellyshovel Apr 20, 2024
e936cae
chore(deps-dev): bump @typescript-eslint/parser from 6.18.1 to 7.7.0
dependabot[bot] Apr 20, 2024
8f61e7a
chore(deps-dev): bump tailwindcss from 3.4.1 to 3.4.3
dependabot[bot] Apr 20, 2024
8541c33
Merge pull request #313 from smellyshovel/dependabot/npm_and_yarn/sta…
smellyshovel Apr 20, 2024
9935cbd
Merge pull request #306 from smellyshovel/dependabot/npm_and_yarn/sta…
smellyshovel Apr 20, 2024
f35a686
Merge pull request #305 from smellyshovel/dependabot/npm_and_yarn/sta…
smellyshovel Apr 20, 2024
f0d4ab2
Merge pull request #300 from smellyshovel/dependabot/npm_and_yarn/sta…
smellyshovel Apr 20, 2024
3e70afd
Merge pull request #295 from smellyshovel/dependabot/npm_and_yarn/sta…
smellyshovel Apr 20, 2024
1767fb3
Merge remote-tracking branch 'smellyshovel/dependabot/npm_and_yarn/st…
smellyshovel Apr 20, 2024
93498ca
Merge remote-tracking branch 'smellyshovel/dependabot/npm_and_yarn/st…
smellyshovel Apr 20, 2024
9946327
Merge remote-tracking branch 'smellyshovel/dependabot/npm_and_yarn/st…
smellyshovel Apr 20, 2024
f0f5107
Merge remote-tracking branch 'smellyshovel/dependabot/npm_and_yarn/st…
smellyshovel Apr 20, 2024
7cd371b
Merge remote-tracking branch 'smellyshovel/dependabot/npm_and_yarn/st…
smellyshovel Apr 20, 2024
7d4c651
Merge remote-tracking branch 'smellyshovel/dependabot/npm_and_yarn/st…
smellyshovel Apr 20, 2024
e8c597a
Merge remote-tracking branch 'smellyshovel/dependabot/npm_and_yarn/st…
smellyshovel Apr 20, 2024
4d52077
Update deps and reformat the code with updated rules
smellyshovel Apr 20, 2024
dfa9452
Merge branch 'main-origin'
smellyshovel Apr 20, 2024
9768879
Update MapLibre to v4
smellyshovel Apr 20, 2024
ab8cc20
Update Vite to 4.5.3 (to fix a vulnerability)
smellyshovel Apr 20, 2024
c600c1c
Merge branch 'maplibre:main' into main
smellyshovel Apr 20, 2024
f2346c1
Merge branch 'maplibre:main' into main
smellyshovel Jul 30, 2024
84d6622
Fix #232 waypoints are not in draggable state after creating them and…
smellyshovel Jul 30, 2024
1383a14
small refactoring (for the comments). Only rely on the public interfa…
smellyshovel Aug 1, 2024
28fbaf4
Fix #233 by not awaiting for fetchDirections to finish before detachi…
smellyshovel Aug 1, 2024
1b6c8f4
Fix #233 and #234 by not disabling the interactivity for when there's…
smellyshovel Aug 1, 2024
e7c2abc
Fix #192
smellyshovel Aug 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 41 additions & 36 deletions src/directions/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,22 +128,15 @@ export default class MapLibreGlDirections extends MapLibreGlDirectionsEvented {

protected async fetchDirections(originalEvent: MapLibreGlDirectionsWaypointEvent) {
/*
* If a request from a previous fetchDirections is already running,
* we abort it as we don't need the previous value anymore
* If a request from a previous fetchDirections is already running (there's no such a check really, but this is
* implied be calling this method), we abort it as we don't need the previous value anymore.
*/
this.abortController?.abort();
const prevInteractive = this.interactive;

if (this._waypoints.length >= 2) {
this.fire(new MapLibreGlDirectionsRoutingEvent("fetchroutesstart", originalEvent));

this.abortController = new AbortController();
const signal = this.abortController.signal;
signal.onabort = () => {
this.interactive = prevInteractive;
};

this.interactive = false;

let timer;
if (this.configuration.requestTimeout !== null) {
Expand Down Expand Up @@ -221,11 +214,6 @@ export default class MapLibreGlDirections extends MapLibreGlDirectionsEvented {
}),
);
} finally {
// see #189 (https://github.com/maplibre/maplibre-gl-directions/issues/189)
if (this.abortController?.signal.reason !== "DESTROY") this.interactive = prevInteractive;

this.abortController = undefined;

clearTimeout(timer);
}

Expand Down Expand Up @@ -600,8 +588,8 @@ export default class MapLibreGlDirections extends MapLibreGlDirectionsEvented {

protected onDragMove(e: MapMouseEvent | MapTouchEvent) {
/*
* when updateOnMove is active, if this timeout ever triggers it means we are dragging,
* but not moving the mouse.
* When the `updateOnMove` option is on, if this timeout ever triggers, it means we are dragging, but not moving the
* mouse.
*/
if (this.configuration.refreshOnMove) {
clearTimeout(this.noMouseMovementTimer);
Expand All @@ -627,12 +615,13 @@ export default class MapLibreGlDirections extends MapLibreGlDirectionsEvented {

this.hoverpoint.geometry.coordinates = [e.lngLat.lng, e.lngLat.lat];
}

this.currentMousePosition = e.point;
this.draw();

/*
* If the user selected a waypoint or a routeline and routes should update while dragging,
* we initiate the live updating process.
* If the user selected a waypoint or a routeline and routes should update while dragging, we initiate the live
* updating process.
*/
if (this.configuration.refreshOnMove && !this.refreshOnMoveIsRefreshing) {
this.liveRefreshHandler(e);
Expand All @@ -641,9 +630,9 @@ export default class MapLibreGlDirections extends MapLibreGlDirectionsEvented {

protected noMouseMovementTimer?: ReturnType<typeof setTimeout>;

protected async onDragUp(e: MapMouseEvent | MapTouchEvent) {
protected onDragUp(e: MapMouseEvent | MapTouchEvent) {
/*
* if routes should update while dragging, there's some cleanup to do when releasing the mouse
* If the routes should update while dragging, there's some cleanup to do when releasing the mouse.
*/
if (this.configuration.refreshOnMove) {
clearTimeout(this.noMouseMovementTimer);
Expand All @@ -655,7 +644,7 @@ export default class MapLibreGlDirections extends MapLibreGlDirectionsEvented {

/*
* Only add a new waypoint or change the dragged one's position if the mouse has been dragged for more than the
* specified threshold. If the specified threshold's value is less than zero then treat it as if it was zero.
* configured threshold. If the specified threshold's value is less than zero then treat it as if it was zero.
*/
if (
Math.abs(e.point.x - this.dragDownPosition?.x) >
Expand All @@ -679,21 +668,20 @@ export default class MapLibreGlDirections extends MapLibreGlDirectionsEvented {
/*
* If the routing request has failed for some reason, restore the waypoint's original position.
*/
try {
await this.fetchDirections(waypointEvent);
} catch (err) {

this.fetchDirections(waypointEvent).catch((err) => {
if (!(err instanceof DOMException && err.name == "AbortError")) {
if (this.waypointBeingDraggedInitialCoordinates) {
if (this.waypointBeingDragged && this.waypointBeingDraggedInitialCoordinates) {
this.waypointBeingDragged.geometry.coordinates = this.waypointBeingDraggedInitialCoordinates;
}
}
}
});

this.waypointBeingDragged = undefined;
this.waypointBeingDraggedInitialCoordinates = undefined;
} else if (this.hoverpoint) {
/*
* If the selected route line has been dragged then add a waypoint at the previously saved index and remove the
* If the selected routeline has been dragged then add a waypoint at the previously saved index and remove the
* hoverpoint.
*/

Expand Down Expand Up @@ -726,7 +714,7 @@ export default class MapLibreGlDirections extends MapLibreGlDirectionsEvented {
this.deHighlight();

/*
* Remove the specifically assigned for the drag-related events listeners.
* Remove the listeners assigned specifically for the drag-related events.
*/
if (e.type === "touchend") {
this.map.off("touchmove", this.onDragMoveHandler);
Expand All @@ -747,6 +735,12 @@ export default class MapLibreGlDirections extends MapLibreGlDirectionsEvented {
this.map.dragPan.enable();

this.draw();

// After the moved waypoint was rendered, imitate hovering the mouse over it (because it actually keeps being
// hovered right after the drug-up event ends).
this.map.once("idle", () => {
this.onMove(e);
});
}

protected lastRequestMousePosition = {
Expand Down Expand Up @@ -798,7 +792,7 @@ export default class MapLibreGlDirections extends MapLibreGlDirectionsEvented {
],
})[0];

if (this._interactive && this.configuration.sensitiveWaypointLayers.includes(feature?.layer.id ?? "")) {
if (this.interactive && this.configuration.sensitiveWaypointLayers.includes(feature?.layer.id ?? "")) {
/*
* If a waypoint is clicked, remove it.
*/
Expand All @@ -810,7 +804,7 @@ export default class MapLibreGlDirections extends MapLibreGlDirectionsEvented {
if (~respectiveWaypointIndex) {
this._removeWaypoint(respectiveWaypointIndex, e);
}
} else if (this._interactive && this.configuration.sensitiveSnappointLayers.includes(feature?.layer.id ?? "")) {
} else if (this.interactive && this.configuration.sensitiveSnappointLayers.includes(feature?.layer.id ?? "")) {
/*
* If a snappoint is clicked, find its respective waypoint and remove it.
*/
Expand All @@ -827,24 +821,30 @@ export default class MapLibreGlDirections extends MapLibreGlDirectionsEvented {
this.configuration.sensitiveAltRoutelineLayers.includes(feature?.layer.id ?? "")
) {
/*
* If an alternative route line is clicked, set its index as the selected route's one.
* If an alternative routeline is clicked, set its index as the selected route's one.
*/

this.selectedRouteIndex = this.routelines.findIndex((routeline) => {
return !!routeline.find((segment) => {
return segment.properties?.id === feature?.properties?.id;
});
});
} else if (this._interactive && !this.configuration.sensitiveRoutelineLayers.includes(feature?.layer.id ?? "")) {
} else if (this.interactive && !this.configuration.sensitiveRoutelineLayers.includes(feature?.layer.id ?? "")) {
/*
* If the selected route line is clicked, don't add a new waypoint. Else do.
* If the selected routeline is clicked, don't add a new waypoint. Else do.
*/

this._addWaypoint([e.lngLat.lng, e.lngLat.lat], undefined, e);
}

// the selected route might have changed, so it's important not to skip its redraw
// The selected route might have changed, so it's important not to skip its redraw.
this.draw(false);

// After the added waypoint was rendered, imitate hovering the mouse over it (because it actually keeps being
// hovered right after the click event ends).
this.map.once("idle", () => {
this.onMove(e);
});
}

protected assignWaypointsCategories() {
Expand Down Expand Up @@ -952,6 +952,12 @@ export default class MapLibreGlDirections extends MapLibreGlDirectionsEvented {
this.map.off("mousemove", this.onMoveHandler);
this.map.off("click", this.onClickHandler);
}

// see #192 (https://github.com/maplibre/maplibre-gl-directions/issues/192)
// There may be cases when the interactivity gets disabled while fetching the routes. In this case it's important
// to manually restore the map's drag-pan functionality, because after the map becomes non-interactive, the event
// listeners responsible for doing that won't be fired anymore.
this.map.dragPan.enable();
}
}

Expand Down Expand Up @@ -1136,8 +1142,7 @@ export default class MapLibreGlDirections extends MapLibreGlDirectionsEvented {
* de-initializing the instance.
*/
destroy() {
// see #189 (https://github.com/maplibre/maplibre-gl-directions/issues/189)
this.abortController?.abort("DESTROY");
this.abortController?.abort();

this.clear();
this.hoverable = false;
Expand Down
Loading