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

feat: send geolocation as adapter #10400

Closed

Conversation

behnammodi
Copy link
Contributor

@behnammodi behnammodi commented Feb 19, 2021

  • This PR can help developers to customize geolocation behavior.
  • We don't have any visual changes.
  • All test cases compatible with these changes.
  • I need your opinion and then add a document.

So now we have:

const myGeolocation: Geolocation = {
   clearWatch: ...,
   getCurrentPosition: ...,
   watchPosition: ...,
};

const geolocateControl = new GeolocateControl({
   geolocation: myGeolocation,
});

map.addControl(geolocateControl);

I'll say welcome to your awesome feedback :)

Continue this PR #10232

@karimnaaji
Copy link
Contributor

As mentioned by @mourner in the other PR, CI will need to be addressed prior to review. The failed tests are build and unit-test:

https://app.circleci.com/pipelines/github/mapbox/mapbox-gl-js/5924/workflows/e1c44d57-030a-42ea-ad10-1d430ff22043/jobs/80315

https://app.circleci.com/pipelines/github/mapbox/mapbox-gl-js/5924/workflows/e1c44d57-030a-42ea-ad10-1d430ff22043/jobs/80309

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.

This is still waiting for green CI builds (currently some tests are failing).

@behnammodi
Copy link
Contributor Author

Do we have any updates about this PR?

@CLAassistant
Copy link

CLAassistant commented Oct 19, 2021

CLA assistant check
All committers have signed the CLA.

@mourner
Copy link
Member

mourner commented Oct 26, 2021

Sorry for the delay @behnammodi — we're investigating why the CI doesn't run all the jobs properly, even though you have rebased everything on main. Otherwise this looks good.

@behnammodi
Copy link
Contributor Author

@mourner No problem, thanks a lot

@mourner
Copy link
Member

mourner commented Mar 17, 2022

I didn't know how to resolve the CI being stuck, so ended up rebasing this and making a new PR with your changes here: #11614

@mourner mourner closed this Mar 17, 2022
@behnammodi
Copy link
Contributor Author

I didn't know how to resolve the CI being stuck, so ended up rebasing this and making a new PR with your changes here: #11614

Go ahead

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.

4 participants