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

Make $#domManip patch jq2.0-compatible #526

Merged
merged 1 commit into from Nov 5, 2013

Conversation

Projects
None yet
5 participants
@zkat
Contributor

zkat commented Nov 2, 2013

This patch makes the inserted event compatible with both jquery 1.9.1 and >2.0.0.
The only relevant difference between the two versions is that >2.0.0 expects arguments[1] to be the callback, instead of arguments[2]. The other arguments can be passed through normally.

@whitecolor

This comment has been minimized.

Show comment
Hide comment
@whitecolor

whitecolor Nov 2, 2013

Contributor

+1

Contributor

whitecolor commented Nov 2, 2013

+1

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Nov 2, 2013

Contributor

We need to setup grunt to test different versions of jQuery.

You'll probably have to add other configurations to builder.json like "jquery-1.9" and "jquery-2.0" that load different things for "jquery/jquery.js".

And, the gruntfile needs to be setup to test the different pages. The qunit object should probably be built from builder.json.

Contributor

justinbmeyer commented Nov 2, 2013

We need to setup grunt to test different versions of jQuery.

You'll probably have to add other configurations to builder.json like "jquery-1.9" and "jquery-2.0" that load different things for "jquery/jquery.js".

And, the gruntfile needs to be setup to test the different pages. The qunit object should probably be built from builder.json.

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Nov 5, 2013

Contributor

I can work on that. We should just remove all those checked in libraries and use Bower for different dependencies. That will get our Codeclimate up quite a bit, too.

Contributor

daffl commented Nov 5, 2013

I can work on that. We should just remove all those checked in libraries and use Bower for different dependencies. That will get our Codeclimate up quite a bit, too.

daffl added a commit that referenced this pull request Nov 5, 2013

Merge pull request #526 from sykopomp/master
Make $#domManip patch jq2.0-compatible

@daffl daffl merged commit db8512d into canjs:master Nov 5, 2013

1 check passed

default The Travis CI build passed
Details
@imjoshdean

This comment has been minimized.

Show comment
Hide comment
@imjoshdean

imjoshdean Nov 8, 2013

Contributor

Node.DOCUMENT_FRAGMENT_NODE breaks in versions of IE. I'm going to be changing this to

var isFragment = elem.nodeType === 11, // Node.DOCUMENT_FRAGMENT_NODE
Contributor

imjoshdean commented on util/jquery/jquery.js in 60dbf49 Nov 8, 2013

Node.DOCUMENT_FRAGMENT_NODE breaks in versions of IE. I'm going to be changing this to

var isFragment = elem.nodeType === 11, // Node.DOCUMENT_FRAGMENT_NODE
@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Nov 26, 2013

Contributor

This should not have been added without tests. Also, no inline docs. Please make sure the code you submit to master has that quality level.

Contributor

justinbmeyer commented on 60dbf49 Nov 26, 2013

This should not have been added without tests. Also, no inline docs. Please make sure the code you submit to master has that quality level.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Nov 26, 2013

Contributor

This could likely have been pre-calculated instead of running each time.

This could likely have been pre-calculated instead of running each time.

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