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

Use new mapboxgl Control system #94

Closed
tmcw opened this issue Nov 10, 2016 · 11 comments
Closed

Use new mapboxgl Control system #94

tmcw opened this issue Nov 10, 2016 · 11 comments
Assignees

Comments

@tmcw
Copy link
Contributor

tmcw commented Nov 10, 2016

mapbox/mapbox-gl-js#3497

The new IControl interface will allow mapbox-gl-directions to remove any dependency on mapboxgl, eliminating a popular class of installation bugs. Once mapboxgl is updated, or before if you're feeling saucy, update to the new interface.

@tmcw
Copy link
Contributor Author

tmcw commented Nov 15, 2016

cc @mollymerp @mapsam @mapbox/appbox does anyone want to potentially hop on this one? And does anyone want to join the general effort of keeping our constellation of mapbox-gl plugins up to date and stable?

@davidtheclark
Copy link

does anyone want to join the general effort of keeping our constellation of mapbox-gl plugins up to date and stable

@tmcw Do you think it makes sense to delegate maintainers for particular plugins? I can take one.

@tmcw
Copy link
Contributor Author

tmcw commented Nov 15, 2016

I think that's a capital idea.

I think GitHub has some rumored system where each project has the 'responsible party' or something. Right now we have three plugins:

  • mapbox-gl-geocoder (I just refactored this one)
  • mapbox-gl-directions (this one)
  • mapbox-gl-draw

@mapsam
Copy link
Contributor

mapsam commented Nov 15, 2016

capital idea

Indeed. Happy to jump on one of these @tmcw. gl-draw is interesting to me, though I know nothing about it.

@davidtheclark
Copy link

@tmcw I'm happy to take on mapbox-gl-directions, but might not be able to do so right away. How urgent should I consider this?

@tmcw
Copy link
Contributor Author

tmcw commented Nov 15, 2016

It's unfortunately rather urgent. If you don't have bandwidth for this immediately, I can try to do a super minimal refactor that just gets it working again and leave the more interesting bits as for the future.

@davidtheclark
Copy link

@tmcw My top priority right now is finishing up the style form refactor; so, yeah, if you'd like to jump in sooner, that would great. I'll start familiarizing myself with this plugin after that.

@tmcw
Copy link
Contributor Author

tmcw commented Nov 15, 2016

@mapsam want to grab this?

So the gist is:

  • Controls should no longer rely on mapboxgl as a global or require. Luckily this module only does in two places afaik - grabbing an access token and wrapping coordinates.
  • Instead of addTo and remove, which basically treat the control as the one in charge, GL JS is in charge with the .addControl and .removeControl methods and the control needs to implement onAdd and onRemove in order to hook into these
  • Instead of the control adding itself to the DOM, it returns an element and GL JS does that in addControl. This also means they no longer use a position property as input: that's handled in addControl.

@mapsam mapsam self-assigned this Nov 15, 2016
@mapsam
Copy link
Contributor

mapsam commented Nov 17, 2016

@tmcw making progress here, but stuck on one thing. The access token makes its way into the directions api calls, but I'm not sure how to give the access token to the geocoder. Any clues?

@mapsam
Copy link
Contributor

mapsam commented Nov 17, 2016

I'm also unable to get this working in master - is there an old screenshot of what the controls should look like? Everything is a bit wonky right now and I'm not 💯 sure where everything is expected to be.

Nvm I wasn't navigating into the /example folder 😁

@tmcw
Copy link
Contributor Author

tmcw commented Nov 17, 2016

In the new Geocoder control, you provide an accesstoken to the control. Slightly less convenient but more direct than grabbing from mapboxgl.accessToken as a global.

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

No branches or pull requests

3 participants