Tree: DnD Using onDrop controlled mode results in wrong child order

[RohdeSchwarz-SDC]

Describe the bug

The drag & drop feature of the tree component works fine, unless we have the following constellation:

Conditions:

• using onDrop controlled mode instead of dropRestrict="sibling".
• ajax event dragdrop with a listener but without update attribute and
• the dragged item is dropped on the same level (reorder only) and
• the dragged item is moved downwards (but not below the last item)

Scenario:
Let there be three items [A, B, C]. To reproduce the bug you must move A one step down between B and C. When the "dragdrop" listener is called, we get a wrong child order:

• Actual [B, C, A],
• Expected [B, A, C]

Reproducer

PrimeFaces-Reproducer-11513.zip

Expected behavior

(see above)

PrimeFaces edition

Community

PrimeFaces version

13

Theme

irrelevant

JSF implementation

Mojarra

JSF version

4.0

Java version

21

Browser(s)

irrelevant

[melloware]

Weird i can't reproduce:
pf-11513.zip

Run mvn clean jetty:run and navigate to localhost:8080

[melloware]

Nevermind I see its the backend printout that is wrong.

[melloware]

Got to the bottom of it. looks like this has been incorrect for a LONG time...

[RohdeSchwarz-SDC]

Excellent! This workaround fixes the symptoms 👍

[melloware]

Great this will be in 13.0.7

[RohdeSchwarz-SDC]

Hang on. Sorry for the premature confirmation.

We might get in trouble with another cornercase this time:

[melloware]

let me look its probably the reverse problem

[melloware]

hmm that is not happening for me.

[TreeBean::init......] root.children = [A, B, C]
[TreeBean::onDragDrop] root.children = [A, C, B]

[melloware]

oh no i see.

[RohdeSchwarz-SDC]

It is very subtle indeed. I also had to look thrice.

[melloware]

Try this patch. It even fixes an issue where the AJAX event should not even be sent if the node is not being moved like your scenario above.

if (PrimeFaces.widget.VerticalTree) {
    PrimeFaces.widget.VerticalTree.prototype.makeDropPoints = function(elements) {
        var $this = this,
            dragdropScope = this.cfg.dragdropScope || this.id;

        elements.droppable({
            hoverClass: 'ui-state-hover',
            accept: '.ui-treenode-content',
            tolerance: 'pointer',
            scope: dragdropScope,
            drop: function(event, ui) {
                var dragSource = PF($(ui.draggable.data('dragsourceid')).data('widget')),
                    dropSource = $this,
                    dropPoint = $(this),
                    dropNode = dropPoint.closest('li.ui-treenode-parent'),
                    dropNodeKey = $this.getRowKey(dropNode),
                    transfer = (dragSource.id !== dropSource.id),
                    draggedSourceKeys = dragSource.draggedSourceKeys,
                    isDroppedNodeCopy = ($this.cfg.dropCopyNode && $this.shiftKey),
                    draggedNodes,
                    dragNodeKey;

                if (draggedSourceKeys) {
                    draggedNodes = dragSource.findNodes(draggedSourceKeys);
                } else {
                    draggedNodes = [ui.draggable];
                }

                if ($this.cfg.controlled) {
                    $this.droppedNodeParams = [];
                }

                $this.invalidSourceKeys = [];

                for (var i = (draggedNodes.length - 1); i >= 0; i--) {
                    var draggedNode = $(draggedNodes[i]),
                        dragMode = ui.draggable.data('dragmode'),
                        dragNode = draggedNode.is('li.ui-treenode') ? draggedNode : draggedNode.closest('li.ui-treenode'),
                        dragNode = (isDroppedNodeCopy) ? dragNode.clone() : dragNode,
                        targetDragNode = $this.findTargetDragNode(dragNode, dragMode);

                    dragNodeKey = $this.getRowKey(targetDragNode);

                    if (!transfer && dropNodeKey && dropNodeKey.indexOf(dragNodeKey + '_') === 0) {
                        return;
                    }

                    if ($this.cfg.controlled) {
                        $this.droppedNodeParams.push({
                            'ui': ui,
                            'dragSource': dragSource,
                            'dragNode': dragNode,
                            'targetDragNode': targetDragNode,
                            'dropPoint': dropPoint,
                            'dropNode': dropNode,
                            'transfer': transfer
                        });
                    } else {
                        $this.onDropPoint(ui, dragSource, dragNode, targetDragNode, dropPoint, dropNode, transfer);
                    }
                }

                if (!draggedSourceKeys) {
                    draggedSourceKeys = [dragNodeKey];
                }
                draggedSourceKeys = draggedSourceKeys.filter(function(key) {
                    return $.inArray(key, $this.invalidSourceKeys) === -1;
                });

                // #11513 dndIndex is based on whether you are dragging up or down
                var dndIndex = dropPoint.prevAll('li.ui-treenode').length;

                if ($this.cfg.controlled) {
                    var isSameParent = dragNodeKey && dragNodeKey.startsWith(dropNodeKey);
                    var draggedIndex = parseInt(dragNodeKey.split('_').pop());

                    if (isNaN(draggedIndex)) {
                        draggedIndex = parseInt(dragNodeKey);
                    }

                    // #11520 if its the same parent but you are not moving the node to a new area it can be dropped
                    if (isSameParent && (draggedIndex === dndIndex || (Math.abs(draggedIndex - dndIndex) === 1 && draggedIndex + 1 === dndIndex))) {
                        draggedSourceKeys = null; // do nothing
                    } else if (draggedIndex < dndIndex) {
                        var nextDndIndex = dropPoint.nextAll('li.ui-treenode').length;
                        if (nextDndIndex > 0) {
                            dndIndex = nextDndIndex;
                        }
                    }
                }

                if (draggedSourceKeys && draggedSourceKeys.length) {
                    draggedSourceKeys = draggedSourceKeys.reverse().join(',');
                    $this.fireDragDropEvent({
                        'dragNodeKey': draggedSourceKeys,
                        'dropNodeKey': dropNodeKey,
                        'dragSource': dragSource.id,
                        'dndIndex': dndIndex,
                        'transfer': transfer,
                        'isDroppedNodeCopy': isDroppedNodeCopy
                    });
                }

                dragSource.draggedSourceKeys = null;
                $this.invalidSourceKeys = null;

                if (isDroppedNodeCopy) {
                    $this.initDraggable();
                }
            }
        });
    }
}

[melloware]

pf-11513.zip

I removed onDrop and added dropRestrict="sibling" and this appears to be working as expected.

[melloware]

This issue is in controlled mode using onDrop it thinks you are in more control of how the drops are handled which is out of the normal course of action. So I think the issue really is the controlled mode really fully baked.