Skip to content

Conversation

@l5yth
Copy link
Owner

@l5yth l5yth commented Jan 7, 2026

ref #522

@l5yth l5yth marked this pull request as draft January 7, 2026 16:30
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 415a6a7fc5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +218 to +222
if (typeof mapCleanup === 'function') {
mapCleanup();
mapCleanup = null;
}
mapCleanup = await initializeNodeDetailMapPanel(content, reference, {

Choose a reason for hiding this comment

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

P2 Badge Prevent map panel from moving after overlay closes

There is no request-token check after awaiting initializeNodeDetailMapPanel, so if the user closes the overlay while the positions fetch is still in flight, the map panel can be moved into the hidden overlay after close() has already run. Because mapCleanup gets assigned after the close, the cleanup never executes and the main map stays detached until another action resets it. This shows up when /api/positions/... is slow and the user closes the overlay quickly; adding a token/visibility guard after the await (or immediately cleaning up when stale) would avoid leaving the map panel in the wrong place.

Useful? React with 👍 / 👎.

Comment on lines +308 to +312
const layerGroup = leaflet.layerGroup().addTo(map);
if (layers?.markersLayer && typeof layers.markersLayer.clearLayers === 'function') {
layers.markersLayer.clearLayers();
}
if (layers?.neighborLinesLayer && typeof layers.neighborLinesLayer.clearLayers === 'function') {

Choose a reason for hiding this comment

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

P2 Badge Restore shared map layers after clearing for history view

The node-detail map clears the shared markersLayer (and related neighbor/trace layers) to render position history, but the cleanup handler only removes the temporary layer group and resets view; it never re-renders the cleared layers. If auto-refresh is disabled or slow, closing the detail view leaves the main dashboard map empty until the next manual refresh. This happens because the shared layers are cleared unconditionally here; consider rebuilding the layers on cleanup or isolating the history markers in a separate overlay so the shared layers are preserved.

Useful? React with 👍 / 👎.

@codecov
Copy link

codecov bot commented Jan 7, 2026

Codecov Report

❌ Patch coverage is 92.02454% with 52 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
web/public/assets/js/app/node-detail-map.js 88.63% 44 Missing ⚠️
web/public/assets/js/app/node-detail-overlay.js 62.50% 6 Missing ⚠️
...ic/assets/js/app/__tests__/node-detail-map.test.js 99.03% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

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