Opened 8 years ago

Closed 8 years ago

#9550 closed enhancement (fixed)

[Patch] [CLA] Adding new item onto a tree can come from a dijit component

Reported by: youngho Owned by: bill
Priority: high Milestone: 1.4
Component: Dijit Version: 1.3.0
Keywords: tree dnd Cc:
Blocked By: Blocking:

Description

Hello,

Adding new item onto a tree can be from a dijit component by dragging. But current implement is only available from a tree or non dijit component.

Here is a patch.

Attachments (3)

dndSource.patch (1.0 KB) - added by youngho 8 years ago.
test_Tree_DnD1.html (8.5 KB) - added by youngho 8 years ago.
Tree_dnd1.html (14.6 KB) - added by youngho 8 years ago.

Download all attachments as: .zip

Change History (11)

Changed 8 years ago by youngho

Attachment: dndSource.patch added

comment:1 Changed 8 years ago by Eugene Lazutkin

Summary: Adding new item onto a tree can come from a dijit component[Patch] Adding new item onto a tree can come from a dijit component

Are you Youngho Cho? Just making sure that we have CLA from you on file. If you are, please add [CLA] to the Summary, otherwise please file a CLA: http://dojotoolkit.org/cla

comment:2 Changed 8 years ago by youngho

Yes I am Youngho Cho.

Sorry I have no idea how to edit the Summary. From next time I will add [CLA] mark in the Summary when I post a patch.

Thanks,

Youngho

comment:3 Changed 8 years ago by youngho

I notice that the attached file has no infomation about against what file.

The patch file is against 'dijit.tree.dndSource.js'

Thanks,

Youngho

comment:4 Changed 8 years ago by bill

Summary: [Patch] Adding new item onto a tree can come from a dijit component[Patch] [CLA] Adding new item onto a tree can come from a dijit component

Can you supply a small test case?

I thought there were other issues w/the tree, namely that the drag source is marked as "text" when it should be marked as "TreeNode?" or "dojo.data item" etc. But I guess that applies to the opposite case, when dragging from the tree.

Changed 8 years ago by youngho

Attachment: test_Tree_DnD1.html added

Changed 8 years ago by youngho

Attachment: Tree_dnd1.html added

comment:5 Changed 8 years ago by youngho

I added a robot test. You may adjust the test file.

comment:6 Changed 8 years ago by bill

Milestone: tbd1.4
Owner: set to bill
Status: newassigned

Fantastic, thanks for writing a test.

comment:7 Changed 8 years ago by bill

I checked out your code and test file. I see that you are fixing the exception calling getParent() when no such function exists... that makes sense.

I think though we might still want to access dragWidget.item (if such an attribute exists), even the the dragWidget is not a TreeNode, in case the item is from the same store.

Does that make sense, or is there a reason not to do that? I changed the code to do that and that seems to work.

comment:8 Changed 8 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [19652]) Number of fixes (!strict) around Tree DnD:

  • When dragging from a Tree and dropping somewhere, source.getItem() should return a dojo.dnd.Item, not a DomNode? (fixes #7058)
    • Pass source to itemCreator() so that the function can see the type of the dragged nodes. Also fixed documentation of itemCreator(). (fixes #9683)
  • fix dragging a random (non TreeNode?) widget and dropping onto the tree. In this case Tree should not assume the dragged widget has a dojo.data item, parentNode, etc. like TreeNode?'s do. (fixes #9550)
  • better documentation (fixes #9684)
  • make Tree drag source support forInSelectedItems(), like dojo.dnd.Source (fixes #9685)
  • Avoid memory leak on IE by using dojo.attr(node, "id", ...) rather than setting id on the node's JS object (not sure if this matters though since id is a standard attribute name, not an expando property). (refs #9614)

I wasn't sure what fields are best to set in the object returned from getItem(). For now it's just setting type == treeNode and data == the TreeNode? widget.

Note: See TracTickets for help on using tickets.