diff --git a/app/javascript/maps_maplibre/utils/settings_manager.js b/app/javascript/maps_maplibre/utils/settings_manager.js index 8daca516..8e5cbf42 100644 --- a/app/javascript/maps_maplibre/utils/settings_manager.js +++ b/app/javascript/maps_maplibre/utils/settings_manager.js @@ -39,6 +39,7 @@ const BACKEND_SETTINGS_MAP = { export class SettingsManager { static apiKey = null + static cachedSettings = null /** * Initialize settings manager with API key @@ -46,20 +47,32 @@ export class SettingsManager { */ static initialize(apiKey) { this.apiKey = apiKey + this.cachedSettings = null // Clear cache on initialization } /** * Get all settings (localStorage first, then merge with defaults) * Converts enabled_map_layers array to individual boolean flags + * Uses cached settings if available to avoid race conditions * @returns {Object} Settings object */ static getSettings() { + // Return cached settings if available + if (this.cachedSettings) { + return { ...this.cachedSettings } + } + try { const stored = localStorage.getItem(STORAGE_KEY) const settings = stored ? { ...DEFAULT_SETTINGS, ...JSON.parse(stored) } : DEFAULT_SETTINGS // Convert enabled_map_layers array to individual boolean flags - return this._expandLayerSettings(settings) + const expandedSettings = this._expandLayerSettings(settings) + + // Cache the settings + this.cachedSettings = expandedSettings + + return { ...expandedSettings } } catch (error) { console.error('Failed to load settings:', error) return DEFAULT_SETTINGS @@ -143,7 +156,7 @@ export class SettingsManager { // Convert enabled_map_layers array to individual boolean flags const expandedSettings = this._expandLayerSettings(mergedSettings) - // Save to localStorage + // Save to localStorage and cache this.saveToLocalStorage(expandedSettings) return expandedSettings @@ -154,11 +167,14 @@ export class SettingsManager { } /** - * Save all settings to localStorage + * Save all settings to localStorage and update cache * @param {Object} settings - Settings object */ static saveToLocalStorage(settings) { try { + // Update cache first + this.cachedSettings = { ...settings } + // Then save to localStorage localStorage.setItem(STORAGE_KEY, JSON.stringify(settings)) } catch (error) { console.error('Failed to save settings to localStorage:', error) @@ -230,6 +246,13 @@ export class SettingsManager { const settings = this.getSettings() settings[key] = value + // If this is a layer visibility setting, also update the enabledMapLayers array + // This ensures the array is in sync before backend save + const isLayerSetting = Object.values(LAYER_NAME_MAP).includes(key) + if (isLayerSetting) { + settings.enabledMapLayers = this._collapseLayerSettings(settings) + } + // Save to localStorage immediately this.saveToLocalStorage(settings) @@ -245,6 +268,7 @@ export class SettingsManager { static resetToDefaults() { try { localStorage.removeItem(STORAGE_KEY) + this.cachedSettings = null // Clear cache // Also reset on backend if (this.apiKey) { diff --git a/e2e/v2/map/settings.spec.js b/e2e/v2/map/settings.spec.js index 23648b31..3ae7a065 100644 --- a/e2e/v2/map/settings.spec.js +++ b/e2e/v2/map/settings.spec.js @@ -169,6 +169,78 @@ test.describe('Map Settings', () => { expect(pointsVisible).toBe(true) expect(routesVisible).toBe(true) }) + + test('rapidly toggling multiple layers without page reload persists all changes', async ({ page }) => { + await page.click('button[title="Open map settings"]') + await page.waitForTimeout(400) + await page.click('button[data-tab="layers"]') + await page.waitForTimeout(300) + + // Get all layer toggles + const pointsToggle = page.locator('label:has-text("Points")').first().locator('input.toggle') + const routesToggle = page.locator('label:has-text("Routes")').first().locator('input.toggle') + const heatmapToggle = page.locator('label:has-text("Heatmap")').first().locator('input.toggle') + + // Record initial states + const initialPoints = await pointsToggle.isChecked() + const initialRoutes = await routesToggle.isChecked() + const initialHeatmap = await heatmapToggle.isChecked() + + // Rapidly toggle all three layers without waiting between toggles + await pointsToggle.click() + await routesToggle.click() + await heatmapToggle.click() + + // Wait for settings to be saved (backend saves are async) + await page.waitForTimeout(1000) + + // Verify toggle states changed + expect(await pointsToggle.isChecked()).toBe(!initialPoints) + expect(await routesToggle.isChecked()).toBe(!initialRoutes) + expect(await heatmapToggle.isChecked()).toBe(!initialHeatmap) + + // Verify settings persisted in localStorage + const settings = await page.evaluate(() => { + return localStorage.getItem('dawarich-maps-maplibre-settings') + }) + + if (settings) { + const parsed = JSON.parse(settings) + expect(parsed.pointsVisible).toBe(!initialPoints) + expect(parsed.routesVisible).toBe(!initialRoutes) + expect(parsed.heatmapEnabled).toBe(!initialHeatmap) + + // Verify enabledMapLayers array is also updated correctly + if (!initialPoints) { + expect(parsed.enabledMapLayers).toContain('Points') + } else { + expect(parsed.enabledMapLayers).not.toContain('Points') + } + if (!initialRoutes) { + expect(parsed.enabledMapLayers).toContain('Routes') + } else { + expect(parsed.enabledMapLayers).not.toContain('Routes') + } + if (!initialHeatmap) { + expect(parsed.enabledMapLayers).toContain('Heatmap') + } else { + expect(parsed.enabledMapLayers).not.toContain('Heatmap') + } + } + + // Toggle them back rapidly + await pointsToggle.click() + await routesToggle.click() + await heatmapToggle.click() + + // Wait for settings to be saved + await page.waitForTimeout(1000) + + // Verify all states returned to initial values + expect(await pointsToggle.isChecked()).toBe(initialPoints) + expect(await routesToggle.isChecked()).toBe(initialRoutes) + expect(await heatmapToggle.isChecked()).toBe(initialHeatmap) + }) }) test.describe('Settings Persistence', () => {