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 namespace prefix to avoid collision #37

Open
gastonelhordoy opened this issue Mar 23, 2016 · 8 comments
Open

Use namespace prefix to avoid collision #37

gastonelhordoy opened this issue Mar 23, 2016 · 8 comments

Comments

@gastonelhordoy
Copy link

The typeahead directive from angular-bootstrap uses the match attribute which causes this directive to be evaluated resulting in the following warning in console: Match validation requires ngModel to be on the element.

It's not a big deal, but I think it's better to avoid potential problems in the future since angular-bootstrap is widely used.

@TheSharpieOne
Copy link
Owner

duplicate of #34 fixed in 19923ab please use the latest version.

@gastonelhordoy
Copy link
Author

Hey @TheSharpieOne thanks for the quick reply! I didn't notice the latest version is not displaying the warning message anymore. However, since the directive name is still the same one, it's being evaluated by angular, isn't it?

I guess it would be better if it doesn't even execute the link function. I'm sure that time consumed in evaluating and instantiating is not that critical, but it's something to consider anyways.

@TheSharpieOne
Copy link
Owner

It doesn't really do anything if there is no ngModel attach to the element (the uib match shouldn't have an ngModel).
see:

if(!ctrl) {
return;
}

Also, what makes more sense, EVERYONE except angular-bootstrap namespace there modules, or just angular-bootstrap use a namespace? If you library is going to have a large footprint that can overlap many others, it should deal with that and not depend on everyone else to adjust for it.
Really, this is an angular-bootstrap defect since a little while ago (v0.14.0) they decided to do just that; namespace their modules (uib). They missed a few in this effort.

@dopry
Copy link

dopry commented Jun 2, 2016

@TheSharpieOne, by that same logic shouldn't you also prefix your directives as well? av-match or validation-match. Happy to submit a PR if you let me know what you'd prefer.

@TheSharpieOne
Copy link
Owner

This isn't a large project which contains many directives, but sure, go for it. I'll accept the PR and publish a new major version.
I've moved away from angular myself, which is whyI am not showing much love to this project.

@dopry
Copy link

dopry commented Jun 2, 2016

Cool... I'll see what I can do next week. We have a project which we did
experience conflict issues with a while back. First I'll check if the new
version resolves the uib typeahead conflict, if it doesn't I'll write a
namespace patch. Do you have an preferences for a namespace?

On Thu, Jun 2, 2016 at 12:35 PM, Evan Sharp notifications@github.com
wrote:

This isn't a large project which contains many directives, but sure, go
for it. I'll accept the PR and publish a new major version.
I've moved away from angular myself, which is whyI am not showing much
love to this project.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#37 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAXqOOH0ElmWcJ4Jq93A8TkFlia3DIaYks5qHwY-gaJpZM4H3IiE
.

Darrel O'Pry
The Spry Group, LLC.
http://www.spry-group.com
718-355-9767 x101

@TheSharpieOne
Copy link
Owner

No preference really, just something short

@dopry
Copy link

dopry commented Jun 2, 2016

cool, then I'll go for something like vm- I'm sure it'll conflict somewhere
else, but meh, it's never perfect.. :)

On Thu, Jun 2, 2016 at 7:27 PM, Evan Sharp notifications@github.com wrote:

No preference really, just something short


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#37 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAXqOLn-s-u9REbGqBm0OwfQtoLTqCBAks5qH2bFgaJpZM4H3IiE
.

Darrel O'Pry
The Spry Group, LLC.
http://www.spry-group.com
718-355-9767 x101

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants