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

Flickering geolocate circle fix #10334

Merged
merged 2 commits into from
Feb 4, 2021
Merged

Conversation

anderswi
Copy link
Contributor

@anderswi anderswi commented Jan 27, 2021

fixes #10276

The metersPerPixel calculation introduced rounding errors. By measuring over 100 pixels instead of 1 pixel, the issue goes away.

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
  • add an entry inside this element for inclusion in the mapbox-gl-js changelog: <changelog>Fix for flickering accuracy circle in GeolocateControl.</changelog>

@CLAassistant
Copy link

CLAassistant commented Jan 27, 2021

CLA assistant check
All committers have signed the CLA.

@mourner
Copy link
Member

mourner commented Feb 1, 2021

Brilliant fix, thanks for taking on this! Just in case, could you also add a test in geolocate.test.js asserting the width of the circle on higher zooms where the issue is apparent, making sure it fails without the fix and passes with it?

@anderswi
Copy link
Contributor Author

anderswi commented Feb 4, 2021

@mourner I've added tests now. I never got the testing to work properly locally (on windows), but the ci tests are passing now and should™ fail without my fix.

Copy link
Member

@mourner mourner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brilliant, thank you!

@mourner mourner merged commit b2b3868 into mapbox:main Feb 4, 2021
@karimnaaji karimnaaji added this to the v2.2 milestone Mar 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GeolocateControl: Accuracy circle size is inconsistent
4 participants