Ticket #6702 (closed feature: fixed)

Opened 4 years ago

Last modified 18 months ago

horizontal sortable not working (and solution)

Reported by: waschmittel Owned by:
Priority: minor Milestone: 1.8.11
Component: ui.sortable Version: 1.8.6
Keywords: Cc:
Blocking: Blocked by:

Description

I tried to make some horizontal divs (float: left;) sortable. They only updated when I dragged the item up or down additionaly to left or rigt. I solved the problem for me by changing jquery.ui.sortable.js line 53 this.floating = this.items.length ? (/left|right/).test(this.items[0].item.css('float')) : false; to this.floating = this.items.length ? (/left|right/).test(this.items[0].item.css('float')) : false; So while I don't know why exactly this is tested (performance?), it would be very practical if this could at least be set as an option so that horizontal sorting is usable.

Change History

comment:1 Changed 4 years ago by waschmittel

I tried to make some horizontal divs (float: left;) sortable. They only updated when I dragged the item up or down additionaly to left or rigt. I solved the problem for me by changing jquery.ui.sortable.js line 53

this.floating = this.items.length ? (/left|right/).test(this.items[0].item.css('float')) : false;

to

this.floating = this.items.length ? (/left|right/).test(this.items[0].item.css('float')) : false;

So while I don't know why exactly this is tested (performance?), it would be very practical if this could at least be set as an option so that horizontal sorting is usable.

comment:2 Changed 4 years ago by scott.gonzalez

There's no difference between what you said you changed the code from and to.

comment:3 Changed 4 years ago by waschmittel

Sorry. I meant I changed

this.floating = this.items.length ? (/left|right/).test(this.items[0].item.css('float')) : false;

to

this.floating = this.items.length ? (/left|right/).test(this.items[0].item.css('float')) : true;

comment:4 Changed 4 years ago by mosevo

UPDATED

I am having a similar problem. This change did not work for me, although I believe my problem is different after doing more testing.

I have several "master list" ULs containing list item "options". These options can be dragged to other "receiving" ULs. If I accidentally drag and hover over one of the other "master list" ULs before hovering over the "receiving" ULs, I cannot drop the ui.item.

If I carefully avoid the other "master list" ULs and drag straight to the "receiving" ULs, I have no problem. At first I thought this was related, now I don't think so.

Last edited 4 years ago by mosevo (previous) (diff)

comment:5 Changed 4 years ago by scott.gonzalez

The fix for this needs to be more involved. We can default empty lists to be marked as floating, but we'll need to update that as soon as the list receives an item. We'd also need to update if the list becomes empty again.

comment:6 Changed 3 years ago by michaelmwu

There do seem to be some peculiarities regarding horizontal sorting, especially when the items are display: inline or inline-block. I am using inline-block to create a horizontal list that does not wrap, as floats would.

I think I fixed some of them by changing the calculation to determine whether the items are floating. It might be more accurate to describe it as a calculation to see if the elements are horizontal or not, by testing if the elements are floating, or are being displayed horizontally, by checking the display property for all display properties that would make elements display horizontally.

Original:

this.floating = this.items.length ? (/left|right/).test(this.items[0].item.css('float')) : false;

New:

this.floating = this.items.length ? (/left|right/).test(this.items[0].item.css('float')) || (/inline|table-cell/).test(this.items[0].item.css('display')) : false;

See demo:  http://www.stanford.edu/~mikemwu/sortable/

Note that the original sortable does not always immediately update the order, while the changes to floating help the order update more naturally.

If this is acceptable, I can submit the change.

comment:7 Changed 3 years ago by michaelmwu

 https://github.com/michaelmwu/jquery-ui/commit/f1d939bc589fd8d211e0dc540f7d92cfae94a411

The change seems to give the horizontal sort better behavior.

comment:8 Changed 3 years ago by michaelmwu

Perhaps floating should be recalculated inside refresh and possibly on receive?

comment:9 Changed 3 years ago by michaelmwu

  • Status changed from new to closed
  • Resolution set to fixed

Sortable: Changed floating calculation to determine also whether items are being displayed horizontally. Helps fix odd sorting behavior for horizontal lists. Fixed #6702 - horizontal sortable not working (and solution)

Changeset: f1d939bc589fd8d211e0dc540f7d92cfae94a411

comment:10 Changed 3 years ago by michaelmwu

Sortable: Changed floating calculation to determine also whether items are being displayed horizontally. Helps fix odd sorting behavior for horizontal lists. Fixed #6702 - horizontal sortable not working (and solution)(cherry picked from commit f1d939bc589fd8d211e0dc540f7d92cfae94a411)

Changeset: 20b010640e837cad1ad7203e02b28b399e328725

comment:11 Changed 3 years ago by scott.gonzalez

  • Milestone changed from 1.9 to 1.8.11

comment:12 Changed 3 years ago by whittet@…

Patch  https://github.com/whittet/jquery-ui/commit/2bf061ea6fd3bad7d6f5a77eebb22b7cc07140e4

1 line code fix submitted via Pull Request to resolve following issues:

Demo problem:  http://jsfiddle.net/whittet/P2yuG/ Demo solution:  http://jsfiddle.net/whittet/P2yuG/12/

Please reopen to fix following issues, which appear in version 1.8.12.

Use case #1: A) click and drag the "2" 10px to the left, B) then move mouse UP 2px, this causes the placeholder to move where it needs to. BUG: Step B should not be required.

Use case #2: A) click and drag the "1" 10px to the right until it is over the next column, B) then move mouse DOWN 2px, BUG: Step B should not be required.

Other potentially related complaints:  http://stackoverflow.com/questions/3627130/how-to-sort-th-in-a-thead-row-using-jquery-ui-sortable http://bugs.jqueryui.com/ticket/4765

Last edited 3 years ago by whittet@… (previous) (diff)

comment:13 Changed 3 years ago by judeosborn

I want to confirm that this is indeed still a problem as of 1.8.13, and a serious one, at that. A sortable list that has been confined to the X axis is essentially unusable. Please re-open.

Last edited 3 years ago by judeosborn (previous) (diff)

comment:14 follow-up: ↓ 15 Changed 3 years ago by scott.gonzalez

  • Status changed from closed to reopened
  • Resolution fixed deleted
  • Milestone 1.8.11 deleted

comment:15 in reply to: ↑ 14 Changed 3 years ago by judeosborn

Okay, I figured out the source of this problem, at least in my case. I'm just not sure yet if it warrants a patch. The problem is that the sortable item did not have any items when it was initiated, and thus no length, which ultimately means it is not considered a horizontal element. I was using drag and drop with this sortable element, so it was empty until the user dragged an item in to it. However, by that time it was too late to consider the element horizontal.

To be more specific, according to the following line of code, no length means the sortable item's "floating" property is never set to true, meaning the item is not horizontal:

this.floating = this.items.length ? o.axis === 'x' || (/left|right/).test(this.items[0].item.css('float')) || (/inline|table-cell/).test(this.items[0].item.css('display')) : false;

The solution in my case was to destroy and re-create the sortable every time an item is dropped in to it. This works, but it's not particularly elegant, and the whole issue can be a source of confusion for developers. My suggestion is to move the above line of code from the "_create" method to the "refresh" method, and perhaps clarify in the documentation that the sortable may need to be refreshed when used in conjunction with drag and drop.

comment:16 Changed 2 years ago by scott.gonzalez

  • Type changed from enhancement to feature

comment:17 Changed 22 months ago by mikesherov

  • Status changed from reopened to open

comment:18 Changed 22 months ago by JaredSavage

Maybe I'm misunderstanding what's happening, but instead of trying to determine how the items are being displayed, couldn't we just rely on the value of axis?

I changed

this.floating = this.items.length ? o.axis === 'x' || (/left|right/).test(this.items[0].item.css('float')) || (/inline-block|inline-table|inline|table-cell/).test(this.items[0].item.css('display')) : false;

to

this.floating = (o.axis === 'x');

and it completely resolved the issue I was having with horizontal sorting being jumpy, without having any negative impact on my vertical lists.

comment:19 Changed 22 months ago by joern.zaefferer

  • Milestone set to 2.0.0

Consider for redesign.

comment:20 Changed 22 months ago by michaldudek

Workaround is even simplier and doesn't require lib rewrite or plugin reinitialization:

   var $el = $('#sortable').sortable();
   $el.data('sortable').floating = true;
   $el.data('uiSortable').floating = true;
Last edited 22 months ago by michaldudek (previous) (diff)

comment:21 Changed 18 months ago by tj.vantoll

  • Status changed from open to closed
  • Resolution set to fixed
  • Milestone changed from 2.0.0 to 1.8.11

The original issue here was indeed fixed in 1.8.11.

The comments after have all been related to the limitation that the sortable interaction only determines floating during initialization. The following is in _create:

//Let's determine if the items are being displayed horizontally
this.floating = this.items.length ? o.axis === "x" || (/left|right/).test(this.items[0].item.css("float")) || (/inline|table-cell/).test(this.items[0].item.css("display")) : false;

Therefore, if you start with an empty sortable it is assumed to be vertical. You can see this in the following two examples -

Vertical works:  http://jsfiddle.net/tj_vantoll/4A42n/ Horizontal doesn't:  http://jsfiddle.net/tj_vantoll/s867Q/

There's already a feature request to recalculate this.floating in refresh (#7498), which should resolve this. I'll add some relevant comments there and re-mark this as fixed since the originally posted issue has been fixed.

Note: See TracTickets for help on using tickets.