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

d3, min, max, avg, precision #216

Closed hupe13 closed 2 years ago

hupe13 commented 2 years ago

Hi Raruto,

unfortunately, I was not able to reduce the size of the pull request. It's all connected somehow.

and some more.

I tested this with your examples and on my website

Raruto commented 2 years ago

Hi @hupe13, let's try to take one small step at a time (ref. https://github.com/Raruto/leaflet-elevation/pull/219 I did it a bit quickly, but if you feel like testing..)

As for: https://github.com/Raruto/leaflet-elevation/pull/216 for now I recommend you just do as follows (at least as regards the number of decimals you want to show):

const toPrecision = (x, n) => parseFloat(x.toPrecision(n)).toFixed(n);

// Save a reference of default "L.Control.Elevation" (for later use)
const elevationProto = L.extend({}, L.Control.Elevation.prototype);

// Override default "_registerHandler" behaviour.
L.Control.Elevation.include({

   // ref: https://github.com/Raruto/leaflet-elevation/blob/c58250e7c20d52490aa3a50b611dbb282ff00a57/src/control.js#L1063-L1128
  _registerHandler: function(props) {

    if (typeof props === 'object') {

      switch(props.name) {

        // ref: https://github.com/Raruto/leaflet-elevation/blob/c58250e7c20d52490aa3a50b611dbb282ff00a57/src/handlers/acceleration.js#L41-L61
        case 'acceleration':
          let label = this.options.accelerationLabel  || L._(this.options.imperial ? 'ft/s²' : 'm/s²');
          props.tooltip.chart                 = (item)        => L._("a: ") + toPrecision(item.acceleration, 2) + " " + label;
          props.tooltip.marker                = (item)        => toPrecision(item.acceleration, 2) + " " + label;
          props.summary.minacceleration.value = (track, unit) => toPrecision(track.acceleration_min || 0, 2) + ' ' + unit;
          props.summary.maxacceleration.value = (track, unit) => toPrecision(track.acceleration_max || 0, 2) + ' ' + unit;
          props.summary.avgacceleration.value = (track, unit) => toPrecision(track.acceleration_avg || 0, 2) + ' ' + unit;
        break;

        // case 'altitude':

        // case 'speed':

        // case 'slope':

        // ...

      }

    }
    elevationProto._registerHandler.apply(this, [props]);
  }
});

// Proceed as usual
var controlElevation = L.control.elevation(opts.elevationControl.options);
controlElevation.load(opts.elevationControl.url);
hupe13 commented 2 years ago

Hi Raruto, please help me a little bit. To test this I should download the branch min-speed-0. Then I should write the above in the html file? Thank you very much.

Raruto commented 2 years ago

Hi hupe, to test that snippet it is not necessary to use it on a particular branch.

I created these two pull requests just to try to split the changes you propose into self-contained changes:

Then I should write the above in the html file?

Yes, and remember, the important thing is that (the override) is done before instantiating any L.Control.Elevation object, ie:

var controlElevation = L.control.elevation(opts.elevationControl.options);
controlElevation.load(opts.elevationControl.url);

Doing this way you can always change the default behavior of this library without ever modifying a source file (as usual, coding new features is very simple but keeping them simple is the real challenge..)

hupe13 commented 2 years ago

You mean I should program the changes from my pull request in the html file?

Raruto commented 2 years ago

Be aware that snippet only concerns the portion relating to the toPrecision() function.

NB actually I'm not particularly for or against your changes, but I'm pretty sure other people might prefer other presets for this as well, (and maybe there is an easier way to solve the question, but this is the one I usually prefer at first because it doesn't require any further edits, and anyone can fastly start working right away ..)

hupe13 commented 2 years ago

Would the pull request be correct now? The code in the html file may be like this. But I still have some things to adjust for my application.

hupe13 commented 2 years ago

I will close the pull request and check it again when you post this one.