Opened 10 years ago
Closed 10 years ago
#13713 closed defect (fixed)
Tree: problem making node a child of itself
Reported by: | Siqi | Owned by: | bill |
---|---|---|---|
Priority: | high | Milestone: | 1.7 |
Component: | Dijit | Version: | 1.6.1 |
Keywords: | dijit.Tree, DnD | Cc: | [email protected]… |
Blocked By: | Blocking: |
Description
Open dijit/tests/tree/test_Tree_DnD.html, following DnD operations on the 'collectionsTree' result in unexpected structure changes.
- Hold on the copy key ('ctrl' for windows), copy the node 'Cereals' and append it to 'Vegetables' through DnD operation.
- Release the copy key, try to move the original 'Cereals' to under the new 'Cereals'.
Result: The second DnD operation cannot be done correctly.
Attachments (1)
Change History (5)
Changed 10 years ago by
comment:1 follow-up: 2 Changed 10 years ago by
Summary: | Internal DnD issue in dijit.Tree. → Tree: problem making node a child of itself |
---|
comment:2 Changed 10 years ago by
Replying to bill:
Actually, the "copy" operation isn't really copying the Cereals item, but merely setting two parents for that item. Then when you try to make Cereals a child of Cereals that is making a node a child of itself, which isn't allowed. #7140 was supposed to explicitly disallow that but I guess it doesn't handle this case. I'll look into why.
Seems I misunderstood the meaning of the 'copy' operation here. It sounds more like the 'link' semantic?
In the fix of #7140: following code in dndSource.js checks whether the node is in the selection by widget id rather than data item id. In the 'Cereals' case, although two 'Cereals' node share the same item, their widget id are different.
if(m.source == this && (newTarget.id in this.selection)
comment:3 Changed 10 years ago by
Milestone: | tbd → 1.7 |
---|---|
Priority: | high → normal |
severity: | major → normal |
Thanks, you are right about the fix, I'll add that in.
As for copy vs. link, yes, the default is to link. I realize that in many cases, like windows explorer, "copy" makes more sense than "link", but in those cases apps just need to supply a custom pasteItem() implementation, one that generates a new item with a new id. Even if TreeStoreModel had code to generate a new item, it would still need help to get a new id.
Note that sometimes link makes more sense than copy, such as an organization tree for a company. You don't want to create a new copy of an employee, because it would imply that you had actually cloned that employee :-). But perhaps most of the time copy is wanted.
comment:4 Changed 10 years ago by
Owner: | set to bill |
---|---|
Resolution: | → fixed |
Status: | new → closed |
In [26413]:
Actually, the "copy" operation isn't really copying the Cereals item, but merely setting two parents for that item. Then when you try to make Cereals a child of Cereals that is making a node a child of itself, which isn't allowed. #7140 was supposed to explicitly disallow that but I guess it doesn't handle this case. I'll look into why.