Only update trip planner route and endpoints on plan button click

[bencodeorg]

I noticed that trip endpoints and routes can get out of whack when changing the start or end location of a route. For example, I saw the following as I chose different start/end locations:

This is slightly a product-y choice, but this change makes it so that the start/stop locations on the map are only updated when you actually click the "Plan Your Trip" button. This seemed sensible since the routes shown (ie, "Itinerary 1", "Itinerary 2", etc.) represent the points on the map that were selected when you last clicked the "Plan Your Trip" button.

There's also a slight timing tweak here to only render the new start/stop locations once we've received the new route from the API, such that the route and the start/stop locations change at the same time.

[coveralls]

coverage: 16.117% (+0.02%) from 16.097%
when pulling c2df0d7 on bencodeorg:ben/trip-planner-handle-start-stop-update
into d4b0cd3 on OneBusAway:main.

[bencodeorg]

@aaronbrethorst I think this is ready for review!

[aaronbrethorst]

Hey Ben! Apologies for taking so long to review this :( Nice catch on the marker sync issue - the fix to defer marker updates until after the API response makes a lot of sense. The atomic update approach (route + markers appearing together) is a good UX improvement.

A few things I noticed:

Needs attention before merge

$effect(drawRoute) in TripPlanModal.svelte

The migration from onMount to $effect has a subtle issue. Passing the function reference directly works, but $effect will re-run whenever its tracked dependencies change. Since drawRoute() accesses itineraries[activeTab], this effect will fire again when activeTab changes - which means you'll get double draws when switching tabs (once from the explicit drawRoute() call in setActiveTab(), and once from the effect reacting).

If the intent is just "draw once on mount" like the original onMount, you could do:

import { untrack } from 'svelte';

$effect(() => {
    untrack(() => drawRoute());
});

Or if you want the effect to handle all redraws, you could remove the drawRoute() call from setActiveTab() and let the effect handle it.

Minor suggestions

1. Silent failure on API error - When fetchTripPlan returns null/fails, loading clears but the user doesn't get any feedback. Might be worth a quick toast or error message.
2. Event listener cleanup - The tabSwitched listener in TripPlan.svelte:155-162 is never removed. Svelte's onMount supports returning a cleanup function:

onMount(() => {
    if (browser) {
        const handler = () => { ... };
        window.addEventListener('tabSwitched', handler);
        return () => window.removeEventListener('tabSwitched', handler);
    }
});

3. Rapid click edge case - If someone clicks "Plan Your Trip" multiple times quickly, the API calls could race. Not a big deal, but something to be aware of.

The core change looks good to me - just the $effect behavior is the main thing I'd want addressed before merging. Let me know if you have questions!