Raruto / leaflet-elevation

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

Dynamically adding checkpoints to an already shown diagram #178

Closed velocat closed 2 years ago

velocat commented 2 years ago

Subject of the issue

My issue is not related to the problem, I would be grateful if you would give me advice

I want to add checkpoints to an already existing chart. To understand how this is implemented, there were no problems:

        controlElevation.on('eledata_added', function(e) {
            var points = track.cmts;
            points.forEach(function(point,i){
                e.target._chart._registerCheckPoint({latlng: L.latLng(point.lat, point.lng), label:point.cmt})
            });
        });

in elevation option set

detached: false,
width: 600,
height: 170,
//etc

points are added, but with strange "flown away" coordinates points

As far as I understand, the coordinates are formed here: https://github.com/Raruto/leaflet-elevation/blob/3f7f4383248befd5fbec053c2883148435b89664/src/chart.js#L439-L440

I don't understand how this happens :sob:

points2

Please, if it's not difficult, tell me what my problem is? :angel:

Raruto commented 2 years ago

I think I have an idea but please could you provide a demo?

Raruto

velocat commented 2 years ago

https://github.com/velocat/leaflet-elevation/tree/test-chkpoint

Raruto commented 2 years ago

In short this is the solution: (UPDATE: see also: https://github.com/Raruto/leaflet-elevation/issues/178#issuecomment-1046081272)

/*
 * STEP 0 - define your custom points
 */
let points = [
  { lat: 55.588101696, lng: 38.144767284, cmt: "test 1" },
  { lat: 55.590132836, lng: 38.14476192, cmt: "test 2" },
  { lat: 55.59345525, lng: 38.153371811, cmt: "test 3" }
];

/*
 * STEP 1 - initaliaze map stuff
 */
let map              = new L.Map('map', opts.map);
let controlElevation = L.control.elevation(opts.elevationControl.options);

/*
 * STEP 2 - reset elevation control (optional)
 */
controlElevation.clear();
controlElevation.redraw();

/*
 * STEP 3 - register your custom checkpoints
 */
controlElevation.on('eledata_loaded', function (e) {
  points.forEach(p => controlElevation._chart._registerCheckPoint({latlng: L.latLng(p.lat, p.lng), label:p.cmt}));
});

/*
 * STEP 4 - load elevation data (gpx, tcx, geojson, ...)
 */
controlElevation.load(opts.elevationControl.url);
controlElevation.addTo(map);

Currently you are doing something like this:


/*
 * NB the following statement is executed for each new instance of L.Control.Elevation
 */
L.Control.Elevation.addInitHook(function () {
  var _this = this;

  this.on('eledata_added', function (e) { // <-- therefore each created instance will share the same checkpoints
    points.forEach(function(point,i){
      _this._chart._registerCheckPoint({latlng: L.latLng(point.lat, point.lng), label:point.cmt});
    });
  });

});

let controlElevation1 = L.control.elevation(opts.elevationControl1.options); // <-- #1 _registerCheckPoint
let controlElevation2 = L.control.elevation(opts.elevationControl2.options); // <-- #2 _registerCheckPoint

controlElevation1.load(opts.elevationControl2.url); // <-- _this != controlElevation1 (just my supposition...)
controlElevation2.load(opts.elevationControl2.url); // <-- _this != controlElevation2 (just my supposition...)

controlElevation1.addTo(map);
controlElevation2.addTo(map);

Raruto


PS try playing with clear() and redraw() regarding dynamic updating of these points (eg. if you intend to do something else after you call load())

velocat commented 2 years ago

Basically, I understood the approach.
But I showed an example with loading a track by link, and in fact, my track is formed in a script and I load it using controlElevation.addData(track.toGeoJSON()); And in this case I don't have such an event as 'eledata_loaded'. If I use this approach in the 'eledata_added' event, I will get an infinite data loading cycle...

velocat commented 2 years ago

Solved! Everything turned out to be really simple :)


  this.on('eledata_added', function (e) { 
    controlElevation.redraw();
    points.forEach(function(point,i){
      _this._chart._registerCheckPoint({latlng: L.latLng(point.lat, point.lng), label:point.cmt});
    });
  });

points3

Thank you so much for the help and the hint !!! :+1:

PS It remains only to figure out how to clear these points, with each addition, without affecting the graph data, i.e. without clear().

velocat commented 2 years ago

There was another question, in continuation of the topic.
The points are added before axis so their labels are under them.

pointsaxis

How would I change the order?

From: pointsaxis2

To: pointsaxis3

Then the result will be correct: pointsaxis4

I would appreciate a hint :angel:

P.S. Perhaps it would be more logical to change align at the label, but it seems to me that overlays can do this (although they are certainly possible anyway), and the implementation is a miscalculation of the width of the text, the margin from the edge, and the like... probably this is unnecessary, at least the correct order is sufficient so that there is no overlapping overlap

Raruto commented 2 years ago

From: pointsaxis2

To: pointsaxis3

It should be something like that:

/**
 * Move the "g.point" element one level down in the dom
 */
controlElevation.on('elechart_updated', () => {
  let pointG = controlElevation._container.querySelector("g:not(.point) > g.point");
  if (pointG.nextElementSibling && pointG.nextElementSibling.classList.contains('axis')) {
    pointG.parentNode.insertBefore(pointG.nextElementSibling, pointG);
  }
});

I honestly do not remember, but maybe I had put it before the "g.axis" pane because there was some conflict with some pointer events (or some other paint order that you are missing).

Raruto

velocat commented 2 years ago

Yes, it works, but still with the same error (periodically, every other time), i.e. if lazyload has not ended yet, then there will be an error: pointsaxis5

https://github.com/Raruto/leaflet-elevation/issues/171

Again, the only solution I see so far is to set "settimeout" for this event, but this is a "crutch".

    /**
     * Move the "g.point" element one level down in the dom
     */
    changePointsPosition = function(){
        let pointG = controlElevation._container.querySelector("g:not(.point) > g.point");
        if (pointG.nextElementSibling && pointG.nextElementSibling.classList.contains('axis')) {
            pointG.parentNode.insertBefore(pointG.nextElementSibling, pointG);
        }
    }
    controlElevation.on('elechart_updated', () => {
        try {
            changePointsPosition();
        } catch(e){
            setTimeout(changePointsPosition, 1000);
        }
    });

PS It would be great if lazyload could be completely disabled. I tried to download the library separately beforehand, but it didn't give the desired result:(

Raruto commented 2 years ago

It would be great if lazyload could be completely disabled. I tried to download the library separately beforehand, but it didn't give the desired result:(

@velocat probably related to https://github.com/Raruto/leaflet-elevation/issues/177

pointsaxis5

Anyway, it's just a missing check:

if (pointG && pointG.nextElementSibling && pointG.nextElementSibling.classList.contains('axis'))

Raruto

velocat commented 2 years ago

if (pointG && pointG.nextElementSibling && pointG.nextElementSibling.classList.contains('axis'))

But in this case, without passing the check, these points will simply not be added to the graph....
And it is possible to add them only after the chart is fully loaded :confused:

Raruto commented 2 years ago

Perhaps the elechart_updated this event is not the right one (maybe it's called too late? I'm just guessing...). You can also try with binding the elechart_init or by forcing the redraw() in some way.

Anyway, I advise you to read carefully how the following method operates, as the main logic behind the load() function call is wrapped up in there, also regarding how to wait for d3js to be currently loaded:

https://github.com/Raruto/leaflet-elevation/blob/58c427ea87764ab62215f79750805f8eb71a0c3f/src/utils.js#L70-L170

Raruto

Raruto commented 2 years ago

For those who haven't already noticed the utils function makes use of: control._registerCheckPoint, which then internally makes a call to: control._chart._registerCheckPoint:

https://github.com/Raruto/leaflet-elevation/blob/58c427ea87764ab62215f79750805f8eb71a0c3f/src/utils.js#L103-L108

https://github.com/Raruto/leaflet-elevation/blob/58c427ea87764ab62215f79750805f8eb71a0c3f/src/control.js#L765-L767


@velocat i tried as follows https://github.com/Raruto/leaflet-elevation/commit/9d71b6114d3d457d6af3ba8e06c9909b6975242b and it works without major problems

/*
 * STEP 1 - initaliaze map stuff
 */
let map              = new L.Map('map', opts.map);
let controlElevation = L.control.elevation(opts.elevationControl.options);

/*
 * STEP 2 - reset elevation control (optional)
 */
controlElevation.clear();
controlElevation.redraw();

/*
 * STEP 3 - load elevation data (gpx, tcx, geojson, ...)
 */
controlElevation.load(opts.elevationControl.url);
controlElevation.addTo(map);

/**
 * STEP 4 - Move the "g.point" element one level down in the dom
 */
controlElevation.on('elechart_updated', () => {
  let pointG = controlElevation._container.querySelector("g:not(.point) > g.point");
  if (pointG && pointG.nextElementSibling && pointG.nextElementSibling.classList.contains('axis')) {
    pointG.parentNode.insertBefore(pointG.nextElementSibling, pointG);
  }
});

/*
 * STEP 5 - register your custom checkpoints
 */
let points = [
  { lat: 55.588101696, lng: 38.144767284, cmt: "test 1" },
  { lat: 55.590132836, lng: 38.14476192, cmt: "test 2" },
  { lat: 55.59345525, lng: 38.153371811, cmt: "test 3" }
];

points.forEach(p => controlElevation._registerCheckPoint({latlng: L.latLng(p.lat, p.lng), label: p.label}));

if it interests you, here's is how you can wait for D3.js (without involving timers):

L.Control.Elevation._d3LazyLoader.then(() => {
  console.log("Executed once and always after D3.js is fully loaded") ;
});

Raruto

velocat commented 2 years ago

In principle, for myself, I solved the problems associated with the subsequent loading of these points, as I already wrote above: https://github.com/Raruto/leaflet-elevation/issues/178#issuecomment-1040772538 And also installed the loading of the d3 script in advance on the page and by installing in the options:

lazyLoadJS: false,

And since I have a non-standard way of using it, and everything works well in my case, I think it's already possible to close this issue. :)

A lot has changed, of course, in the latest versions, and there is no backward compatibility, so I had a lot of questions to adapt what was before to the new realities. For which, of course, I apologize, but I want to already provide users with a ready and working product. I still have questions, but I think I will state them separately))