457536 - [Today Pane] Wrong time shown on events spanning multiple days

Status: RESOLVED FIXED

(Calendar :: Lightning Only, defect)

Description

[Fabi]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.3) Gecko/2008092417 Firefox/3.0.3
Build Identifier: Lightning 2008092720

Events with a start and end date spanning multiple days is badly displayed in the today pane. The start time is displayed on the following day although the event doesn't start that day. A similar problem was already fixed in the month view while including markers for multiple day events (bug 199732) 

Reproducible: Always

Steps to Reproduce:
1. Create an event from "today" 23:00 until "tomorrow" 02:00
2. Switch to the today pane
Actual Results:  
The event is displayed under "Today" starting at 23:00 and under "Tomorrow" starting at 23:00

Expected Results:  
The event should be displayed under "Today" starting at 23:00 and ending "Tomorrow" at 02:00

Comment 1

[Martin Schröder [:mschroeder]]

Confirmed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081122 Lightning/1.0pre Shredder/3.0b1pre.

Comment 5

[Decathlon]

Attachment: screenshot

What about something as shown in the screenshot?
With three icons that represent the "start", "end" and the "intermediate" date for the spanning multiple days events, we can identify the state of the event for the "today", "tomorrow" and "soon" section in the agenda.

The only problem would be the "soon" section where an event is displayed only one time although it takes more days, so I've thought to show:
the "start" icon if the event starts in the "soon" days;
the "end" icon if the event ends in the "soon" days;
the "continue" icon if the event starts before and ends after the "soon" days.
Every date showed is the start or end datec, and no date is showed for the days between start and end.

The screenshot is the result of a work in progress patch. It shows a comparison with the actual state for some multiday events.
What's your opinion?

Comment 6

[Decathlon]

Attachment: patch -v1

I've completed a first attempt of patch, so I ask for review. Should I ask for UI review too?

This patch also includes all-day events and events that end at midnight. The next screenshot shows some cases not included in the previous one.
Instead of using a particular icon for the "midnight" case (as it has been done for the month-view), I've inserted the string "midnight" for the time part of the label, not sure if a string "24:00" or "12:00 PM" should be used instead.

Apart from the icons, that could be changed with better/different ones (the icons in the following screenshot are a bit better), this patch should cover every lack of the today pane about spanning multiple days events. The only problem could be the lack of the date for the all-day events that are in the "soon" section, but this is bug 417222.

Comment 7

[Decathlon]

Attachment: Screenshot: all-day events and events that end at midnight

Comment 8

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

Comment on attachment 438753 [details]
Screenshot: all-day events and events that end at midnight

Yes, this requires ui-review.

Where did you get those icons? Did you make them yourself? If these were taken from somewhere, we need to make sure they have a compatible license.

Comment 9

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

Comment on attachment 438751 [details] [diff] [review]
patch -v1

Codewise this looks fine, waiting for ui-r now.

Comment 10

[Decathlon]

(In reply to comment #8)

> Where did you get those icons? Did you make them yourself? If these were taken
> from somewhere, we need to make sure they have a compatible license.

I made them by myself, after a little fight with GIMP, because I didn't find such icons with two contrasting colors (to make them visible with both dark and clear background), but I wouldn't know whether they look like others existing icons. It's possible, they are very simple.
However, would be nice if someone could change them with something better, though, with a 10x10 pixels room, there isn't a big space for creativity.

Comment 11

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 438753 [details]
Screenshot: all-day events and events that end at midnight

The overall change looks good.

Perhaps we can have Andreas take a look at the icons.  I appreciate the icon work (it's not that easy!) but they still look a little jagged.  New icons could be a followup patch.

Comment 12

[Bryan Clark (DevTools PM) [:clarkbw]]

(In reply to comment #11)
> Perhaps we can have Andreas take a look at the icons.  I appreciate the icon
> work (it's not that easy!) but they still look a little jagged.  New icons
> could be a followup patch.

Andreas ^^
https://bugzilla.mozilla.org/attachment.cgi?id=438753

Comment 13

[Andreas Nilsson (:andreasn)]

Sure!

Comment 14

[Martin Schröder [:mschroeder]]

We should check the patch in asap, and postpone the icon update! Philipp?

Comment 15

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

Pushed to comm-1.9.2 <http://hg.mozilla.org/releases/comm-1.9.2/rev/9f6235ad561a>
-> FIXED

Comment 16

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

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/282e94e84189>
-> FIXED

Comment 17

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

I'd appreciate if someone could file the bug for the icon update.

Comment 18

[Decathlon]

(In reply to comment #17)
> I'd appreciate if someone could file the bug for the icon update.

Filed bug 565281.

Comment 19

[Decathlon]

There is a problem for the "intermediate" occurrence of events that start today and span three days. I'm going to attach a patch. Sorry :(

Comment 20

[Decathlon]

Attachment: fix an error

When an event starts today and has a duration of three days, the 2nd occurrence, showed on "Tomorrow" section of the agenda, has the icon "end" instead of the icon "continue".
This patch changes the end date of the periods (Today, Tomorrow, Soon) in order to not overlap the end date of a period with the start date of the next one.

Comment 21

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

Comment on attachment 445472 [details] [diff] [review]
fix an error

r=philipp

Comment 22

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

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/3ecdde3bca04>
-> FIXED

Comment 23

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

Pushed to comm-1.9.2 <http://hg.mozilla.org/releases/comm-1.9.2/rev/cf0eb57fd5ca>
-> FIXED