Sortable: When options.axis is "x" or "y", allow drag to rearrange placeholder element in connected lists, even when outside of the container element. #650

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
@MoonScript
Contributor

MoonScript commented May 7, 2012

You can see this fix in-action here:
http://jsfiddle.net/MoonScript/eba5T/4/show/

Once you grab an item and start dragging up and down, move your mouse to the left as you drag, and try to move the item to the other list. You'll see that the yellow placeholder element will jump to the other (connected) list, and move up and down along with your mouse.

You can see how it currently works to compare:
http://jsfiddle.net/MoonScript/eba5T/show/

Sortable: update placeholder when axis is x or y for connected lists.…
… Fixed #8301 - Placeholder doesn't move when using connectWith option
@mikesherov

This comment has been minimized.

Show comment
Hide comment
@mikesherov

mikesherov Nov 5, 2012

Member

Hi @MoonScript, thanks again for contributing this patch. We recently re-enabled the test suite for sortable. In order for us to land this patch, we'd need a few tests added to the test suite proving this doesn't break existing functionality and also that it fixes the bug as described. Can you add some tests here please?

Member

mikesherov commented Nov 5, 2012

Hi @MoonScript, thanks again for contributing this patch. We recently re-enabled the test suite for sortable. In order for us to land this patch, we'd need a few tests added to the test suite proving this doesn't break existing functionality and also that it fixes the bug as described. Can you add some tests here please?

@mikesherov

This comment has been minimized.

Show comment
Hide comment
@mikesherov

mikesherov Nov 9, 2012

Member

@MoonScript, we're closing pulls that can't be merged as is… I like the pull, and appreciate the effort, but it needs tests. Feel free to open a new one with tests and we'll land it!

Member

mikesherov commented Nov 9, 2012

@MoonScript, we're closing pulls that can't be merged as is… I like the pull, and appreciate the effort, but it needs tests. Feel free to open a new one with tests and we'll land it!

@mikesherov mikesherov closed this Nov 9, 2012

@MoonScript

This comment has been minimized.

Show comment
Hide comment
@MoonScript

MoonScript Nov 9, 2012

Contributor

OK, thanks for the reminder.

-Jason Moon

On Nov 9, 2012, at 11:37 AM, Mike Sherov notifications@github.com wrote:

@MoonScript, we're closing pulls that can't be merged as is… I like the pull, and appreciate the effort, but it needs tests. Feel free to open a new one with tests and we'll land it!


Reply to this email directly or view it on GitHub.

Contributor

MoonScript commented Nov 9, 2012

OK, thanks for the reminder.

-Jason Moon

On Nov 9, 2012, at 11:37 AM, Mike Sherov notifications@github.com wrote:

@MoonScript, we're closing pulls that can't be merged as is… I like the pull, and appreciate the effort, but it needs tests. Feel free to open a new one with tests and we'll land it!


Reply to this email directly or view it on GitHub.

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