Skip to content

Fix vector map route offset and marker sync race condition #6731

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.

Sign up for GitHub

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

Open
SinghSwayam wants to merge 2 commits into
base: master
Choose a base branch
from
+31 −0
Open

Fix vector map route offset and marker sync race condition #6731

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a1adf85
Fix vector map route offset and marker sync race condition
SinghSwayam Jan 21, 2026
7823fd6
Replace fixed timeout with transitionend promise and fix lint errors
SinghSwayam Jan 21, 2026
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
31 changes: 31 additions & 0 deletions app/assets/javascripts/index/directions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,42 @@ OSM.Directions = function (map) {
$("#directions_loader").prop("hidden", false);
$("#directions_error").prop("hidden", true).empty();
$("#directions_route").prop("hidden", true);

const sidebarAnimation = new Promise((resolve) => {
const container = map.getContainer();

const onEnd = (e) => {
if (e.target === container && (e.propertyName === "width" || e.propertyName === "size")) {
container.removeEventListener("transitionend", onEnd);
resolve();
}
};

container.addEventListener("transitionend", onEnd);

// Failsafe for reduced motion or missed events
setTimeout(() => {
container.removeEventListener("transitionend", onEnd);
resolve();
}, 400);
});

map.setSidebarOverlaid(false);

controller = new AbortController();
chosenEngine.getRoute(points, controller.signal).then(async function (route) {
await sidebarLoaded();
$("#directions_route").prop("hidden", false);

await sidebarAnimation;
routeOutput.remove();
map.eachLayer((layer) => {
if (layer._glMap) {
layer._glMap.resize();
}
});
map.invalidateSize({ animate: false });
Comment on lines +145 to +150
Copy link
Copy Markdown
Member

@tomhughes tomhughes Jan 22, 2026

Choose a reason for hiding this comment

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

So the maplibre-gl plugin for leaflet doesn't respond correctly to size invalidation I assume, hence the need to manually poke those layers? Has that been reported to maplibre-gl-leaflet upstream?

Copy link
Copy Markdown
Contributor Author

@SinghSwayam SinghSwayam Jan 23, 2026

Choose a reason for hiding this comment

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

@tomhughes

Re: Upstream Issue
You are spot on. Ideally, the plugin should listen for invalidateSize and handle the context resize automatically. I saw that PR #76 (Fix resize issue of vector maps) was merged upstream, but it clearly didn't cover this specific race condition involving CSS transitions. I haven't found an open issue tracking this specific "transition desync" scenario, so I can certainly open one to track it.

Re: Generalization
That sounds like a much more robust approach. Moving the transitionend listener into OSM.loadSidebarContent (or the sidebar controller) would solve this class of race condition for all sidebar interactions, not just routing.

Would you prefer I update this PR to refactor OSM.loadSidebarContent to fire a "sidebarReady" event that we can listen to? Or should we keep this PR focused on the routing fix and handle the architecture change separately?

Copy link
Copy Markdown
Contributor

@matkoniecz matkoniecz Jan 28, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

@SinghSwayam are you reviewing and verifying LLM content before posting it as comments or PR code?

Copy link
Copy Markdown
Contributor Author

@SinghSwayam SinghSwayam Jan 28, 2026

Choose a reason for hiding this comment

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

@SinghSwayam are you reviewing and verifying LLM content before posting it as comments or PR code?

@matkoniecz I use tools to assist with drafting my comments, but I verify the content myself. The code changes here were manually written and tested I experienced locally.


routeOutput.write(route);

if (fitRoute) {
Expand Down