Skip to content

Make control directive rely on control.setTarget #143

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

Merged
merged 1 commit into from
Jan 16, 2015

Conversation

elemoine
Copy link
Contributor

This is WIP depending on openlayers/openlayers#3139.

@elemoine elemoine changed the title Make control directive rely on control.setTarget (WIP) Make control directive rely on control.setTarget Jan 16, 2015
@elemoine
Copy link
Contributor Author

This is now ready for review. With this the control directive relies on a control being made available in the parent scope, as opposed to a function that returned a control.

Please review.

@pgiraud
Copy link
Contributor

pgiraud commented Jan 16, 2015

It makes things simpler. I like it.
This looks good. Please merge.

@elemoine
Copy link
Contributor Author

And the problem was that the function was executed with window as its scope, instead of the controller as one would have expected.

elemoine pushed a commit that referenced this pull request Jan 16, 2015
Make control directive rely on control.setTarget
@elemoine elemoine merged commit 805b341 into camptocamp:master Jan 16, 2015
@elemoine
Copy link
Contributor Author

I guess this implies changes in projects that use the ngeo-control directive?

Yes, the user just needs to provide an expression that evaluates to a control (with no target and not already added to the map).

@elemoine elemoine deleted the control-expression branch January 16, 2015 14:38
elemoine added a commit to Geoportail-Luxembourg/geoportailv3 that referenced this pull request Jan 16, 2015
@sbrunner sbrunner added this to the Older milestone Aug 23, 2019
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.

3 participants