Prevent shallow copy of default options (multiple charts) #240

Raruto · 2023-04-04T15:13:38Z

Before

opts = L.setOptions(this, opts);

After

opts = L.setOptions(this, L.extend({}, structuredClone(Options), opts));

More info:

Closes: #238

closes: #238

Raruto · 2023-04-04T15:16:29Z

@flo-schn @hupe13 please let me know if you notice any strange behavior

fschn90 · 2023-04-05T09:04:21Z

works for me only when options.legend = false

when options.legend = true, the options.margin.bottom is off again:

which btw, was the same case when applying the simple workaround:
margins: { top: 30, right: 30, bottom: 30, left: 40 }
and options.legend = false

Raruto · 2023-04-05T09:44:52Z

@flo-schn be aware that to test this change you have to compile this pull request code (eg. npm run build or npm run dev ) or at least edit the same line within your local static file (eg. dist/leaflet-elevation.js).

Personally, I have tested this edit only on this file:

• examples/leaflet-elevation_multiple-maps.html

but, edited as follows (ie. setting ../dist/ and ../libs/ as base urls):

<!DOCTYPE html>
<html>

<head>
	<title>leaflet-elevation.js</title>
	<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
	<meta name="viewport" content="initial-scale=1.0, user-scalable=no" />
	<link rel="dns-prefetch" href="https://tile.openstreetmap.org">
	<link rel="dns-prefetch preconnect" href="https://unpkg.com" />
	<link rel="preload" href="https://unpkg.com/leaflet@1.7.1/dist/leaflet.js" as="script">
	<link rel="preload" href="https://unpkg.com/leaflet-ui@0.5.9/dist/leaflet-ui.js" as="script">
	
	<style>@import '../libs/fullpage.css';</style>

	<!-- leaflet-ui -->
	<script src="https://unpkg.com/leaflet@1.7.1/dist/leaflet.js"></script>
	<script src="https://unpkg.com/leaflet-ui@0.5.9/dist/leaflet-ui.js"></script>

	<!-- leaflet-elevation -->
	<link rel="stylesheet" href="../dist/leaflet-elevation.min.css" />
	<script src="../dist/leaflet-elevation.min.js"></script>

</head>

<body style="flex-direction: row; flex-wrap: wrap; gap: 10px;">

	<div id="map-1" class="leaflet-map" style="flex: calc(50% - 5px);"></div>
	<div id="map-2" class="leaflet-map" style="flex: calc(50% - 5px);"></div>
	<div id="map-3" class="leaflet-map" style="flex: 100%; height: 50%;"></div>

	<script>
		let opts = {
			map: {
				center: [41.4583, 12.7059],
				zoom: 5,
				preferCanvas: true,
				rotate: true,
				// bearing: 45,
				rotateControl: {
					closeOnZeroBearing: true
				},
				fullscreenControl: false,
				minimapControl: false,
				searchControl: false,
				locateControl: false,
				pegmanControl: false,
				zoomControl: false,
				resizerControl: false,
				layersControl: false,
			},
			elevationControl: {
				urls: {
					'map-1': "https://raruto.github.io/leaflet-elevation/examples/via-emilia.gpx",
					'map-2': "https://raruto.github.io/leaflet-elevation/examples/via-aurelia.gpx",
					'map-3': "https://raruto.github.io/leaflet-elevation/examples/demo.tcx",
				},
				options: {
					theme: "lightblue-theme",
					collapsed: false,
					autohide: false,
					autofitBounds: true,
					position: "bottomleft",
					detached: false,
					summary: "inline",
					imperial: false,
					// altitude: "disabled",
					slope: "disabled",
					speed: false,
					acceleration: false,
					time: "summary",
					legend: true,
					followMarker: true,
					almostOver: true,
					distanceMarkers: false,
					hotline: false,
				},
			},
			layersControl: {
				options: {
					collapsed: false,
				},
			},
		};

		let maps = [], charts = [];

		for (const id of ['map-1', 'map-2', 'map-3']) {
			opts.elevationControl.options.detached = ['map-3'].includes(id);

			let map = L.map(id, opts.map);

			let controlElevation = L.control.elevation(opts.elevationControl.options).addTo(map);
			let controlLayer = L.control.layers(null, null, opts.layersControl.options);

			controlElevation.on('eledata_loaded', ({layer, name}) => controlLayer.addTo(map) && layer.eachLayer((trkseg) => trkseg.feature.geometry.type != "Point" && controlLayer.addOverlay(trkseg, trkseg.feature && trkseg.feature.properties && trkseg.feature.properties.name || name)));

			controlElevation.load(opts.elevationControl.urls[id]);

			maps.push(map);
			charts.push(controlElevation);
		}
	</script>

	<!-- i18n -->
	<script>

		// Register a custom locale
		L.registerLocale('en:18', {
			"Acceleration"      : "Acceleration",
			"Altitude"          : "Elevation",
			"Slope"             : "Slope",
			"Speed"             : "Velocity",
			"Total Length: "    : "L: ",
			"Max Elevation: "   : "E Max: ",
			"Min Elevation: "   : "E Min: ",
			"Avg Elevation: "   : "E Avg: ",
			"Total Time: "      : "T: ",
			"Total Ascent: "    : "Asc: ",
			"Total Descent: "   : "Desc: ",
			"Min Slope: "       : "S Min: ",
			"Max Slope: "       : "S Max: ",
			"Avg Slope: "       : "S Avg: ",
			"Min Speed: "       : "V Min: ",
			"Max Speed: "       : "V Max: ",
			"Avg Speed: "       : "V Avg: ",
			"Min Acceleration: ": "A Min: ",
			"Max Acceleration: ": "A Max: ",
			"Avg Acceleration: ": "A Avg: ",
		});

		// Enable a custom locale
		// L.setLocale('en:18');

		// You can also override a previously defined object
		L.locales['en'] = L.extend({
			"y: "               : "",
			"x: "               : "",
			"t: "               : "",
			"T: "               : "",
			"m: "               : "",
			"v: "               : "",
			"a: "               : "",
		}, L.locales['en']);

		// Switch the language
		L.setLocale('en');

	</script>

	<a href="https://github.com/Raruto/leaflet-elevation" class="view-on-github" style="position: fixed;top: 30px;left: calc(50% - 81px);z-index: 9999;"> <img alt="View on Github" src="https://raruto.github.io/img/view-on-github.png" title="View on Github" width="163"> </a>

</body>

</html>

👋 Raruto

fschn90 · 2023-04-05T10:58:24Z

am aware of,

@flo-schn be aware that to test this change you have to compile this pull request code (eg. npm run build or npm run dev ) or at least edit the same line within your local static file (eg. dist/leaflet-elevation.js).

and did via editing file (eg. dist/leaflet-elevation.js) to test on a personal page, which results is as described above. just now double checked and am still confirming strange behavior:

when options.legend = true, the options.margin.bottom is off again

Raruto · 2023-04-23T16:44:02Z

released in v2.3.0

@flo-schn examples/leaflet-elevation_multiple-maps.html it seems to work fine to me..

BTW, I also added a dedicated test file in case anyone else wants to go deeper:

leaflet-elevation/examples/leaflet-elevation_multiple-maps.spec.js

Lines 1 to 25 in 382451f

    
           /** 
        
            * examples/leaflet-elevation_multiple-maps.html 
        
            */ 
        
           import * as assert from 'uvu/assert'; 
        
           import { suite } from '../test/setup/http_server.js'; 
        
           const test = suite('examples/leaflet-elevation_multiple-maps.html'); 
        
           /** 
        
            * @see https://github.com/Raruto/leaflet-elevation/issues/238#issuecomment-1493717835 
        
            * @see https://github.com/Raruto/leaflet-elevation/issues/238#issuecomment-1493826767 
        
            */ 
        
           test('multiple-maps', async ({ page }) => { 
        
               const { charts, default_margins } = await page.evaluate(() => new Promise(resolve => { 
        
                   resolve({ charts, default_margins: L.Control.Elevation.prototype.options.margins }); 
        
               })); 
        
               charts.forEach(chart => { assert.is(chart.options.legend, true); }); 
        
               assert.snapshot( 
        
                   JSON.stringify(default_margins), 
        
                   JSON.stringify({ top: 30, right: 30, bottom: 30, left: 40 }) 
        
               ); 
        
           }); 
        
           test.run();

👋 Raruto

Fixes a regression introduced by: #240 Closes: #255

* Replace `structuredClone` with a custom `cloneDeep` function Fixes a regression introduced by: #240 Closes: #255 * handle arrays

Prevent shallow copy of default options (multiple charts)

703fb96

closes: #238

Update control.js

9aa2d28

Raruto merged commit bfafa91 into master Apr 23, 2023

Raruto deleted the shallow-copy branch April 23, 2023 16:34

Raruto added a commit that referenced this pull request May 14, 2023

Replace structuredClone with a custom cloneDeep function

f6e5e44

Fixes a regression introduced by: #240 Closes: #255

Raruto mentioned this pull request May 14, 2023

Replace native structuredClone with custom cloneDeep function #256

Merged

Raruto added a commit that referenced this pull request May 16, 2023

Replace native structuredClone with custom cloneDeep function (#256)

b8f9733

* Replace `structuredClone` with a custom `cloneDeep` function Fixes a regression introduced by: #240 Closes: #255 * handle arrays

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent shallow copy of default options (multiple charts) #240

Prevent shallow copy of default options (multiple charts) #240

Raruto commented Apr 4, 2023

Raruto commented Apr 4, 2023

fschn90 commented Apr 5, 2023

Raruto commented Apr 5, 2023

fschn90 commented Apr 5, 2023 •

edited

Raruto commented Apr 23, 2023

Prevent shallow copy of default options (multiple charts) #240

Prevent shallow copy of default options (multiple charts) #240

Conversation

Raruto commented Apr 4, 2023

Before

After

Raruto commented Apr 4, 2023

fschn90 commented Apr 5, 2023

Raruto commented Apr 5, 2023

• examples/leaflet-elevation_multiple-maps.html

fschn90 commented Apr 5, 2023 • edited

Raruto commented Apr 23, 2023

fschn90 commented Apr 5, 2023 •

edited