Opened 7 years ago

Closed 6 years ago

#16692 closed defect (fixed)

Tree: (Dnd, ObjectStoreModel) dropping node onto its parent causes it to become invisible

Reported by: noxryan Owned by: bill
Priority: undecided Milestone: 1.10
Component: Dijit Version: 1.8.3
Keywords: Cc:
Blocked By: Blocking:

Description

This issue only occurs when using the new ObjectStoreModel? and Memory store. If you drag an item into its current parent it disappears. This is especially a problem if you have a betweenThreshold that allows the re-ordering of items within the same parent. This issue can be reproduced using the dijit Dnd Tree 1.8 example here:

http://dojotoolkit.org/documentation/tutorials/1.8/store_driven_tree/demo/demo.php

I am not entirely sure if this issue has to do with dijit/Tree or dijit/tree/ObjectStoreModel.

Here are some others that have reported the issue:

http://dojo.6188.n7.nabble.com/v1-8-dijit-Tree-DnD-of-nodes-makes-change-invisible-until-subsequent-DnD-action-td8630.html

http://dojo-toolkit.33424.n3.nabble.com/dijit-Tree-DND-betweenThreshold-bug-please-help-td3992612.html

Change History (11)

comment:1 Changed 6 years ago by bill

Summary: dijit/Tree: (Dnd, ObjectStoreModel) Dragging a node within the same parent causes it to become invisibleTree: (Dnd, ObjectStoreModel) Dragging a node within the same parent causes it to become invisible

comment:2 Changed 6 years ago by bill

#17389 is a duplicate of this ticket.

comment:3 Changed 6 years ago by Wouter Hager

The problem is simple: around line 467 in dijit/tree/dndSource it says

// Expand the target node (if it's currently collapsed) so the user can see
// where their node was dropped.   In particular since that node is still selected.
this.tree._expandNode(target);

But around line 198 in dijit/tree/ObjectStoreModel it says

if(!bCopy){
	// In order for DnD moves to work correctly, childItem needs to be orphaned from oldParentItem
	// before being adopted by newParentItem.   That way, the TreeNode is moved rather than
	// an additional TreeNode being created, and the old TreeNode subsequently being deleted.
	// The latter loses information such as selection and opened/closed children TreeNodes.
	// Unfortunately simply calling this.store.put() will send notifications in a random order, based
	// on when the TreeNodes in question originally appeared, and not based on the drag-from
	// TreeNode vs. the drop-onto TreeNode.

	var oldParentChildren = [].concat(this.childrenCache[this.getIdentity(oldParentItem)]), // concat to make copy
		index = array.indexOf(oldParentChildren, childItem);
	oldParentChildren.splice(index, 1);
	this.onChildrenChange(oldParentItem, oldParentChildren);
}

This is plain wrong, because if the child treeNodes are orphaned, they can never be expanded. If the target is going to be removed first, shouldn't there be a _loadDeferred on that treeNode?

Ouch. ObjectStoreModel? should not have been promoted like this.

comment:4 Changed 6 years ago by Wouter Hager

Furthermore, childrenCache contains promises, so should be queried by it's getChildren interface.

comment:5 Changed 6 years ago by bill

if the child treeNodes are orphaned, they can never be expanded.

No one is trying to expand the dragged nodes (i.e. the "child treeNodes"). The idea is to expand the drop target. And the drop target is not being orphaned.

Also, the idea is to orphan the child nodes temporarily, but then attach them to their new parent.

comment:6 Changed 6 years ago by Wouter Hager

Because the node is dropped in between, the target is also a child.

But I think the main problem is that pasteItem doesn't call getChildren, but uses childrenCache[parentId], which returns a Promise.

comment:7 Changed 6 years ago by bill

Because the node is dropped in between, the target is also a child.

That's hard to believe. The drop target should still be the parent node. In the signature of pasteItem:

pasteItem: function(/*Item*/ childItem, /*Item*/ oldParentItem, /*Item*/ newParentItem,
			/*Boolean*/ bCopy, /*int?*/ insertIndex, /*Item*/ before){

oldParentItem and newParentItem should both point to the same item, the parent of the node being dragged.

But I think the main problem is that pasteItem doesn't call getChildren, but uses childrenCache[parentId], which returns a Promise.

That does sound like a bug, although the demo test page uses the synchronous Memory store, in which case IIRC childrenCache will contain the children itself, not promises.

So I don't think that's what's causing the problem reported here.

Last edited 6 years ago by bill (previous) (diff)

comment:8 Changed 6 years ago by Wouter Hager

You're right, but it might be related. I've created a new ticket specifically for all problems regarding Tree and JsonRest? (it may be split into sub-problems).

See #17836

comment:9 Changed 6 years ago by Wouter Hager

Sorry for the confusion. The target in dndSource.onDndDrop is the sibling you drag before or after.

comment:10 Changed 6 years ago by bill

Milestone: tbd1.10
Status: newassigned
Summary: Tree: (Dnd, ObjectStoreModel) Dragging a node within the same parent causes it to become invisibleTree: (Dnd, ObjectStoreModel) dropping node onto its parent causes it to become invisible

I may backport the Tree fixes from 1.10 to previous releases but I want to let them bake for a while (in the master branch) first.

comment:11 Changed 6 years ago by Bill Keese <bill@…>

Resolution: fixed
Status: assignedclosed

In 3a1e6be4f9ce0c7b2fcd72ef68d2600bcb6f75a4/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 
Note: See TracTickets for help on using tickets.