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

GeoLocation accuracy-bubble going places (randomly chaning size) #2449

Closed
Philip2809 opened this issue Apr 26, 2023 · 6 comments · Fixed by #2450
Closed

GeoLocation accuracy-bubble going places (randomly chaning size) #2449

Philip2809 opened this issue Apr 26, 2023 · 6 comments · Fixed by #2450
Labels
bug Something isn't working PR is more than welcomed Extra attention is needed

Comments

@Philip2809
Copy link
Contributor

maplibre-gl-js version: 2.4.0

browser: Firefox

Steps to Trigger Behavior

  1. Go to: https://maplibre.org/maplibre-gl-js-docs/example/locate-user/
  2. Allow location access
  3. Zoom in on the accuracy bubble and when you have a low zoom in looks really bad

Link to Demonstration

(you get a gif instead, but you can try it out for yourself at https://maplibre.org/maplibre-gl-js-docs/example/locate-user/)
firefox_1SSyJ6W6sa

Expected Behavior

To not do this, like mapbox does:
https://docs.mapbox.com/mapbox-gl-js/example/locate-user/

I know this is a fork of mapbox when it was still on an open license and they have since either fix this bug or something else brought it up, however I do this it should be allowed to fix such a bug that is breaking both the visuals and the function.

@Philip2809 Philip2809 changed the title GeoLocation Acuraccy bubble going places (randomly chaning size) GeoLocation accuracy-bubble going places (randomly chaning size) Apr 26, 2023
@HarelM HarelM added bug Something isn't working PR is more than welcomed Extra attention is needed labels Apr 26, 2023
@HarelM
Copy link
Collaborator

HarelM commented Apr 26, 2023

Thanks for taking the time to open this issue.
The main issue here is the flickering of the accuracy circle which needs to be fixed.
Feel free to submit a PR.

@Philip2809
Copy link
Contributor Author

I can fix a PR for this but I have some question regarding "You are not allowed to backport code from Mapbox projects which has been contributed under this new license.", especially since mapbox has fixed this bug. What exactly does this mean and what type of "fixes" would be a no go in this case? I mean since this was probobly a bug in mapbox as well but the have fixed it, will fixing it violate the copyright, or how does this work?

@HarelM
Copy link
Collaborator

HarelM commented Apr 26, 2023

It means you can't look at their code in order to fix this bug. If you haven't looked at the mapbox code related to this fix everything you do is legit.

@Philip2809
Copy link
Contributor Author

Alright then good, I used some code from stackoverflow: https://stackoverflow.com/a/54928479
I updated the "_updateCircleRadius" in "geolocate_control.ts" to:

    _updateCircleRadius() {
        const bounds = this._map.getBounds();
        const southEastPoint = bounds.getSouthEast();
        const northEastPoint = bounds.getNorthEast();
        const mapHeightInMeters = southEastPoint.distanceTo(northEastPoint);
        const mapHeightInPixels = this._map._container.clientHeight;
        const circleDiameter = 2 * (this._accuracy / (mapHeightInMeters / mapHeightInPixels));
        this._circleElement.style.width = `${circleDiameter}px`;
        this._circleElement.style.height = `${circleDiameter}px`;
    }

After this change it seams to work, I will open a PR/branch. How long do you think until it is accepted and out on npm?

@HarelM
Copy link
Collaborator

HarelM commented Apr 26, 2023

A few days if you are ok with a pre-release, a bit more if not.

@Philip2809
Copy link
Contributor Author

I have opened a PR (#2450), doing the checklist now, it says I should confirm there is no backport, should I now look at their code?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working PR is more than welcomed Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants