Skip to content

Conversation

matt-hoskins
Copy link

...d. Fixes #9335 - sortover event does not consistently fire.

…nged. Fixes #9335 - sortover event does not consistently fire.
@tjvantoll
Copy link
Member

Thanks @matt-hoskins. Could you write some tests to support this change to prevent against regressions? The tests from the fix for #9041 might be a good reference: 8b1b34c.

@matt-hoskins
Copy link
Author

Hi @tjvantoll - I'm not at all familiar with the jQuery UI test system/environment so it would take me a while to get up to speed with it before trying to figure out how to write a test and I'm afraid that would take more time than I can spare at the moment :(.

@matt-hoskins
Copy link
Author

Hi @tjvantoll - I've had a little bit of spare time so I've started looking at testing - I notice there's no test for the "out" event in the sortable so I'm thinking of writing a test for the "out" that also tests the "over" event following an "out". To properly test the scenario with connected sortables for FS#9335 a continuous drag out and back over is needed and inspecting the state before the mouse up (i.e. mousedown, mousemove, mousemove). The jquery simulate plugin included with the tests has a drag which always does a mouseup at the event. The choice seems to be to either clone the functionality of the simulate drag into my own function in the test to be able to simulate a drag without a mouse up, or else extend the jquery simulate plugin to have a couple of extra options to simulateDrag of maybe nomouseup and nomousedown which can be used to request selectively the mouseup and/or mousedown events aren't triggered (thus a drag initiation could be simulated and then a sequence of mouse moves still dragging followed by the mouse up). Maybe this is something I should raise on a mailing list or forum - but I was wondering if patching the included jquery simulate plugin in this way would be ok :)

@scottgonzalez
Copy link
Member

Modifying the simulate plugin directly in jQuery UI is not acceptable. You should just add a helper method (see the helper files).

@matt-hoskins
Copy link
Author

Ok - I thought it might become generally useful to test a drag-in-progress and it would only require a small addition, however I'll clone the code across into a helper and adjust it there.

…ected lists and draggable in tests (covering issue raised in #9335).
@matt-hoskins
Copy link
Author

Some notes on my latest commit to add tests:
I had to add draggable to the includes for the sortable tests as one of the interactions reported on #9335 was an interaction between draggable and sortable. I've tested that the new test fails against 1.10.3 in the expected ways. I've tested that the new tests pass against this branch on the versions of firefox, chrome and IE I have to hand. In the case of IE - I have IE10 (under Windows 7) and IE7 (under XP) and the test passed under both.

@stephankaag
Copy link

What's the status?

@mikesherov
Copy link
Member

This can not be merged as it's very old at this point, and the tests do way too much here. I do believe that another PR fixes the same issue: #1170 Closing this for now until we can find a suitable test case instead.

@mikesherov mikesherov closed this Aug 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants