Fix JQuery Sortable to Operate with Meteor UI (Blaze)

[sotarules]

As you can see in the DIFF view, my fix is largely limited to functions "_clear" and "cancel" (plus an additional property in the domPosition object). The idea is to take special care so that when a JQuery Sortable drag/drop operation is cancelled, the node that was being dragged is returned to precisely the same position in the DOM relative to its original siblings. In my own system, this seems to work beautifully. Since the DOM is unchanged, Meteor UI is able to properly perform "differencing" in the DOM to update it correctly in response to state change notifications from Mongo.

[sotarules]

Friends, the one part I'm a little queasy about is the while loop in _clear, I got the idea for this elsewhere in the code where they were using the JQuery .not() function to skip over unwanted nodes. I tried to do something similar but it didn't work, so I coded the while.

[scottgonzalez]

Please follow the instructions in the contributing guidelines and let us know when the PR is ready for a review. Thanks.

[sotarules]

I see that I have some JSLint issues, let me fix them.

[scottgonzalez]

Why is next sibling more important than previous sibling? I'm still not sure what problem you're having. In the ticket you said "However, the internal arrangement of nodes at a low level (siblings) is changed." but didn't explain which elements are being moved around that shouldn't. I don't see how the elements could end up in the wrong order if the visual order is correct.

[sotarules]

Scott, this took a little learning on my part, so maybe a picture can help: Consider a series of sortable items (ITEM A, ITEM B, ITEM C), represented in DOM as list items (li1 - li3).

Meteor creates so called "marker" text nodes to "sandwich" the list items, so graphically you could represent the DOM like this:

ITEM A: textstart text text li1 text text textend
ITEM B: textstart text text li2 text text textend
ITEM C: textstart text text li3 text text textend

Meteor UI code comments use the word "significant" to describe the li1, li2, li3 element nodes, but Meteor also keeps track of the insignificant text nodes as well, in particular the start and end text nodes, which Meteor uses to delimit ranges of information in the DOM. It carries references to the start and end nodes in Meteor "DomRange" metadata.

Now, if the user were to drag ITEM C around and then drop it back in precisely the same position (the end), this is what happens to the DOM:

ITEM A: textstart text text li1 text text textend
ITEM B: textstart text text li2 li3 text text textend
ITEM C: textstart text text text text textend

Note that li3 was removed from ITEM C range, but added to the range of siblings "owned" by ITEM B. After the drag operation, ITEM B range has two "significant" nodes, yet ITEM C range has no "significant" nodes. ITEM C range now only has the insignificant text nodes.

When the drag ends, JQuery Sortable reinstates li3 immediately after li2. The display looks fine; however, note the li3 element has moved out of the ITEM C range and into ITEM B range. Although the UI looks right, JQuery Sortable did not return the DOM to its original state. The element node has been moved significantly from its original position via-a-vis ITEM C text nodes.

Meteor UI (Blaze) uses these delimiting start and end text nodes to define ranges that are automatically managed as data in Mongo is updated. IMHO this is actually a design issue in Meteor that needs more work, because creating marker start and end text nodes and keeping track of them is not friendly for tools like JQuery UI that manipulate the DOM at the higher level (i.e., elements).

I have noted this in an already-open defect in Meteor regarding inconsistent behavior with JQuery Sortable. Unfortunately, to fix this correctly would be a significant change in Meteor, so to deal with this in the interim, my changes make it so that li3 will be returned to precisely the same position relative to the original "sibling" text nodes in the range of ITEM C.

In my own testing, this change yields good behavior in Meteor and will be welcomed by Meteor developers, who look forward to JQuery Sortable working seamlessly as a rite of passage for Meteor UI. Note that my code only affects how drag operation is cancelled, or when an element is returned to its original position.

[scottgonzalez]

Thanks for explaining that in detail, though I still don't understand why using the next sibling would work if using previous sibling won't. Anyway, this is similar to the insanity that Angular does with comment nodes (see http://bugs.jqueryui.com/ticket/9727). I don't think we want to support this.

[sotarules]

Scott, I think the simplest way to explain it is that nextSibiling and/or previousSibling is not necessarily an element (node type 1). It can be a text node (node type 3). Important: "sibling" in this context isn't the what people coding JQuery are used to; it is the lower-level DOM definition, namely the next or previous entry in the DOM's childNodes array, which contains a mixture of both type 1 and type 3 nodes.

The current coding of JQuery Sortable doesn't recognize or care about anything but type 1 nodes, this is built into the very selectors that JQuery uses, which choose type 1 nodes. JQuery definitions of previous sibling or next sibling are elements. But at the lower level, the DOM is comprised of a mixture of both element nodes and text nodes. Unfortunately for us, Meteor operates at this lower level, and takes the text nodes (type 3) into account as significant.

I do understand your decision not to support this, though. What I think I'll do next is try to write a "punch" to dynamically modify JQuery Sortable cancel and _clear methods. I'll make this a meteorite so the Meteor community can use Sortable without affecting the JQuery UI community at large.

Thankfully, unlike Angular, Meteor doesn't use comment nodes (node type 8), but the concept of bastardizing the nodes as "markers" seems similar.

[scottgonzalez]

I understand the different element types and how jQuery works. I just didn't understand why you added the next sibling tracking instead of adjusting the previous sibling tracking, but now I'm realizing it was because you didn't want to (and probably couldn't) change the other parts of the existing code that do the position checks.

[tjvantoll]

@sotarules Just fyi, the widget factory gives you lots of hooks to help you "punch" the widget in an extension: http://learn.jquery.com/jquery-ui/widget-factory/extending-widgets/. And if you do come up with a solution, please comment on the original ticket (http://bugs.jqueryui.com/ticket/10062). We can add it in bold on the top to help others trying to integrate the widget with Meteor.

[sotarules]

Scott, that's exactly right, I was trying to be super careful (since I'm noob), so I added nextSibling (which is admittedly redundant with domPosition.prev). Now that I look at all references to domPosition.prev, it would be possible to completely eliminate domPosition.prev and use only domPosition.nextSibling. The key would be to refactor a complex condition that controls when the update event is triggered:

        if((this.fromOutside || this.domPosition.prev !== this.currentItem.prev().not(".ui-sortable-helper")[0] || this.domPosition.parent !== this.currentItem.parent()[0]) && !noPropagation) {
            delayedTriggers.push(function(event) { this._trigger("update", event, this._uiHash()); }); //Trigger update callback if the DOM position has changed
        }

I think when I saw this condition, I got cold feet about removing domPosition.prev, but now that I look again, it isn't so bad.

Tjvanttoll, thanks very much for the tip, I'll look into extending widgets. The Meteor community should be stoked if I can pull this off.