search

jquery / jquery-simulate

jQuery Event Unit Testing Helpers
Other
173 stars 114 forks source link

Dragging object #12

Closed robkinyon closed 11 years ago

robkinyon commented 11 years ago

I needed a way of doing part of a dragging simulation in order to test the in-drag activities of a proprietary jQuery plugin. So, simulateDrag doesn't change its behavior, but its behavior is now addressable in parts.

mikesherov commented 11 years ago

Thanks for contributing! In order for me to land this, you'll need to run grunt to make sure everything passes, and add some test cases to cover the new code that you wrote. Can you kindly do that? Once you do, I can give you feedback on the rest of the PR. Thanks again!

robkinyon commented 11 years ago

Mike -

I am unable to run grunt because the grunt dependencies require grunt

0.4, but the Gruntfile forces the installation of 0.3.1. This is a new installation running on Ubuntu 12.04. I don't know enough about Grunt to make it work. I'm really sorry about that, but I'm not sure what the next step would be.

Rob

On Mon, Mar 11, 2013 at 8:47 PM, Mike Sherov notifications@github.comwrote:

Thanks for contributing! In order for me to land this, you'll need to run grunt to make sure everything passes, and add some test cases to cover the new code that you wrote. Can you kindly do that? Once you do, I can give you feedback on the rest of the PR. Thanks again!

— Reply to this email directly or view it on GitHubhttps://github.com/jquery/jquery-simulate/pull/12#issuecomment-14752479 .

Thanks, Rob Kinyon

mikesherov commented 11 years ago

Thanks for the fast response, @robkinyon. Can you run the test suite locally? All you have to do is open the index file in the test directory to run the QUnit tests. In the meantime, I'll figure out the grunt issues... thanks for reporting.

robkinyon commented 11 years ago

I opened test/index.html from my checkout in Chrome, and 23 of 23 tests passed.

Note - test/qunit is empty. I had to copy qunit.js and qunit.css into test/qunit/qunit. I suspect grunt would have populated those files.

On Mon, Mar 11, 2013 at 9:11 PM, Mike Sherov notifications@github.comwrote:

Thanks for the fast response, @robkinyon https://github.com/robkinyon. Can you run the test suite locally? All you have to do is open the index file in the test directory to run the QUnit tests. In the meantime, I'll figure out the grunt issues... thanks for reporting.

— Reply to this email directly or view it on GitHubhttps://github.com/jquery/jquery-simulate/pull/12#issuecomment-14753195 .

Thanks, Rob Kinyon

scottgonzalez commented 11 years ago

@robkinyon Grunt should work now, you'll need to pull master and run npm install again.

robkinyon commented 11 years ago

I've updated the pull request per grunt running jslint and finding some extra commas and the like. Tests ran cleanly.

mikesherov commented 11 years ago

Hi @robkinyon, in order for me to land this request, I'll also need some new tests showing that the functionality you added works properly. Do you think you can do that? Please let me know when you get a chance.

robkinyon commented 11 years ago

Sorry - I just switched jobs, so I haven't had time to even think about this. I apologize for the delay. I'll try and get to it soon.

On Tue, Apr 2, 2013 at 12:15 PM, Mike Sherov notifications@github.comwrote:

Hi @robkinyon https://github.com/robkinyon, in order for me to land this request, I'll also need some new tests showing that the functionality you added works properly. Do you think you can do that? Please let me know when you get a chance.

— Reply to this email directly or view it on GitHubhttps://github.com/jquery/jquery-simulate/pull/12#issuecomment-15796288 .

Thanks, Rob Kinyon

mikesherov commented 11 years ago

No apologies needed. Congrats on the new job! I'll just leave the PR open until you get around to it.

mikesherov commented 11 years ago

Actually, now that I think about it. I'm probably not going to land this unless there's a strong need. I would love this help on some of the open issues if you'd like to tackle those, but I'm going to close this.

robkinyon commented 11 years ago

I do have need for this on a project I'm working on, which is why I provided the pull request. The ability to test in-drag changes is important for my deliverables.

On Tue, Apr 2, 2013 at 5:31 PM, Mike Sherov notifications@github.comwrote:

Actually, now that I think about it. I'm probably not going to land this unless there's a strong need. I would love this help on some of the open issues if you'd like to tackle those, but I'm going to close this.

— Reply to this email directly or view it on GitHubhttps://github.com/jquery/jquery-simulate/pull/12#issuecomment-15811280 .

Thanks, Rob Kinyon

mikesherov commented 11 years ago

@robkinyon, what I meant was that there doesn't seem to be a strong community need for this, and this add a lot of complexity and indirection to the code base.