From c4c829b4b0a8e34e3e13cc39ba2c9519c2add6f6 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Fri, 1 Aug 2025 18:39:01 +0200 Subject: [PATCH] Fix some nitpicks --- app/javascript/controllers/maps_controller.js | 25 ++++--- .../controllers/trips_controller.js | 4 +- app/javascript/maps/fog_of_war.js | 14 ++-- app/javascript/maps/live_map_handler.js | 2 +- app/javascript/maps/marker_factory.js | 65 +++++++++++-------- app/javascript/maps/markers.js | 8 +-- app/javascript/maps/visits.js | 17 +++-- 7 files changed, 72 insertions(+), 63 deletions(-) diff --git a/app/javascript/controllers/maps_controller.js b/app/javascript/controllers/maps_controller.js index 2a456d02..8e3349b6 100644 --- a/app/javascript/controllers/maps_controller.js +++ b/app/javascript/controllers/maps_controller.js @@ -81,7 +81,7 @@ export default class extends BaseController { this.userSettings = {}; } this.clearFogRadius = parseInt(this.userSettings.fog_of_war_meters) || 50; - this.fogLinethreshold = parseInt(this.userSettings.fog_of_war_threshold) || 90; + this.fogLineThreshold = parseInt(this.userSettings.fog_of_war_threshold) || 90; // Store route opacity as decimal (0-1) internally this.routeOpacity = parseFloat(this.userSettings.route_opacity) || 0.6; this.distanceUnit = this.userSettings.maps?.distance_unit || "km"; @@ -119,9 +119,6 @@ export default class extends BaseController { const div = L.DomUtil.create('div', 'leaflet-control-stats'); let distance = parseInt(this.element.dataset.distance) || 0; const pointsNumber = this.element.dataset.points_number || '0'; - // Original stats data loading disabled: - // let distance = parseInt(this.element.dataset.distance) || 0; - // const pointsNumber = this.element.dataset.points_number || '0'; // Convert distance to miles if user prefers miles (assuming backend sends km) if (this.distanceUnit === 'mi') { @@ -237,7 +234,7 @@ export default class extends BaseController { // Add visits buttons after calendar button to position them below this.visitsManager.addDrawerButton(); - + // Initialize Live Map Handler this.initializeLiveMapHandler(); } @@ -322,7 +319,7 @@ export default class extends BaseController { heatmapLayer: this.heatmapLayer, fogOverlay: this.fogOverlay }; - + const options = { maxPoints: 1000, routeOpacity: this.routeOpacity, @@ -330,15 +327,15 @@ export default class extends BaseController { distanceUnit: this.distanceUnit, userSettings: this.userSettings, clearFogRadius: this.clearFogRadius, - fogLinethreshold: this.fogLinethreshold, + fogLineThreshold: this.fogLineThreshold, // Pass existing data to LiveMapHandler existingMarkers: this.markers || [], existingMarkersArray: this.markersArray || [], existingHeatmapMarkers: this.heatmapMarkers || [] }; - + this.liveMapHandler = new LiveMapHandler(this.map, layers, options); - + // Enable live map handler if live mode is already enabled if (this.liveMapEnabled) { this.liveMapHandler.enable(); @@ -601,7 +598,7 @@ export default class extends BaseController { // Enable fog of war when layer is added this.fogOverlay = event.layer; if (this.markers && this.markers.length > 0) { - this.updateFog(this.markers, this.clearFogRadius, this.fogLinethreshold); + this.updateFog(this.markers, this.clearFogRadius, this.fogLineThreshold); } } @@ -718,7 +715,7 @@ export default class extends BaseController { // Update fog if enabled if (this.map.hasLayer(this.fogOverlay)) { - this.updateFog(this.markers, this.clearFogRadius, this.fogLinethreshold); + this.updateFog(this.markers, this.clearFogRadius, this.fogLineThreshold); } }) .catch(error => { @@ -756,12 +753,12 @@ export default class extends BaseController { return null; } - updateFog(markers, clearFogRadius, fogLinethreshold) { + updateFog(markers, clearFogRadius, fogLineThreshold) { const fog = document.getElementById('fog'); if (!fog) { initializeFogCanvas(this.map); } - requestAnimationFrame(() => drawFogCanvas(this.map, markers, clearFogRadius, fogLinethreshold)); + requestAnimationFrame(() => drawFogCanvas(this.map, markers, clearFogRadius, fogLineThreshold)); } initializeDrawControl() { @@ -1388,7 +1385,7 @@ export default class extends BaseController { // Initialize fog of war if enabled in settings if (this.userSettings.fog_of_war_enabled) { - this.updateFog(this.markers, this.clearFogRadius, this.fogLinethreshold); + this.updateFog(this.markers, this.clearFogRadius, this.fogLineThreshold); } // Initialize visits manager functionality diff --git a/app/javascript/controllers/trips_controller.js b/app/javascript/controllers/trips_controller.js index 1a555de1..6275716a 100644 --- a/app/javascript/controllers/trips_controller.js +++ b/app/javascript/controllers/trips_controller.js @@ -7,8 +7,8 @@ import BaseController from "./base_controller" import L from "leaflet" import { createAllMapLayers } from "../maps/layers" import { createPopupContent } from "../maps/popups" -import { showFlashMessage } from '../maps/helpers'; -import { fetchAndDisplayPhotos } from '../maps/photos'; +import { showFlashMessage } from "../maps/helpers" +import { fetchAndDisplayPhotos } from "../maps/photos" export default class extends BaseController { static targets = ["container", "startedAt", "endedAt"] diff --git a/app/javascript/maps/fog_of_war.js b/app/javascript/maps/fog_of_war.js index 49252f95..1b13dc54 100644 --- a/app/javascript/maps/fog_of_war.js +++ b/app/javascript/maps/fog_of_war.js @@ -23,7 +23,7 @@ export function initializeFogCanvas(map) { return fog; } -export function drawFogCanvas(map, markers, clearFogRadius, fogLinethreshold) { +export function drawFogCanvas(map, markers, clearFogRadius, fogLineThreshold) { const fog = document.getElementById('fog'); // Return early if fog element doesn't exist or isn't a canvas if (!fog || !(fog instanceof HTMLCanvasElement)) return; @@ -55,7 +55,7 @@ export function drawFogCanvas(map, markers, clearFogRadius, fogLinethreshold) { // 4) Mark which pts are part of a line const connected = new Array(pts.length).fill(false); for (let i = 0; i < pts.length - 1; i++) { - if (pts[i + 1].time - pts[i].time <= fogLinethreshold) { + if (pts[i + 1].time - pts[i].time <= fogLineThreshold) { connected[i] = true; connected[i + 1] = true; } @@ -78,7 +78,7 @@ export function drawFogCanvas(map, markers, clearFogRadius, fogLinethreshold) { ctx.strokeStyle = 'rgba(255,255,255,1)'; for (let i = 0; i < pts.length - 1; i++) { - if (pts[i + 1].time - pts[i].time <= fogLinethreshold) { + if (pts[i + 1].time - pts[i].time <= fogLineThreshold) { ctx.beginPath(); ctx.moveTo(pts[i].pixel.x, pts[i].pixel.y); ctx.lineTo(pts[i + 1].pixel.x, pts[i + 1].pixel.y); @@ -119,7 +119,7 @@ export function createFogOverlay() { // Draw initial fog if we have markers if (controller.markers && controller.markers.length > 0) { - drawFogCanvas(map, controller.markers, controller.clearFogRadius, controller.fogLinethreshold); + drawFogCanvas(map, controller.markers, controller.clearFogRadius, controller.fogLineThreshold); } } } @@ -134,7 +134,7 @@ export function createFogOverlay() { // Redraw fog after resize if (this._controller && this._controller.markers) { - drawFogCanvas(map, this._controller.markers, this._controller.clearFogRadius, this._controller.fogLinethreshold); + drawFogCanvas(map, this._controller.markers, this._controller.clearFogRadius, this._controller.fogLineThreshold); } } }; @@ -155,9 +155,9 @@ export function createFogOverlay() { }, // Method to update fog when markers change - updateFog: function(markers, clearFogRadius, fogLinethreshold) { + updateFog: function(markers, clearFogRadius, fogLineThreshold) { if (this._map) { - drawFogCanvas(this._map, markers, clearFogRadius, fogLinethreshold); + drawFogCanvas(this._map, markers, clearFogRadius, fogLineThreshold); } } }); diff --git a/app/javascript/maps/live_map_handler.js b/app/javascript/maps/live_map_handler.js index 19d49b9d..1232eef5 100644 --- a/app/javascript/maps/live_map_handler.js +++ b/app/javascript/maps/live_map_handler.js @@ -33,7 +33,7 @@ export class LiveMapHandler { this.distanceUnit = options.distanceUnit || 'km'; this.userSettings = options.userSettings || {}; this.clearFogRadius = options.clearFogRadius || 100; - this.fogLinethreshold = options.fogLinethreshold || 10; + this.fogLineThreshold = options.fogLineThreshold || 10; // State tracking this.isEnabled = false; diff --git a/app/javascript/maps/marker_factory.js b/app/javascript/maps/marker_factory.js index 08a86898..b4c257d5 100644 --- a/app/javascript/maps/marker_factory.js +++ b/app/javascript/maps/marker_factory.js @@ -1,12 +1,23 @@ import { createPopupContent } from "./popups"; +const MARKER_DATA_INDICES = { + LATITUDE: 0, + LONGITUDE: 1, + BATTERY: 2, + ALTITUDE: 3, + TIMESTAMP: 4, + VELOCITY: 5, + ID: 6, + COUNTRY: 7 +}; + /** * MarkerFactory - Centralized marker creation with consistent styling - * + * * This module provides reusable marker creation functions to ensure * consistent styling and prevent code duplication between different * map components. - * + * * Memory-safe: Creates fresh instances, no shared references that could * cause memory leaks. */ @@ -29,7 +40,7 @@ export function createStandardIcon(color = 'blue', size = 8) { /** * Create a basic marker for live streaming (no drag handlers, minimal features) * Memory-efficient for high-frequency creation/destruction - * + * * @param {Array} point - Point data [lat, lng, battery, altitude, timestamp, velocity, id, country] * @param {Object} options - Optional marker configuration * @returns {L.Marker} Leaflet marker instance @@ -39,7 +50,7 @@ export function createLiveMarker(point, options = {}) { const velocity = point[5] || 0; // velocity is at index 5 const markerColor = velocity < 0 ? 'orange' : 'blue'; const size = options.size || 8; - + return L.marker([lat, lng], { icon: createStandardIcon(markerColor, size), // Live markers don't need these heavy features @@ -54,8 +65,8 @@ export function createLiveMarker(point, options = {}) { /** * Create a full-featured marker with drag handlers and popups * Used for static map display where full interactivity is needed - * - * @param {Array} point - Point data [lat, lng, battery, altitude, timestamp, velocity, id, country] + * + * @param {Array} point - Point data [lat, lng, battery, altitude, timestamp, velocity, id, country] * @param {number} index - Marker index in the array * @param {Object} userSettings - User configuration * @param {string} apiKey - API key for backend operations @@ -67,7 +78,7 @@ export function createInteractiveMarker(point, index, userSettings, apiKey, rend const pointId = point[6]; // ID is at index 6 const velocity = point[5] || 0; // velocity is at index 5 const markerColor = velocity < 0 ? 'orange' : 'blue'; - + const marker = L.marker([lat, lng], { icon: createStandardIcon(markerColor), draggable: true, @@ -79,20 +90,20 @@ export function createInteractiveMarker(point, index, userSettings, apiKey, rend markerData: point, // Store the complete marker data renderer: renderer }); - + // Add popup marker.bindPopup(createPopupContent(point, userSettings.timezone, userSettings.distanceUnit)); - + // Add drag event handlers addDragHandlers(marker, apiKey, userSettings); - + return marker; } /** * Create a simplified marker with minimal features * Used for simplified rendering mode - * + * * @param {Array} point - Point data [lat, lng, battery, altitude, timestamp, velocity, id, country] * @param {Object} userSettings - User configuration (optional) * @returns {L.Marker} Leaflet marker with basic drag support @@ -101,36 +112,36 @@ export function createSimplifiedMarker(point, userSettings = {}) { const [lat, lng] = point; const velocity = point[5] || 0; const markerColor = velocity < 0 ? 'orange' : 'blue'; - + const marker = L.marker([lat, lng], { icon: createStandardIcon(markerColor), draggable: true, autoPan: true }); - + // Add popup if user settings provided if (userSettings.timezone && userSettings.distanceUnit) { marker.bindPopup(createPopupContent(point, userSettings.timezone, userSettings.distanceUnit)); } - + // Add simple drag handlers marker.on('dragstart', function() { this.closePopup(); }); - + marker.on('dragend', function(e) { const newLatLng = e.target.getLatLng(); this.setLatLng(newLatLng); this.openPopup(); }); - + return marker; } /** * Add comprehensive drag handlers to a marker * Handles polyline updates and backend synchronization - * + * * @param {L.Marker} marker - The marker to add handlers to * @param {string} apiKey - API key for backend operations * @param {Object} userSettings - User configuration @@ -140,14 +151,14 @@ function addDragHandlers(marker, apiKey, userSettings) { marker.on('dragstart', function(e) { this.closePopup(); }); - + marker.on('drag', function(e) { const newLatLng = e.target.getLatLng(); const map = e.target._map; const pointIndex = e.target.options.pointIndex; const originalLat = e.target.options.originalLat; const originalLng = e.target.options.originalLng; - + // Find polylines by iterating through all map layers map.eachLayer((layer) => { // Check if this is a LayerGroup containing polylines @@ -190,7 +201,7 @@ function addDragHandlers(marker, apiKey, userSettings) { e.target.options.originalLat = newLatLng.lat; e.target.options.originalLng = newLatLng.lng; }); - + marker.on('dragend', function(e) { const newLatLng = e.target.getLatLng(); const pointId = e.target.options.pointId; @@ -231,12 +242,12 @@ function addDragHandlers(marker, apiKey, userSettings) { const updatedMarkerData = [ parseFloat(data.latitude), parseFloat(data.longitude), - originalMarkerData[2], // battery - originalMarkerData[3], // altitude - originalMarkerData[4], // timestamp - originalMarkerData[5], // velocity - data.id, // id - originalMarkerData[7] // country + originalMarkerData[MARKER_DATA_INDICES.BATTERY], + originalMarkerData[MARKER_DATA_INDICES.ALTITUDE], + originalMarkerData[MARKER_DATA_INDICES.TIMESTAMP], + originalMarkerData[MARKER_DATA_INDICES.VELOCITY], + data.id, + originalMarkerData[MARKER_DATA_INDICES.COUNTRY] ]; // Update the marker's stored data @@ -258,4 +269,4 @@ function addDragHandlers(marker, apiKey, userSettings) { alert('Failed to update point position. Please try again.'); }); }); -} \ No newline at end of file +} diff --git a/app/javascript/maps/markers.js b/app/javascript/maps/markers.js index 12d31abd..b54b2f4a 100644 --- a/app/javascript/maps/markers.js +++ b/app/javascript/maps/markers.js @@ -1,4 +1,5 @@ import { createInteractiveMarker, createSimplifiedMarker } from "./marker_factory"; +import { haversineDistance } from "./helpers"; export function createMarkersArray(markersData, userSettings, apiKey) { // Create a canvas renderer @@ -29,11 +30,8 @@ export function createSimplifiedMarkers(markersData, renderer, userSettings) { const [prevLat, prevLon, , , prevTimestamp] = previousMarker; const timeDiff = currTimestamp - prevTimestamp; - // Note: haversineDistance function would need to be imported or implemented - // For now, using simple distance calculation - const latDiff = currLat - prevLat; - const lngDiff = currLon - prevLon; - const distance = Math.sqrt(latDiff * latDiff + lngDiff * lngDiff) * 111000; // Rough conversion to meters + // Use haversineDistance for accurate distance calculation + const distance = haversineDistance(prevLat, prevLon, currLat, currLon, 'km') * 1000; // Convert to meters // Keep the marker if it's far enough in distance or time if (distance >= distanceThreshold || timeDiff >= timeThreshold) { diff --git a/app/javascript/maps/visits.js b/app/javascript/maps/visits.js index c2f5db6b..0ceeb415 100644 --- a/app/javascript/maps/visits.js +++ b/app/javascript/maps/visits.js @@ -491,11 +491,14 @@ export class VisitsManager { // Update the drawer content if it's being opened - but don't fetch visits automatically if (this.drawerOpen) { - console.log('Drawer opened - showing placeholder message'); - // Just show a placeholder message in the drawer, don't fetch visits const container = document.getElementById('visits-list'); if (container) { - container.innerHTML = '

Enable "Suggested Visits" or "Confirmed Visits" layers to see visits data

'; + container.innerHTML = ` +
+

No visits data loaded

+

Enable "Suggested Visits" or "Confirmed Visits" layers from the map controls to view visits.

+
+ `; } } // Note: Layer visibility is now controlled by the layer control, not the drawer state @@ -578,12 +581,12 @@ export class VisitsManager { console.log('Visit circles populated - layer control will manage visibility'); console.log('visitCircles layer count:', this.visitCircles.getLayers().length); console.log('confirmedVisitCircles layer count:', this.confirmedVisitCircles.getLayers().length); - + // Check if the layers are currently enabled in the layer control and ensure they're visible const layerControl = this.map._layers; let suggestedVisitsEnabled = false; let confirmedVisitsEnabled = false; - + // Check layer control state Object.values(layerControl || {}).forEach(layer => { if (layer.name === 'Suggested Visits' && this.map.hasLayer(layer.layer)) { @@ -593,7 +596,7 @@ export class VisitsManager { confirmedVisitsEnabled = true; } }); - + console.log('Layer control state:', { suggestedVisitsEnabled, confirmedVisitsEnabled }); } catch (error) { console.error('Error fetching visits:', error); @@ -679,7 +682,7 @@ export class VisitsManager { displayVisits(visits) { // Always create map circles regardless of drawer state this.createMapCircles(visits); - + // Update drawer UI only if container exists const container = document.getElementById('visits-list'); if (!container) {