[wip] Replace permalink/Share URL feature with automatic URL rewriting on change.

[bagage]

So as talked earlier, here is a PR hopefully doing things the rightway :).

I removed "Permalink" button and Share URL menu, instead the URL of the window automatically updates on map changes (and reciprocally). Now URLs look like:

http://HOST/brouter-web/#map=16/43.6404/3.8691/OpenStreetMap/brouter?lonlats=3.869483,43.640594|3.865385,43.640858&nogos=3.867778,43.641107,180&profile=trekking

If you don't like/think it's unnecessary the part #map=zoom/x/y, I believe we could remove it and instead center the map on… the first point of lonlats? Compute a boundingbox of it? Something else?

Also, I fixed a tiny issue with circles drawing that were 10px radius due to Leaflet 1.x migration.

By the way this will break old links - but if it's mandatory I think we can handle that.

Testing/Reviews're welcome!

TODO list:

• Route doesn't show when copying link to a new page.
  It seems all layers - including vector layers - get removed by fullHash on update, probably a bug.
• Can't switch to Layers with space (?) in Firefox (switches back to first)
  I guess the idea is to have a separate layer list (including overlays) with keys instead of names, see the demo
• Single marker doesn't work anymore (useful to have a link with marker starting from home)
• Only no-gos doesn't work anymore (e.g. for re-using standard no-gos in your home area)
• I somehow don't like the /brouter? in the URL, that's for the server call, replace with a simple &?
• Support old links - yes, I would prefer to also support/convert old links
• The original leaflet-fullHash.js code in js/plugin/ should ideally be an external dependency in bower, and any modifications made by extending the original class.

[nrenner]

Thanks for working on this.

• #map=zoom/x/y is fine, also had this before
• I'm undecided about removing the Share URL, we could also keep it as a visible alternative, but I don't really mind either way

It's a tricky task, and unless I missed something, there are some issues (see first post)

[bagage]

Thanks for these comments!
There are definitively issues with it but I wanted to do a first shot, avoiding doing too much if it was plain wrong.
I'll update the TODO list as job gets done, and hopefully this would be ready soon 🎆.

Again I am not a JS developer so I am still learning a lot about best practices and such :).

[nrenner]

The original leaflet-fullHash.js code in js/plugin/ should ideally be an external dependency in bower, and any modifications made by extending the original class.

For reference, see Extending Leaflet tutorial and other examples in js/plugin/ e.g. Bing.js. It might be a bit tricky with fullHash, as it doesn't seem to use L.Class itself, just ask when it's not clear or you get stuck.

[bagage]

It tooks me some time to understand what was broken, but it seems that removing/readding layer on page reload (to select the proper layer I believe) breaks the Routing plugin. @nrenner do you have an idea how to solve that? edit: actually it's what you said in the first place… anyway!
I'll work on remaining items (extending Leaflet will probably be the last one I think).
Fixed!

[bagage]

It might be a bit tricky with fullHash, as it doesn't seem to use L.Class itself, just ask when it's not clear or you get stuck.

Indeed @nrenner, it looks tricky - would you mind helping on this actually?

[bagage]

Regarding old style URL support, would you mind taking at look at it (eg. am I doing it a correct way? Is there a more "standard" way of doing it?).

[bagage]

I somehow don't like the /brouter? in the URL, that's for the server call, replace with a simple &?

Instead of a &, I replaced it with a ? while removing the /brouter part.

[nrenner]

Will have a look at it tomorrow.

[nrenner]

Regarding extending leaflet-fullHash I needed to test it myself, so I made two examples:

• Gist, Demo - basic template with console logging
• Gist, Demo - adding a Marker

Some resources:

• Object.create() | MDN
• Vjeux » Javascript Presentation – Slides & Video - Slides 3-16
  (where Object.extend ^= L.Util.extend and JS.Class ^= L.Class)
• Leaflet Class.extend implementation
• Introducing JavaScript objects - Learn web development | MDN
• (Classes | MDN in ES6)

Let me know if that helps.

[nrenner]

Regarding old style URL support, would you mind taking at look at it (eg. am I doing it a correct way? Is there a more "standard" way of doing it?).

Looks OK to me for our use case, don't know if there is a best practice or standard way.

[nrenner]

Instead of a &, I replaced it with a ? while removing the /brouter part.

Hm, I know this is handy with split and ? is also allowed in the hash/fragment URI part, but ? has a special meaning for separating the query URI part and map=/// is basically just another parameter, which are usually separated with &.

Maybe we can use URLSearchParams - we have the polyfill already included - like I did in the marker test for more parsing convenience?

[bagage]

@nrenner Only item left is extending upstream leaflet-fullHash. I reduced to the minimal number of required modifications, but I feel like it won't be super great to extend it anyway:

• we have to rewriteL.Hash.parseHash and L.hash.update totally.
• we have to rewrite L.Hash.formatHash partially (should be Ok here though).
• It hasn't been updated for a year and is unlikely to change anytime soon since it does the job.

So we'll end up with a copy/paste of 2 functions - do you think it's still better to extend it? If you think it's worth it, I'll do it but with my current (poor) experience I don't see any benefit of doing so. Again, just asking and I appreciate your feedbacks!

[nrenner]

So we'll end up with a copy/paste of 2 functions - do you think it's still better to extend it?

Not sure what to say here. Better than copy/pasting the whole thing?

I guess for me it's about organizing the project in a certain way and to stick with it. And the current structure is:

• the js folder should only contain own code
• all external libraries used should be managed with Bower

This has the benefit that for all external code (with some limitations):

• it is clear and documented where it is from and what version is used
• the license is automatically downloaded as well (e.g. MIT requirement: "The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.")
• it allows to automatically check for updates

It hasn't been updated for a year and is unlikely to change anytime soon since it does the job.

Of course there will be no updates if the author is happy with it and everyone else just copies and modifies it in their code ;-) Looking at the open PRs of the original leaflet-hash, there sure are people who wish for improvements and have shared theirs.

Ideally, the solution to both bad options of modifying a file copy or extending and copying functions, would be to fix the original leaflet-fullHash code and make it extendable, either by creating issues or pull requests.

But I leave it up to you if you want to make this extra effort. So I'm merging this now as is and create a separate reminder to sort this out later.