Raruto / leaflet-elevation

Leaflet plugin that allows to add elevation profiles using d3js
GNU General Public License v3.0
254 stars 84 forks source link

Showing multiple maps one page lead to increased vertical chart padding (ref: `L.setOptions` and `options.margins.bottom`) #238

Closed fschn90 closed 1 year ago

fschn90 commented 1 year ago

multiple maps and elevation divs on one page lead to pecular illustration

Please see screenshots: Screenshot from 2023-03-25 10-48-21 Screenshot from 2023-03-25 10-48-45 Screenshot from 2023-03-25 10-48-57

Your environment

Steps to reproduce

<head>
<style> html, body, #map, #elevation-div { height: 100%; width: 100%; padding: 0; margin: 0; } #map { height: 500px; } #elevation-div { height: 25%; font: 12px/1.5 "Helvetica Neue", Arial, Helvetica, sans-serif; } </style>

<link rel="stylesheet" href="https://unpkg.com/@raruto/leaflet-elevation/dist/leaflet-elevation.css" />
<script src="https://unpkg.com/@raruto/leaflet-elevation/dist/leaflet-elevation.js"></script>

<script src='https://api.mapbox.com/mapbox.js/plugins/leaflet-fullscreen/v1.0.1/Leaflet.fullscreen.min.js'></script>
<link href='https://api.mapbox.com/mapbox.js/plugins/leaflet-fullscreen/v1.0.1/leaflet.fullscreen.css' rel='stylesheet' />

</head>
<body>
        <div  id="binder"></div>
        <div  id="binder_2"></div>
        <div  id="binder_3"></div>
</body>
var elevation_options = {

    // Default chart colors: theme lime-theme, magenta-theme, ...
    theme: "lightblue-theme",

    // Chart container outside/inside map container
    detached: true,

    // if (detached), the elevation chart container
    elevationDiv: "#elevation-div",

    // if (!detached) autohide chart profile on chart mouseleave
    autohide: false,

    // if (!detached) initial state of chart profile control
    collapsed: false,

    // if (!detached) control position on one of map corners
    position: "bottomright",

    // Toggle close icon visibility
    closeBtn: true,

    // Autoupdate map center on chart mouseover.
    followMarker: true,

    // Autoupdate map bounds on chart update.
    autofitBounds: true,

    // Chart distance/elevation units.
    imperial: false,

    // [Lat, Long] vs [Long, Lat] points. (leaflet default: [Lat, Long])
    reverseCoords: false,

    // Acceleration chart profile: true || "summary" || "disabled" || false
    acceleration: false,

    // Slope chart profile: true || "summary" || "disabled" || false
    slope: false,

    // Speed chart profile: true || "summary" || "disabled" || false
    speed: false,

    // Altitude chart profile: true || "summary" || "disabled" || false
    altitude: true,

    // Display time info: true || "summary" || false
    time: false,

    // Display distance info: true || "summary" || false
    distance: true,

    // Summary track info style: "inline" || "multiline" || false
    summary: 'inline',

    // Download link: "link" || false || "modal"
    downloadLink: false,

    // Toggle chart ruler filter
    ruler: true,

    // Toggle chart legend filter
    legend: true,

    // Toggle "leaflet-almostover" integration
    almostOver: true,

    // Toggle "leaflet-distance-markers" integration
    distanceMarkers: false,

    // Toggle "leaflet-hotline" integration
    hotline: true,

    // Display track datetimes: true || false
    timestamps: false,

    // Display track waypoints: true || "markers" || "dots" || false
    waypoints: true,

    // Toggle custom waypoint icons: true || { associative array of <sym> tags } || false
    wptIcons: {
      '': L.divIcon({
        className: 'elevation-waypoint-marker',
        html: '<i class="elevation-waypoint-icon"></i>',
        iconSize: [20, 20],
        iconAnchor: [8, 20],
      }),
    },

    // Toggle waypoint labels: true || "markers" || "dots" || false
    wptLabels: "markers",

    // Render chart profiles as Canvas or SVG Paths
    preferCanvas: true,

  };

    var binder = window.leafletMap = L.map(binder).setView([51.505, -0.09], 2);
    L.tileLayer('https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', {
        attribution: 'Map data &copy; <a href="http://www.osm.org">OpenStreetMap</a>'
    }).addTo(binder);
    L.control.scale().addTo(binder);
    new L.Control.Fullscreen().addTo(binder);
    var controlElevation = L.control.elevation(elevation_options).addTo(binder);
    controlElevation.load(gpx_file_1);

    var binder_2 = window.leafletMap = L.map(binder_2).setView([51.505, -0.09], 2);
    L.tileLayer('https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', {
        attribution: 'Map data &copy; <a href="http://www.osm.org">OpenStreetMap</a>'
    }).addTo(binder_2);
    L.control.scale().addTo(binder_2);
    new L.Control.Fullscreen().addTo(binder_2);
    var controlElevation = L.control.elevation(elevation_options).addTo(binder_2);
    controlElevation.load(gpx_file_2);

    var binder_3 = window.leafletMap = L.map(binder_3).setView([51.505, -0.09], 2);
    L.tileLayer('https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', {
        attribution: 'Map data &copy; <a href="http://www.osm.org">OpenStreetMap</a>'
    }).addTo(binder_3);
    L.control.scale().addTo(binder_3);
    new L.Control.Fullscreen().addTo(binder_3);
    var controlElevation = L.control.elevation(elevation_options).addTo(binder_3);
    controlElevation.load(gpx_file_3);

Expected behaviour

an elevation graph using the entire space available.

Actual behaviour

the elevation graph being too small and using only a fraction of the space available. see screenshots above.

hupe13 commented 1 year ago

I use leaflet-elavation in a WordPress plugin. There it is possible to have multiple profiles on one page. But I don't see in your code why it doesn't work. However, the tracks I used for testing are not so long as yours.

fschn90 commented 1 year ago

please see: https://github.com/Raruto/leaflet-elevation/issues/237#issuecomment-1488177345

Raruto commented 1 year ago

@flo-schn did you try using one of the files in the /examples folder as a starting point (ie. just changing the elevation settings and the gpx file as your need)

Sometimes setting these meta parameters differently can cause problems:

https://github.com/Raruto/leaflet-elevation/blob/868179234d8e9b8f7beaea57d0e34d8bd79a2622/examples/leaflet-elevation.html#L4-L23

👋 Raruto

fschn90 commented 1 year ago

hey, just did and issue persists.

see screenshot: image

when five or more maps on a page the graph doesnt use the entire elevation-div anymore and leaves some space unused at the bottom.

best, flo

hupe13 commented 1 year ago

I have 7 maps on a page (same track) - no problem. In my tests I have a page with 5 different maps, no problem also.

Raruto commented 1 year ago

I think the bug is due to that the following function L.setOptions doesn't deep merge between the default and currently provided options (ref: nested objects):

https://github.com/Raruto/leaflet-elevation/blob/868179234d8e9b8f7beaea57d0e34d8bd79a2622/src/control.js#L176

so that this value options.margins.bottom is increased with each new instance of the class:

https://github.com/Raruto/leaflet-elevation/blob/868179234d8e9b8f7beaea57d0e34d8bd79a2622/src/control.js#L192

Below some code that recreates the above situation:

<!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 'https://unpkg.com/@raruto/leaflet-elevation@2.2.8/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="https://unpkg.com/@raruto/leaflet-elevation@2.2.8/dist/leaflet-elevation.min.css" />
    <script src="https://unpkg.com/@raruto/leaflet-elevation@2.2.8/dist/leaflet-elevation.js"></script>

</head>

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

    <div id="map" class="leaflet-map"></div>
    <div id="map1" class="leaflet-map"></div>
    <div id="map2" class="leaflet-map"></div>
    <div id="map3" class="leaflet-map"></div>
    <div id="map4" class="leaflet-map"></div>
    <div id="map5" class="leaflet-map"></div>
    <div id="map6" class="leaflet-map"></div>
    <div id="map7" class="leaflet-map"></div>
    <div id="map8" class="leaflet-map"></div>
    <div id="map9" class="leaflet-map"></div>
    <div id="map10" class="leaflet-map"></div>

    <script>
        let opts = {
            map: {
                center: [41.4583, 12.7059],
                zoom: 5,
                fullscreenControl: false,
                resizerControl: true,
                preferCanvas: true,
                rotate: true,
                // bearing: 45,
                rotateControl: {
                    closeOnZeroBearing: true
                },
            },
            elevationControl: {
                url: "https://raruto.github.io/leaflet-elevation/examples/via-emilia.gpx",
                options: {
                    theme: "lightblue-theme",
                    collapsed: false,
                    autohide: false,
                    autofitBounds: true,
                    position: "bottomleft",
                    detached: true,
                    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,
                },
            },
        };

    L.Control.Elevation.addInitHook(function(){
        console.log(this.options.margins); // <-- default margins: { top: 30, right: 30, bottom: 60, left: 40 }
    });

    for(id of ['map', 'map1', 'map2', 'map3', 'map4', 'map5', 'map6', 'map7', 'map8', 'map9', 'map10']) {
        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('via-emilia.gpx');
    }

   </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>

I have 7 maps on a page (same track) - no problem. In my tests I have a page with 5 different maps, no problem also.

Could it also be that you always pass all the available options? (so in this case the reference to the options.margins is not taken from the default one but it is always created as a new one?)

👋 Raruto

hupe13 commented 1 year ago

Could it also be that you always pass all the available options? (so in this case the reference to the options.margins is not taken from the default one but it is always created as a new one?)

You are right:

https://github.com/hupe13/extensions-leaflet-map-github/blob/5d4b863c0014ebdaea65f0fb4bcc803290350110/php/elevation.php#L441-L449

Maybe I coded this for the same reason, but didn't remember that.

Raruto commented 1 year ago

Ok, so here's a simple workaround:

// Related to: https://github.com/Raruto/leaflet-elevation/issues/238#issuecomment-1493717835

// let controlElevation = L.control.elevation(opts.elevationControl.options).addTo(map);

let controlElevation = L.control.elevation({
  ...opts.elevationControl.options,
  margins: { top: 30, right: 30, bottom: 60, left: 40 } // prevent keeping a reference of default margins object
}).addTo(map);
fschn90 commented 1 year ago

thats it, does the trick. works

appreciate the support, thanks very much!!

Raruto commented 1 year ago

I reopen, because this is a bug.. 🐞 (until someone finds a solution)