Opened 9 years ago
Closed 8 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:
Change History (11)
comment:1 Changed 9 years ago by
Summary: | dijit/Tree: (Dnd, ObjectStoreModel) Dragging a node within the same parent causes it to become invisible → Tree: (Dnd, ObjectStoreModel) Dragging a node within the same parent causes it to become invisible |
---|
comment:2 Changed 9 years ago by
comment:3 Changed 8 years ago by
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 8 years ago by
Furthermore, childrenCache contains promises, so should be queried by it's getChildren interface.
comment:5 Changed 8 years ago by
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 8 years ago by
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 8 years ago by
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.
comment:8 Changed 8 years ago by
comment:9 Changed 8 years ago by
Sorry for the confusion. The target in dndSource.onDndDrop is the sibling you drag before or after.
comment:10 Changed 8 years ago by
Milestone: | tbd → 1.10 |
---|---|
Status: | new → assigned |
Summary: | Tree: (Dnd, ObjectStoreModel) Dragging a node within the same parent causes it to become invisible → Tree: (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 8 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
#17389 is a duplicate of this ticket.