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

Move directives to misc module (third and last pack) #3310

Merged
merged 7 commits into from
Dec 22, 2017

Conversation

ger-benjamin
Copy link
Member

@ger-benjamin ger-benjamin commented Dec 21, 2017

For and close GSGMF-214

@@ -14,8 +14,6 @@ goog.require('ol.interaction.Interaction');
*
* <input type="checkbox" ngModel="interaction.active" />
*
* See our live example: [../examples/interactiontoggle.html](../examples/interactiontoggle.html)
Copy link
Member Author

Choose a reason for hiding this comment

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

Address comment from PR: #3303 (comment)

@ger-benjamin ger-benjamin added this to the 2.3 milestone Dec 21, 2017
@ger-benjamin ger-benjamin self-assigned this Dec 21, 2017
@ger-benjamin ger-benjamin changed the title [WIP] Move directives to misc module (third and last pack) Move directives to misc module (third and last pack) Dec 21, 2017
// …
};
ngeo.module.directive('ngeoControl', ngeo.controlDirective);
ngeo.misc.controlComponent.directive('ngeoMiscControl', ngeo.misc.controlComponent.component_);
Copy link
Member

Choose a reason for hiding this comment

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

I think you should keep 'ngeoControl'.

Copy link
Member Author

@ger-benjamin ger-benjamin Dec 21, 2017

Choose a reason for hiding this comment

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

Yes, thanks, I don't know why I've done that

Copy link
Member

Choose a reason for hiding this comment

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

Otherwise LGTM.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahhh... but it's the guidlines. Does it matter ?
Perhaps it's even better to remove them...

Copy link
Member

Choose a reason for hiding this comment

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

The guideline describes how to deal with the Closure compiler symbols but says nothing about renaming the names used in Angular.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I understand what you mean now! ^^
OK, we can merge as it is.

@gberaudo
Copy link
Member

@pfirpfel, if you have some time can you merge this PR tomorrow? Thanks.

@fredj fredj merged commit da09860 into master Dec 22, 2017
@fredj fredj deleted the move_directives_to_misc_module branch December 22, 2017 07:52
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