Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

mauriciabad
Copy link

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
Copy link
Owner

Raruto commented Feb 21, 2024

@mauriciabad let me know about 84af42b

@mauriciabad
Copy link
Author

mauriciabad commented Feb 21, 2024

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

@mauriciabad
Copy link
Author

mauriciabad commented Feb 27, 2024

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.

@mauriciabad mauriciabad requested a review from Raruto February 27, 2024 13:42
@Raruto
Copy link
Owner

Raruto commented Mar 1, 2024

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: 58d5b52

@mauriciabad
Copy link
Author

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
Copy link
Owner

Raruto commented Mar 12, 2024

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

@Raruto
Copy link
Owner

Raruto commented Nov 26, 2024

@mauriciabad is this ok? (I'm not going to test it for you...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants