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

Remove jquery dependency #3

Closed
wants to merge 1 commit into from
Closed

Remove jquery dependency #3

wants to merge 1 commit into from

Conversation

jwangnz
Copy link

@jwangnz jwangnz commented May 14, 2014

It seems that the dependency of jQuery is not necessary.

@davidlgj
Copy link
Contributor

hmmm... not exactly. We do depend on jQuery by using find([ng-model]), angular doesn't have any selector support. But I think that might be the only place.

And that code is only to support having "manual" fields in the form at the top, a feature that sounded good in my head but I don't think it's used. A proper solution would be to use find if jQuery is defined otherwise try document.querySelectorAll, or just bail out if none of them are defined, or a smaller solution would be to only support having predefined fields if jQuery is defined.

@davidlgj
Copy link
Contributor

Actually it looks @zackbloom already solved it with querySelector in his branch.

cloudflare@70e4280

@jwangnz
Copy link
Author

jwangnz commented May 14, 2014

@davidlgj OK, that is a better solution. Will you merge that commit from EagerIO?

@davidlgj
Copy link
Contributor

Yep, I can do that. Closing this.

@davidlgj davidlgj closed this May 14, 2014
@davidlgj
Copy link
Contributor

@Tsing it's landed in the development branch

@jwangnz
Copy link
Author

jwangnz commented May 14, 2014

@davidlgj Thank you. 👍

@zackbloom
Copy link
Contributor

Wow, first time I've had a PR merged without actually making one!

There's one comment on that commit which should be fixed, if we want the ng-model feature to keep working.

@jwangnz jwangnz deleted the remove-jquery-dependency branch May 15, 2014 07:52
@davidlgj davidlgj mentioned this pull request Sep 21, 2015
6 tasks
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