419343 - allow drag & drop of events from today pane

Status: NEW

(Calendar :: General, enhancement)

Description

[Przemyslaw Bialik]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.12) Gecko/20080201 Firefox/2.0.0.12
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.9) Gecko/20071031 Lightning/0.8pre ID:2008022321 Thunderbird/2.0.0.9 ID:2007103104

Allow drag & drop of events from today pane.

Reproducible: Always

Steps to Reproduce:
1. Try to drag & drop event from today pane to the switch2mail or switch2task button (or tasks section on today pane).
Actual Results:  
You can't.

Expected Results:  
You can, event conversion is possible.

For parity with tasks on today pane and for user friendliness events from today pane should also be dragable.

Comment 1

[Sven Giermann]

Would you agree to solve this together with bug 413663?

I would set this one as a dup for 413663 and add the request there...

Comment 2

[Przemyslaw Bialik]

The bug is currently unassigned and I'm not sure I'm gonna take it.
These bugs are related but I think I would rather set - this bug blocks bug 413663 than marking one as a dupe.

Comment 3

[Sven Giermann]

Make your decision and go on to solve this one...

It would be okay for me to block bug 413663, as I'm waiting for 0.8 to be out before I continue work on this - it's up to you.

Comment 4

[Przemyslaw Bialik]

Well as I've said, feel free to take it, the bug status correctly shows that nobody is working on it.

Comment 5

[Sven Giermann]

Attachment: Add draggesture handler to agenda items.

I just copied the handler for 'draggesture' from 'calendar-month-view.xml' after I recognized, that DND is working on allday-events.
I removed the lines for hiding the event in month view which produces JS errors as 'this.parentBox' is not defined in this context.

There is still this error on allday-events but I think this should be handled in bug 417222, since I think this would remove the re-use of 'calendar-month-day-box-item' anyway... (I will add a comment there)

Comment 6

[Berend Cornelius]

>There is still this error on allday-events but I think this should be handled
>in bug 417222, since I think this would remove the re-use of
>'calendar-month-day-box-item' anyway... (I will add a comment there)

I would not be so sure about this. Christian has not yet added a comment on this and I am not sure if we have the time to change this till 0.9 delivery. Apart from that the month-box-items brings us some advantages on the way namely alarm display  and display of categories.

Regarding your js-error I think I have a simple solution. If you modify your draggesture handler like
> <handler event="draggesture" phase="capturing"><![CDATA[
>    event.stopPropagation();
than the draggesture handler of the month-box-binding won't be called.

Yet I can't help myself to deny the review because of all the code redundancies that you introduced. I know that you are not the first one who did this but that should not be an argument. I think that we should consolidate the several existing "draggesture" code implementations (probably into "calendar-ui-utils.js") and use that new implementation from as many places as possible - maybe not in 1 step, but if we provide a general-to be used function the project will definitely benefit more of it - or have I probably overseen something that makes this hard to accomplish?

Comment 7

[Berend Cornelius]

Comment on attachment 323714 [details] [diff] [review]
Add draggesture handler to agenda items.

denying review based on my prior comments

Comment 8

[Sven Giermann]

(In reply to comment #6)
> Regarding your js-error I think I have a simple solution. If you modify your
> draggesture handler like
> > <handler event="draggesture" phase="capturing"><![CDATA[
> >    event.stopPropagation();
> than the draggesture handler of the month-box-binding won't be called.

Oh, I did not knew this method. But unfortunately the month-box-binding is called BEFORE the newly introduced... Do I have any chance then?

> Yet I can't help myself to deny the review because of all the code redundancies
> that you introduced. I know that you are not the first one who did this but
> that should not be an argument. I think that we should consolidate the several
> existing "draggesture" code implementations (probably into
> "calendar-ui-utils.js") and use that new implementation from as many places as
> possible - maybe not in 1 step, but if we provide a general-to be used function
> the project will definitely benefit more of it - or have I probably overseen
> something that makes this hard to accomplish?

I can clearly understand this.
I'm not sure if it's possible, since the month-box-binding want's to execute some code AFTER initializing the DND content, but BEFORE invokeDragSession().

I don't know enough background information about all those bindings and events to consolidate the handlers - maybe someone else can help me out...

Comment 9

[Berend Cornelius]

>Oh, I did not knew this method. But unfortunately the month-box-binding is
>called BEFORE the newly introduced... Do I have any chance then?
That's why you explicitly assign the "phase" attribute. See also

http://www.xulplanet.com/tutorials/xultu/events.html

Comment 10

[Berend Cornelius]

>I'm not sure if it's possible, since the month-box-binding want's to execute
>some code AFTER initializing the DND content, but BEFORE invokeDragSession().

Couldn't you pass a listener to your function and fire "initializeDone" or something like that? Or just pass a reference to a function similarily to the toolboxes where a "customizeDone". See "CustomizeApplicationToolbar" in messenger-overlay-sidebar.js"

Comment 11

[Sven Giermann]

(In reply to comment #9)
> That's why you explicitly assign the "phase" attribute. See also
> 
> http://www.xulplanet.com/tutorials/xultu/events.html
I cannot see a possibility to ASSIGN the handler to a specific phase...
But thanks for the link.

Is the only way to add it later using 'addEventListener()' or is it best to call 'removeEventListener()' on the 'calendar-month-day-box-item' to prevent the month-box-handler?

Comment 12

[Berend Cornelius]

Is my proposed code not working on your system? I tested it and it worked fine...

Comment 13

[Sven Giermann]

Upps... it seems I couldn't see the wood for the trees!

> phase="capturing"
indeed makes it working, thanks.

I'll see if I can build up a general solution, as you proposed.

Comment 14

[Berend Cornelius]

Attachment: [checked in] patch v. #1

The patch attached initializes a dragsession for the agenda-listbox as well as for the task-trees. What still lacks is that the drop targets are not always dealing correctly with the items of the sourceNode. I would like to deal with these issues in other bugs respectively  
- Bug 459641 -  New Tabs should perform as drop target 
and
-  Bug 416138 -  Moving of all-day-events in weekview not possible, but shows possibility
I don't mind if we leave this issue open until these dependencies are resolved and all droptargets react well, yet I would like to have this patch reviewed beforehand - if possible.
To test this patch I find the best way is to drag the items to the calendar-month-view.

Comment 15

[Daniel Boelzle [:dbo]]

Comment on attachment 352324 [details] [diff] [review]
[checked in] patch v. #1

r=dbo

Comment 16

[Berend Cornelius]

patch v. #1 pushed to comm-central:

http://hg.mozilla.org/comm-central/rev/f76c25e255ec

As said in comment #14 I leave this issue open until the droptargets deal with the drag-session appropriately.