Raruto / leaflet-elevation

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

prevent displaying chart on resize if hidden #60

Closed gegeweb closed 4 years ago

gegeweb commented 4 years ago

Fix TypeError: this_.map is null if control not yet in map when resizing.

Fix a bug that occurs while resizing the window: the graph is displayed even if it was hidden.

Raruto commented 4 years ago

Are you sure there are no minor issues with some other code?

That line is executed after this call on the _map object

this._map.on('resize', this._resizeChart, this);
gegeweb commented 4 years ago

I think no. I've do that because I've modified my code to prevent displaying a chart when no data on resizing window so, I do that:

.on('routeselected', function(e) {
  if (!elevationControl._map) elevationControl.addTo(map);
  elevationControl.clear();
  elevationControl.addData(L.polyline(e.route.coordinates).toGeoJSON(), controlRouting.getLine());
  //elevationControl.show();
  map.addControl(controlDownload);
  let btn = L.DomUtil.get('download_route');
  L.DomUtil.removeClass(btn, 'hide');
})
.on('waypointschanged', function(e) {
  // console.log(e)
  if (!plan.isReady()) {
    let btn = L.DomUtil.get('download_route');
    L.DomUtil.addClass(btn, 'hide');
    map.removeControl(controlDownload);
    elevationControl.clear();
    //elevationControl.hide();
    if (elevationControl._map) map.removeControl(elevationControl);
  }
});

As you can see before I hid it with a hide(), but on resizing window the graph was displayed even if there was no data.

After modifying like above, I had the error when resizing the window when the control was not yet added to the map.

Raruto commented 4 years ago

As you can see before I hid it with a hide(), but on resizing window the graph was displayed even if there was no data.

You can try to do / edit as follows:

/**
 * Hacky way for handling chart resize. Deletes it and redraw chart.
 */
_resizeChart: function() {
  if (this._container.style.display == "none") return;
 ...
}

After modifying like above, I had the error when resizing the window when the control was not yet added to the map.

The following call:

map.removeControl(elevationControl);

does not necessarily end the subsequent calls to this event:

this._map.on('resize', this._resizeChart, this);

to fix it, you can try adding these lines:

/**
 * Clean up control code and related event listeners.
 * Called on control.remove().
 */
onRemove: function(map) {
  this._container = null;

  this._map.off('zoom viewreset zoomanim', this._hidePositionMarker, this);
  this._map.off('resize', this._resetView, this);
  this._map.off('resize', this._resizeChart, this);
  this._map.off('mousedown', this._resetDrag, this);

  this._map.off('eledata_added', this._updateSummary, this);
 ...
},

NB I have not checked, but probably there are some others ones that should be turned off...

gegeweb commented 4 years ago

You can try to do / edit as follows:

/**
 * Hacky way for handling chart resize. Deletes it and redraw chart.
 */
_resizeChart: function() {
  if (this._container.style.display == "none") return;
 ...
}

This solution works fine with my previous code.

Raruto commented 4 years ago

👍

Patch released in version 1.1.4