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

Rewrite the route snapper #8

Merged
merged 33 commits into from
Nov 25, 2024
Merged

Rewrite the route snapper #8

merged 33 commits into from
Nov 25, 2024

Conversation

dabreegster
Copy link
Collaborator

@dabreegster dabreegster commented Nov 20, 2024

This is a total refresh of the idea of the route and area snapper:

  • Previously, the Rust side held all of the state and MapLibre events were forwarded at a low level. Now, most of the state and logic happens in Svelte. The Rust side becomes stateless and just calculates routes between snapped waypoints.
  • Hovering on and dragging things is now done by maplibre markers, which are rendered in the DOM on top of the map. This fixes some occasional problems trying to grab small circles, makes it much easier to style those markers, and seems to feel snappier.
  • To solve the problem of the preview line covering up everything when the user is trying to adjust the middle of the route, there are 3 distinct modes introduced now. It also handles extending the route from the start or end.
  • The user can now click a waypoint to switch between snapped/freehand, and right click to delete. This matches what people in UX studies were trying to do -- the previous behavior was confusing.
  • The two area tools are consolidated into one now; the user can mix between free and snapped waypoints!

I think this PR represents the MVP. #9 has some smaller followups that aren't critical.

Demo: Run locally or https://acteng.github.io/atip/route_snapper_next_gen/files.html?authority=TA_West%20Yorkshire%20Combined%20Authority&schema=v2

Remove the option to avoid doubling back. It doesn't have much effect on spurs in general and has only confused users

Rethink the route snapper state management from scratch -- move it to
the Svelte side. Use markers for managing waypoints. Basic idea started,
but no intermediate waypoints yet

Introduce explicit modes. Show intermediate waypoints as more markers.
Inserting them doesn't quite work yet, but started.

Switch to new stateless WASM API. Intermediate nodes are now properly insertable, but not yet draggable

Show preview when appending/prepending

Merge with draw toolbar changes (leaving some problems)
@dabreegster dabreegster force-pushed the route_snapper_next_gen branch from c82a5cb to 1ef7cc3 Compare November 21, 2024 15:38
@dabreegster dabreegster marked this pull request as ready for review November 25, 2024 14:54
Copy link
Contributor

@Pete-Y-CS Pete-Y-CS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good apart from a couple of functions which looked now unused to me

controls = "route";
} else if (feature.geometry.type == "Polygon") {
if (feature.properties.waypoints) {
$routeTool?.editExistingArea(feature as Feature<Polygon, AreaProps>);
$routeTool?.addEventListenerSuccess(onSuccess);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to now delete these functions? looks like they're no longer used to me

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addEventListenerSuccess and such? They're in the upstream package https://github.com/dabreegster/route_snapper/blob/main/route-snapper-ts/src/index.ts. We essentially stop using route-snapper-ts in this PR, using part of the WASM API directly. I'm still depending on route-snapper-ts here for now to get RouteProps and a few convenient definitions, but could maybe clean that up later too

@dabreegster dabreegster merged commit b0d5fb5 into main Nov 25, 2024
@dabreegster dabreegster deleted the route_snapper_next_gen branch November 25, 2024 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants