search

Raruto / leaflet-rotate

Leaflet plugin that allows to add rotation functionality to map tiles
GNU General Public License v3.0
81 stars 23 forks source link

Prevent map from rotating when user just wants to zoom #48

Open mauriciabad opened 8 months ago

mauriciabad commented 8 months ago

The problem

The current implementation, when you zoom in, the map also rotates slightly. This is very annoying and causes a lot of unnecessary lag.

If you compare it with other maps like Google maps or MapBox, they detect if you are trying to zoom or rotate, and only do one of them. This is way more confortable.

Introduced changes

I just made a quick fix for my app. It is still annoying because when the threshold is passed, it instantly rotates from the original position. But I still prefer this than always rotating.

Next steps

It introduces a new option minBearingThreshold of type number | undefined, the .d.ts files should be updated. I can do it after this is merged.

A new release should be published as well.

Discussion

If the feature is not good enough for your standards, feel free to tweak it, or indicate me what to change. For me, this is "good enough" for the amount of time I'm willing to spend (the UX of the leaflet will still be horrible anyways XD).

Raruto commented 8 months ago

@mauriciabad let me know about https://github.com/Raruto/leaflet-rotate/pull/48/commits/84af42b8ce6b63894b9e669599f7a74f0e482350

mauriciabad commented 8 months ago

Awesome! Thanks for taking the time to polish it ❤️.

mauriciabad commented 8 months ago

Okay, now it starts rotating at the bearing that the threshold has been passed. So it is way smoother.

Sometimes there can be a glitch where it rotates instantly, but it's very rare, and I can't figure out what gesture causes it.

Besides that, it works fine. I have no idea about how to write tests for that.

Raruto commented 8 months ago

Using this._startBearing is a good idea to reuse variables. But since the map has to remember if it has overcomed the inertia or not, it seems unavoidable to add another variable.

@mauriciabad Yep, mostly I was looking for a code structure with less chained/nested if conditions (math itself is already quite complex within this repo..)

Let me know: https://github.com/Raruto/leaflet-rotate/pull/48/commits/58d5b5201739d2fecdedb6b41a29adf6f45f5d87

mauriciabad commented 8 months ago

I can't code anymore because I can't run the code...

I'm doing npm run dev, going to http://127.0.0.1:8080/test/, and any change I do in JS files is not reflected, only the ones in the html. Not even with a clean clone of the original repo...

In the previous commits, I managed do run it but I forgot what I did...

Raruto commented 8 months ago

In the previous commits, I managed do run it but I forgot what I did...

@mauriciabad clone this repo under a local xampp/lamp server (ie. http://localhost:80/leaflet-elevation/test), and then run npm run build.

In the worst case scenario, make sure you have disabled browser cache..

👋 Raruto