Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Adding ng-jq functionality, the ability to choose your angular.element l... #10750

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,36 @@ function equals(o1, o2) {
return false;
}

/**
* @ngdoc directive
* @name ng.directive:ngJq
*
* @element html
* @param {string} (optional) the name of the library available under `window` to be used for angular.element
*
* @description
*
* Use this directive to force the angular.element library. This should be used to force
* either jqLite by leaving ng-jq blank or setting the name of the jquery variable under window
* (eg. jQuery).
*
* This directive is global for the whole of the angular library, hence the directive can only
* be added to the top-most HTML element.
*
*/
var jq = function () {
if (isDefined(jq.name_)) return jq.name_;

var el = document.querySelector('[ng-jq]') || document.querySelector('[data-ng-jq]');
Copy link
Contributor

Choose a reason for hiding this comment

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

we should follow the same mechanism that we have for ng-app

Copy link
Contributor

Choose a reason for hiding this comment

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

@lgalfaso this branch looks like it's very old, a lot of things are missing. In current branch, it's possible to use getNgAttribute(rootElement, 'jq') to check for these bootstrap parameters

Copy link
Contributor

Choose a reason for hiding this comment

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

you are right, the branch is really old

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lgalfaso I'm going on @mzgol suggestions on how to implement this, which was based on ng-csp since angular.element is a global variable, unlike ng-app which you can have multiple in a page.

As for the branch itself, this was the latest that I could get from github, fetching the latest from upstream and making sure that my master has the same last commit than the angular one. If there's any branch 'weirdness', I can't comment why that would be.

Copy link
Contributor

Choose a reason for hiding this comment

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

The parent of 9b7b0fc is 28fe446

28fe446 was committed on Oct 6, 2013

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Crap. That's weird. Something must of messed up somewhere.

Let me add some of your suggestions and I'll create a new pull request based on the right parent.

Cheers.

var name = undefined;

if (el) {
name = el.getAttribute('ng-jq') || el.getAttribute('data-ng-jq') || '';
}

return (jq.name_ = name);
};


function concat(array1, array2, index) {
return array1.concat(slice.call(array2, index));
Expand Down Expand Up @@ -1147,7 +1177,7 @@ function snake_case(name, separator){

function bindJQuery() {
// bind to jQuery if present;
jQuery = window.jQuery;
jQuery = jq() ? window[jq()] : window.jQuery;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will not do what you are expecting as '' is falsy

Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, please do not call jq() twice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lgalfaso hm, guess I need to change my unit tests then. Thanks for the heads up. Will change right now.

// reset to jQuery or default to us.
if (jQuery) {
jqLite = jQuery;
Expand Down
51 changes: 51 additions & 0 deletions test/AngularSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,57 @@ describe('angular', function() {
});
});

describe('jq', function () {

var element;

beforeEach(function () {
element = document.createElement('html');
});

afterEach(function () {
jq.name_ = undefined;
delete jq.name_;
});

it('should return undefined when jq is not set (the default)', function () {
expect(jq()).toBe(undefined);
});

it('should return empty string when jq is enabled manually via [ng-jq] with empty string', function () {
element.setAttribute('ng-jq', '');
spyOn(document, 'querySelector').andCallFake(function (selector) {
if (selector == '[ng-jq]') return element;
});
expect(jq()).toBe('');
});

it('should return empty string when jq is enabled manually via [data-ng-jq] with empty string', function () {
element.setAttribute('data-ng-jq', '');
spyOn(document, 'querySelector').andCallFake(function (selector) {
if (selector == '[data-ng-jq]') return element;
});
expect(jq()).toBe('');
expect(document.querySelector).toHaveBeenCalledWith('[data-ng-jq]');
});

it('should return "jquery" when jq is enabled manually via [ng-jq] with value "jquery"', function () {
element.setAttribute('ng-jq', 'jquery');
spyOn(document, 'querySelector').andCallFake(function (selector) {
if (selector == '[ng-jq]') return element;
});
expect(jq()).toBe('jquery');
});

it('should return "jquery" when jq is enabled manually via [data-ng-jq] with value "jquery"', function () {
element.setAttribute('data-ng-jq', 'jquery');
spyOn(document, 'querySelector').andCallFake(function (selector) {
if (selector == '[data-ng-jq]') return element;
});
expect(jq()).toBe('jquery');
expect(document.querySelector).toHaveBeenCalledWith('[data-ng-jq]');
});
});

describe('parseKeyValue', function() {
it('should parse a string into key-value pairs', function() {
Expand Down