OpenGIS / Waymark

Waymark adds powerful mapping features to WordPress that are easy to use. Create beautiful, interactive Maps that you can customise to suit your needs.
https://www.waymark.dev
GNU General Public License v2.0
20 stars 6 forks source link

Ascent and descent metadata in elevation chart #29

Closed MaximeChallon closed 6 months ago

MaximeChallon commented 6 months ago

Hi,

During my use of Waymark on my site, I found that two pieces of information were missing from the elevation chart: total ascent and total descent. After some research, I discovered that the elevation chart was based on the library leaflet-elevation, but it seemed to be customized for Waymark as this library was condensed into a single file, leaflet-elevation.js.

The version used in Waymark is not up-to-date compared to the latest version of leaflet-elevation (which is not a problem), but leaflet-elevation now includes these ascent and descent features.

The documentation for this library includes information on ascent and descent (example; main code). Therefore, I took the liberty of proposing an improvement to Waymark so that Total Ascent and Total Descent appear in the graph data.

This merge request can be considered a simple proposal, and I see a few drawbacks:

I hope that this proposal can be integrated into a future version, Best regards,

morehawes commented 6 months ago

Thank you so much for adding these @MaximeChallon, I really appreciate it! This has definitely been suggested before and I just have not had the time to delve into this one. Your changes look great and will be included in the next release :)

The version used in Waymark is not up-to-date compared to the latest version of leaflet-elevation (which is not a problem), but leaflet-elevation now includes these ascent and descent features.

Yes that's right. Unfortunately the combination of Leaflet plugins and my own custom code/hacks got to he point where I had to nail down specific versions. I do plan on changing this in the future!

I couldn't find any regression unit tests, so I hope this doesn't introduce any bugs.

Also something I would like to rectify in the future :0)

I minified waymark-js for testing, so one of the commits in this pull request only concerns the minified file waymark-js.min.js in case the minification wasn't done correctly.

There is a built step that combines and minifies the assets (as well as some other plugin chores like adding localization/translation entries). This is done with Grunt and I will endeavour to add more information about that to the GitHub page because I want to improve the "developer experience" for the plugin.

The addition of Total Ascent and Total Descent is made for everyone: is it relevant, or should it be made an option (in shortcode or in Waymark settings)? (If it should be an option, unfortunately, it exceeds my skills. :( )

I think adding for everyone is fine for now, can revisit/add documentation on hiding with CSS if necessary.

Thanks again!