Move events between calendars using drag-and-drop

ASSIGNED
Assigned to

Status

Calendar
Calendar Views
--
enhancement
ASSIGNED
16 years ago
4 years ago

People

(Reporter: Henrik Gemal, Assigned: Fallen)

Tracking

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

16 years ago
It seems impossible to drag'n'drop events between the different calenders. I try
to drag an event from the Events view to the Calendars event.

Updated

15 years ago
Severity: normal → enhancement

Comment 1

15 years ago
New contact from mikep@oeone.com to mostafah@oeone.com
Filter on string OttawaMBA to get rid of these messages. 
Sorry for the spam.
Assignee: mikep → mostafah

Comment 2

14 years ago
*** Bug 202357 has been marked as a duplicate of this bug. ***

Comment 3

13 years ago
I repost here Pieter Ennes' comment given to duplicate bug 202357:

Also, it should then be possible to change the "Calendar file" box in the
event's properties for the same effect.

Updated

12 years ago
QA Contact: colint → sunbird
Reassigning all automatically assigned bugs from Mostafa to nobody@m.o

Bugspam filter: TorontoMostafaMove
Assignee: mostafah → nobody
Component: Sunbird Only → Calendar Views
OS: Windows XP → All
QA Contact: sunbird → views
Hardware: PC → All

Updated

9 years ago
Keywords: helpwanted
Summary: Move events between Calendars (drag'n'drop) → Move events between calendars using drag-and-drop
(Assignee)

Updated

9 years ago
Duplicate of this bug: 496814
(Assignee)

Comment 6

9 years ago
Given there is already some code in place to handle dropping things on the calendar list, this shouldn't be too hard. If not as a good first bug, then as a good second bug :-)

When implementing, please make use of the new drag and drop API as much as possible, see https://developer.mozilla.org/En/DragDrop/Drag_and_Drop
Whiteboard: [good first bug]
(Assignee)

Comment 7

9 years ago
Created attachment 397518 [details] [diff] [review]
Fix - v1

This should take care. I hope that no twistys are drawn. It was needed to make all items empty containers so that its possible to drop items *on* the tree rows, not before/after.
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Attachment #397518 - Flags: review?(mschroeder)
(Assignee)

Updated

9 years ago
Keywords: helpwanted
Comment on attachment 397518 [details] [diff] [review]
Fix - v1

Drag-and-drop of calendar item:
* Event is copied not moved.
* No automatic refresh of view/today pane after DnD.
* DnD seems to be possible on disabled calendars (with no error message) and on read-only calendars (with MODIFICATION FAILED error message), but drop shouldn't be available at all in those cases.

Drag-and-drop of calendar:
* Visual indicator (insertion point) of DnD could be improved (follow-up bug).
Attachment #397518 - Flags: review?(mschroeder) → review-
(In reply to comment #7)
> This should take care. I hope that no twistys are drawn. It was needed to make
> all items empty containers so that its possible to drop items *on* the tree
> rows, not before/after.

Is it possible to show always an insertion point for a dragged calendar, also when the cursor is over a calendar. Maybe just under this one?
(Assignee)

Comment 10

9 years ago
(In reply to comment #8)
> (From update of attachment 397518 [details] [diff] [review])
> Drag-and-drop of calendar item:
> * Event is copied not moved.
> * No automatic refresh of view/today pane after DnD.
I couldn't reproduce this, WFM.

> * DnD seems to be possible on disabled calendars (with no error message) and on
> read-only calendars (with MODIFICATION FAILED error message), but drop
> shouldn't be available at all in those cases.
Yes, I'll need to fix this, forgot about that.

> Drag-and-drop of calendar:
> * Visual indicator (insertion point) of DnD could be improved (follow-up bug).
Right now the only visual indicator is the drag icon, which I agree isn't the best. We could add some styles for drag over on tree rows, I'd need a UI decision on that though.



(In reply to comment #9)
> Is it possible to show always an insertion point for a dragged calendar, also
> when the cursor is over a calendar. Maybe just under this one?

dragged calendar? There should be insertation points also without this patch for dragging calendars. Or do I misunderstand?
(Assignee)

Updated

9 years ago
Whiteboard: [good first bug] → [needs review]
(In reply to comment #10)
> (In reply to comment #8)
> > (From update of attachment 397518 [details] [diff] [review] [details])
> > Drag-and-drop of calendar item:
> > * Event is copied not moved.

This is just true for recurring events!


> > * No automatic refresh of view/today pane after DnD.
> I couldn't reproduce this, WFM.

Can't reproduce it anymore (maybe fixed in another bug)!


> > Drag-and-drop of calendar:
> > * Visual indicator (insertion point) of DnD could be improved (follow-up bug).
> Right now the only visual indicator is the drag icon, which I agree isn't the
> best. We could add some styles for drag over on tree rows, I'd need a UI
> decision on that though.

The behavior before applying this patch was different. Dragging a calendar exactly over another one showed the appropriate insertion point, now it shows the symbol for a not possible drop.


> (In reply to comment #9)
> > Is it possible to show always an insertion point for a dragged calendar, also
> > when the cursor is over a calendar. Maybe just under this one?
> 
> dragged calendar? There should be insertation points also without this patch
> for dragging calendars. Or do I misunderstand?

This is connected with the other drag-and-drop of calendar issue mentioned... it's the suggestion to restore the old behavior. ;)
Whiteboard: [needs review]
(Assignee)

Comment 12

9 years ago
Created attachment 405692 [details] [diff] [review]
Fix - v2

(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #8)
> > > (From update of attachment 397518 [details] [diff] [review] [details] [details])
> > > Drag-and-drop of calendar item:
> > > * Event is copied not moved.
> 
> This is just true for recurring events!
Fixed. This brings up a new issue: How should we copy recurring events? Should the whole series be copied? Should just that occurrence be copied? If the latter, should the event in the new calendar be a single event?

What I did now is to add some admittedly cludgy code to:

* ask the user if only this occurrence or all occurrences should be "edited"
  - Our Dialog doesn't do "move" at the moment, we need strings if this is
    wanted.

* if he wants the series, then move the master event
* if he wants the occurrence, create a batch transaction that
  - removes the occurrence from the original event
  - creates a new, non-recurring event on the target calendar


> The behavior before applying this patch was different. Dragging a calendar
> exactly over another one showed the appropriate insertion point, now it shows
> the symbol for a not possible drop.
Yes, I can imagine why this happens. The only way to allow dropping on items at all is to return true in isContainer. This makes the tree code differentiate between dropping before/on/after instead of just before/after, which causes the behavior you mentioned. There is no easy way to make both work, other than maybe changing whats returned in isContainer when starting the drag, but that doesn't really seem right. I guess we just have to live with it.
Attachment #397518 - Attachment is obsolete: true
Attachment #405692 - Flags: review?
Philipp, did you want to ask review of someone in particular?
(Assignee)

Comment 14

9 years ago
Comment on attachment 405692 [details] [diff] [review]
Fix - v2

Yes, thanks for catching it!
Attachment #405692 - Flags: review? → review?(mschroeder)
@Philipp:
Perhaps it's time to clean this patch from bitrot and search for a new reviewer?
(Assignee)

Comment 16

6 years ago
Comment on attachment 405692 [details] [diff] [review]
Fix - v2

Definitely, I'm working on it. Putting this in my own queue just as a reminder
Attachment #405692 - Flags: review?(mschroeder) → review?(philipp)
(Assignee)

Comment 17

4 years ago
Created attachment 8392816 [details] [diff] [review]
Fix - v3

Here is an unbitrotted patch. Martin as you did the initial review and I've been hearing a little more from you lately, do you want to complete this review? If you don't have enough time you can defer to Decathlon or someone else.
Attachment #405692 - Attachment is obsolete: true
Attachment #405692 - Flags: review?(philipp)
Attachment #8392816 - Flags: review?(mschroeder)
(Assignee)

Updated

4 years ago
Attachment #8392816 - Flags: review?(mschroeder) → review?(bv1578)

Comment 18

4 years ago
Comment on attachment 8392816 [details] [diff] [review]
Fix - v3

Philipp, I tested on trunk but, unless I missed something, this patch doesn't work for me.
When dragging events from the views to the calendar list, the pointer's visual indicator doesn't show dropping possibility when hovering the calendars and all the calendars panel.
More precisely, when dragging from month/multiweek views, the pointer says that dropping is not possible, instead when dragging from day/week view, the pointer doesn't show any information. In both cases nothing happens when dropping the items on the calendars.
Attachment #8392816 - Flags: review?(bv1578) → review-

Comment 19

4 years ago
I've taken a look to the code, it seems that the variable "dragItem" has not been declared (and assigned) inside the method "canDrop".
Adding something such as 

   let dragItem = dragSession.sourceNode && dragSession.sourceNode.sourceObject;

on the bottom of this part of the code:

>@@ -742,17 +742,24 @@
>         <parameter name="aOrientation"/>
>         <body><![CDATA[
>           let dragSession = cal.getDragService().getCurrentSession();
>-          let dataTransfer = dragSession && dragSession.dataTransfer;
>-          if (!this.allowDrag || !dataTransfer) {
>-              // If dragging is not allowed or there is no data transfer then
>+          if (!this.allowDrag || !dragSession) {
>+              // If dragging is not allowed or there is no drag session
>               // we can't drop (i.e dropping a file on the calendar list).
>               return false;
>           }
> 
>-          let dragCalId = dataTransfer.getData("application/x-moz-calendarID");
>+          let dataTransfer = dragSession && dragSession.dataTransfer;
>+          let dragCalId = dataTransfer && dataTransfer.getData("application/x-moz-calendarID");

makes the patch working, but before testing everything would be better if you could check if it's right or something else is missing.
(Assignee)

Comment 20

4 years ago
Hey Decathlon, thanks for taking a look. I assume this is right, but I'll be happy to double check.
You need to log in before you can comment on or make changes to this bug.