Bug 370283 - [api] allow clients to be notified of drop events in the task list
Summary: [api] allow clients to be notified of drop events in the task list
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Mylyn (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 7
: P2 enhancement (vote)
Target Milestone: 3.7   Edit
Assignee: Sam Davis CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2012-01-31 17:40 EST by Sam Davis CLA
Modified: 2012-02-15 18:33 EST (History)
3 users (show)

See Also:


Attachments
patch (6.93 KB, patch)
2012-01-31 18:57 EST, Sam Davis CLA
no flags Details | Diff
mylyn/context/zip (19.76 KB, application/octet-stream)
2012-01-31 18:57 EST, Sam Davis CLA
no flags Details
tooltip (10.52 KB, image/png)
2012-02-01 13:27 EST, Sam Davis CLA
no flags Details
patch (20.87 KB, patch)
2012-02-15 14:21 EST, Sam Davis CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Davis CLA 2012-01-31 17:40:33 EST
I would like to be able to extend the TaskListDropAdapter, in particular to add custom behaviour when a task is dragged onto another task if some modifier key is held.
Comment 1 Sam Davis CLA 2012-01-31 18:57:14 EST
Created attachment 210344 [details]
patch

Something like this could work. I'm not sure what the dragOver method was doing other than changing the drag icon, and I think it makes more sense to indicate that dragging to a repository task without modifiers is not allowed, which this does.
Comment 2 Sam Davis CLA 2012-01-31 18:57:18 EST
Created attachment 210345 [details]
mylyn/context/zip
Comment 3 Steffen Pingel CLA 2012-02-01 09:31:13 EST
Sam, can you provide an example what kind of behavior extensions would implement? I'm worried that if we make DnD extensible it's going to be very hard to predict for users what happens when they drag and drop. Also this seems very difficult to discover.
Comment 4 Sam Davis CLA 2012-02-01 13:27:51 EST
Created attachment 210391 [details]
tooltip

That's part of the reason I want to restrict it to dragging tasks onto repository tasks - this is something which currently has no effect (although the UI makes it look like it will do something), and the behaviour for other kinds of drags will not be able to be changed.

In this case, there is a connector which has a field for listing related tasks in other repositories. I would like to be able to add a task to that field by dragging in the task list. I agree this is not easy to discover - it would be a more advanced feature that users of the connector would most likely have to read about, as a shortcut to opening the task and editing the field directly.

We could also consider popping up some kind of tooltip on drag to inform the user of what will happen, similar to what Windows does (see attachment).
Comment 5 Benjamin Muskalla CLA 2012-02-02 04:27:40 EST
> this is something which currently has no effect 
Last time I looked at the drop adapter, it had only an effect for local tasks to move the task to the same container (eg category), a behavior I really didn't expect ;)
Comment 6 Sam Davis CLA 2012-02-02 11:43:47 EST
(In reply to comment #5)
> > this is something which currently has no effect
> Last time I looked at the drop adapter, it had only an effect for local tasks to
> move the task to the same container (eg category), a behavior I really didn't
> expect ;)

Right, dragging onto _repository_ tasks has no effect.
Comment 7 Steffen Pingel CLA 2012-02-02 18:21:29 EST
I like how Windows shows the action that is happening. Can we do something similar with SWT?

I really like the idea of adding associations through drag and drop. My sense is though that the drag and drop handler is not the right point to provide this extensibility. I would prefer if we added support for creating associations to the framework to support this for any connector that has these kind of associations. This is also related to bug 341307: Support local tasks as subtasks of repository tasks.
Comment 8 Sam Davis CLA 2012-02-03 14:43:23 EST
Rather than allowing overriding of DnD behaviour, which could be problematic, it would be sufficient to provide a notification mechanism for when tasks are dropped on other tasks. I have pushed a code review.
Comment 9 Steffen Pingel CLA 2012-02-03 17:11:38 EST
Thanks! Can you move the review to the master branch? It looks like it's currently building against the e_3_7_m_3_6_x branch: http://review.mylyn.org/#change,255
Comment 10 Sam Davis CLA 2012-02-03 17:23:09 EST
How do I do that? Do I just rebase my branch on master?
Comment 11 Steffen Pingel CLA 2012-02-03 17:45:40 EST
(In reply to comment #10)
> How do I do that? Do I just rebase my branch on master?

Yes, that should do the trick. You can then push the rebased branch again and it should create another patch set.
Comment 12 Sam Davis CLA 2012-02-03 19:12:39 EST
review: http://review.mylyn.org/#change,255
Comment 13 Steffen Pingel CLA 2012-02-10 11:01:22 EST
I have commented on the review. It'd be great if you addressed the nits and attached a patch.
Comment 14 Sam Davis CLA 2012-02-10 18:20:28 EST
Shawn has pointed out that it would make sense for clients to also be notified when a task is dropped into the task editor, instead of trying attach an xml.zip file. I will look into adding that as part of the same notification mechanism, with another Operation type:
public static enum Operation {
		COPY, LINK, DROP_ON_TASK_EDITOR
	};
Comment 15 Steffen Pingel CLA 2012-02-11 07:05:16 EST
We may also want to consider the same event mechanism for tasks in the search view.
Comment 16 Sam Davis CLA 2012-02-13 21:53:36 EST
(In reply to comment #15)
> We may also want to consider the same event mechanism for tasks in the search
> view.

I wasn't able to quickly see a way to do that and I probably won't have time to look into it further for this release.
Comment 17 Steffen Pingel CLA 2012-02-15 07:48:57 EST
(In reply to comment #16)
> I wasn't able to quickly see a way to do that and I probably won't have time to
> look into it further for this release.

Sounds good. We can always add that later if needed.

I have commented on the latest patch set. I'd be great if you could address the last two nits and then attach a patch. Thanks!
Comment 18 Sam Davis CLA 2012-02-15 14:21:48 EST
Created attachment 211068 [details]
patch
Comment 19 Steffen Pingel CLA 2012-02-15 14:39:45 EST
It doesn't apply cleanly. Please rebase/merge against the latest master.
Comment 20 Steffen Pingel CLA 2012-02-15 14:40:22 EST
I take that back, I had the wrong branch. Merging now.
Comment 21 Steffen Pingel CLA 2012-02-15 18:33:10 EST
Great contribution! I have applied the patch to master.