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

Extended summary #59

Closed gegeweb closed 4 years ago

gegeweb commented 4 years ago

Adding

(*) Perhaps it should be option too.

You can see these in action here: https://parcours.scasb.org after adding two or more points on the route (by clicking on the map or add waypoint with the control)

gegeweb commented 4 years ago

Here is a screenshot of the result of these changes:

Capture d’écran 2020-06-04 à 16 59 38
gegeweb commented 4 years ago

About summary elements and what is displaying in tooltip or marker position it should be fine to have options like these (by default):

'summary_elements': {
    'distance': true,
    'alt_max': true,
    'alt_min': true,
    'ascent': false,
    'descent': false,
    'slope_max': false,
    'slope_min':  false,
},
'tooltip_elements': {
    'alt': true,
    'slope': false,
    'distance': true,
},
'marker_elements': {
   'alt': true,
   'slope': false,
}

If you like this improvement proposal, I can keep working on it.

Raruto commented 4 years ago

About summary elements and what is displaying in tooltip or marker position it should be fine to have options like these (by default):

'summary_elements': {
    'distance': true,
    'alt_max': true,
    'alt_min': true,
    'ascent': false,
    'descent': false,
    'slope_max': false,
    'slope_min':  false,
},
'tooltip_elements': {
   'alt': true,
   'slope': false,
   'distance': true,
},
'marker_elements': {
  'alt': true,
  'slope': false,
}

I had thought about it too, but then I gave up because:


I have slightly edited your pull, try to check out the following files:

/libs/extended-ui.js /examples/leaflet-elevation_extended-ui.html

Maybe not the best or definitive solution, but I think that in this way it makes things a little more flexible / easier to extend.

Let me know, Raruto

gegeweb commented 4 years ago

It's your project, we propose, you dispose. ;) I'm going to test this on my project, but it looks good.

gegeweb commented 4 years ago

I have this error with modal summary.

Capture d’écran 2020-06-05 à 17 05 10

That is because I've no .downloadelement in summary:

this.summaryDiv.querySelector('.download')

I look how to fix that.

I've aded an event on _hideMarkerPosition() to hide the slope in extended-ui when emited.

gegeweb commented 4 years ago

How do you minimise the js file inside libs/? Everything seems now working fine with my last commit.

gegeweb commented 4 years ago

Last little "bug", the position of the tooltip:

Capture d’écran 2020-06-05 à 17 54 44

That doesn't follow exactly the y position.

I have not found how to fix, I'm still not very comfortable with D3 and the svg.

Raruto commented 4 years ago

I've aded an event on _hideMarkerPosition() to hide the slope in extended-ui when emited.

I opted for turning the _focuslabelSlope into a "svg:tspan" element, in this way all the elements inside the "svg:text" are automatically hidden.

How do you minimise the js file inside libs/?

I use the Google Closure Compiler through the atom-minify package.

Last little "bug", the position of the tooltip:

Capture d’écran 2020-06-05 à 17 54 44

That doesn't follow exactly the y position.

I rewrote that part to prevent the label overflowing at the top of the svg chart, does it work better now?

I'm still not very comfortable with D3 and the svg.

Mee to... :)

gegeweb commented 4 years ago

Looks good with your last commit, thanks!

Capture d’écran 2020-06-05 à 19 09 36

For my taste, I will leave a small vertical space between the line and the text of the position marker and a little less between the two lines of text ;) But that's a detail!

And it is effectively better to customize with these "plugins" than in the core code.

gegeweb commented 4 years ago

I use the Google Closure Compiler through the atom-minify package.

OK, I'll look if there is the same in VScode. Otherwise, i'll use Atom.

Why do you do not this step with rollup at the build step with npm? About the build step, I used it as inspiration to package my plugin. ;)

gegeweb commented 4 years ago

I'm still not very comfortable with D3 and the svg.

Mee to... :)

;)

Have you seen this libs for the tooltip? https://github.com/Caged/d3-tip It would propably more customisable with a div and html content.

Not yet play with…

gegeweb commented 4 years ago

I've pushed this version in production on my map, works fine.

Thanks a lot for the improvements! (And this plugin!)

Raruto commented 4 years ago

Why do you do not this step with rollup at the build step with npm?

Theoretically, to do things properly, each library should live in its own dedicated repository (such as the d3-tip plugin), but since those files change less frequently than the main ones (mostly I added them as sample files), all I do is use the "default" behavior of the editor (on some projects it is simply faster to work in this other way).

About the build step, I used it as inspiration to package my plugin. ;)

If it helps, you could also take a look at the source code of these repositories:

I've pushed this version in production on my map, works fine.

👍

gegeweb commented 4 years ago

If it helps, you could also take a look at the source code of these repositories:

  • leaflet ( core obviously.. )
  • leaflet-npm
  • leaflet-ui ( in particular, the plugins loader can be a viable alternative to the "traditional" way of designing / handling a leaflet project )

OK, thanks for everything!

A fortnight ago, when I started, I didn't know I'd be at this stage with this map. Or even that I would have written a plugin or contributed a bit to Leaflet Elevation!

I like free software! ;)

gegeweb commented 4 years ago

Sometimes, there are a few wrong values, but I think it's because of the data in the trace. I'll look how to smooth out the results to rule out the crazy values...

But it's more complex than calculating between two points ;)

Capture d’écran 2020-06-06 à 08 15 48
Raruto commented 4 years ago

Sometimes, there are a few wrong values, but I think it's because of the data in the trace. I'll look how to smooth out the results to rule out the crazy values...

If you are interested in deepening, you can also take a look at the following testing branch:

multiple-charts/test/leaflet-elevation.html