476227 - Drop shadow for multiday events has incorrect length

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[Stefan Sitter [:ssitter]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090130 Calendar/1.0pre (BuildID: 20090130133638)

Steps to Reproduce:
1. Start Sunbird with new profile.
2. Create a new event that spans more than one day.
3. Switch to month view and drag and drop the event to another day.

Actual Results:
The drop shadow covers only one day.

Expected Results:
The drop shadow covers as many days as the event lasts. If the event spans three days the drop shadow spans three days too.

Regression range: Works in Sunbird 0.9 (2008-09-17)
                  Fails in Sunbird 1.0pre (2008-12-03)
                  (drag'n'drop in earlier builds was broken due to Bug 432704)

Comment 1

[Stefan Sitter [:ssitter]]

This regression has been introduced after the 0.9 release. 
It should be decided if it must be fixed before the 1.0 release or not.

Comment 2

[Decathlon]

Attachment: [medium] patch

I'm trying to fix this. Not sure if everything is done in the right way in this patch.
I've drawn on code of Calendar 0.9 adapted to the new 'dragndropContainer' binding and I've added missing parts in particular for all day events in week view.
It seems it works fine for month/multiweek view and all day events in week view, I don't know if there are particular cases to test, though.

The patch also fixes bug 357112 (I moved here the patch's code I had attached there) and bug 502438 (at least for the final result after the drop, but it persists during drag session), moreover fixes another 0.9 version's bug that I can't find on Bugzilla: the shadow has incorrect length dragging multi-day events with starting position that straddles two view (current and following/previous view). There is bug 392194 but is only for day view and week view.

Unlike 0.9 version, with this patch the original event remains in the start position until the drop is completed. In my opinion this behavior is better because during the drag you always have the original position to be compared with the new one (the original item is selected while the shadows move in the view so it's always recognizable). 

I have some doubts, among others, in particular about "dragstart" handler and about the property "parentBox" that I moved into "calendar-editable-item", like I thought to do in bug 357112 comment #14, to make it available for all day events in week view. Let me know what's wrong.

Comment 3

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Just from testing (I haven't looked at code yet), I like the new behavior. One things that bugs me though, if you drag away and then back to the original dates, then both the shadow and the original events are visible.

Comment 4

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Maybe we can go even a step further and give the drop shadow some coloring. I could imagine maybe the original events get a transparent look, and the drop shadows are actual event boxes that contain the dragged event. I'm fine with doing this in a separate bug though. Bryan, do you have some ideas for this?

Comment 5

[Decathlon]

(In reply to comment #3)
> ... One
> things that bugs me though, if you drag away and then back to the original
> dates, then both the shadow and the original events are visible.

My initial thought was just to have both visible, shadow and original item, to easier repositioning the event if you change idea about the drag and you have a view with many events (maybe with the same calendar color) near the starting  position. I know there is also the undo function, but it's another thing.
Having both visible is also useful when you have a multiday event with many days and you have to move it some day before or after i.e. when you move near the original dates.

Leaving the original event selected is only the simplest way (for my poor coding skill) to get the behavior mentioned above, but every other idea is welcome.

Just to compare, in Google calendar the original event becomes transparent.

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 405306 [details] [diff] [review]
[medium] patch


>+      <method name="findDayBoxForDate">
>+        <parameter name="aDate"/>
>+        <body><![CDATA[
>+            let col = this.findColumnForDate(aDate);
>+            return (col? col.header : null);
>+        ]]></body>
>+      </method>
return (col && col.header); should work out here, that returns false instead of null and is a bit shorter.


Otherwise this looks good. About showing both event and dropshadow, I also think we should show both, just with one exception: If you start dragging the event somewhere else, then drag to the exact same day you started on, then you see the dropshadows and their event boxes adjacent to each other. I think we should only hide the shadow in this case.

I hope I explained a bit better, I'm not finding the right words :-)

r=philipp for this patch, I'm going to file a followup bug for the above mentioned issue.

Comment 7

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Followup bug 546943 filed.

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/33bd686be527>

-> FIXED

Comment 8

[Decathlon]

(In reply to comment #6)
 
> About showing both event and dropshadow, I also
> think we should show both, just with one exception: If you start dragging the
> event somewhere else, then drag to the exact same day you started on, then you
> see the dropshadows and their event boxes adjacent to each other. I think we
> should only hide the shadow in this case.
> 
> I hope I explained a bit better, I'm not finding the right words :-)

You explained well the first time too :-), but I didn't understand because I compared this case with drag session in day and multiday views where the shadow doesn't disappear when you drag back to the starting position.
Actually it's a different case because in day/multiday views the shadow *covers* the original item, so no adjacent element (original item and shadow) are shown during the drag session.

I will try to fix the followup bug too.