543744 - Add calendar name to event tooltip information

Status: RESOLVED FIXED

Description

[Simon Paquet [:sipaq]]

Currently we only show the following data in the event tooltip

- Title
- Location
- Time and Date
- Status

We should think about adding the Calendar name to the mouseover tooltip to make it easier for users to identify to which calendar their event really belongs. See also bug 543742 for a different approach at improving this situation.

Comment 1

[Christophe Humbert]

Hello Simon,

I am ready to take my chance at iy if you can indicate where to look in the code

Kind Regards

Comment 2

[Simon Paquet [:sipaq]]

The relevant code should be here:
http://mxr.mozilla.org/comm-central/source/calendar/resources/content/mouseoverPreviews.js

Alternatively you can look here:
http://hg.mozilla.org/comm-central/file/default/calendar/resources/content/mouseoverPreviews.js

Comment 3

[Christophe Humbert]

Created attachment 425430 [details]
Add Calendar name in tooltip for event

Hello Simon

Here are my two files that I have changed
mouseoverPreviews.js
and
calendar.properties for en-us.

I have taken only care of the event for the moment. I will do others if that is ok for event.

Thanks

Comment 4

[Christophe Humbert]

Created attachment 425432 [details]
the properties file to go with previous attachment

Comment 5

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

Hello Christophe,

thank you very much for looking into this issue. I'd appreciate if you could create a patch for the changes you've made. Take a look at

https://developer.mozilla.org/en/Creating_a_patch

and specifically the first note about:

https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_diff_and_patch_files.3F

After you've done this, you can request review from a module owner. Go ahead and request review from me.

Comment 6

[Graham Perrin]

Created attachment 425700 [details] [diff] [review]
diff based on the attachment at comment 3

[macbookpro08:~/Desktop] gjp22% diff -u current new > org.mozilla.bugzilla-543744-3.diff
[macbookpro08:~/Desktop] gjp22% diff -u current new
--- current	2010-02-07 13:34:32.000000000 +0000
+++ new	2010-02-07 13:34:25.000000000 +0000
@@ -218,7 +218,12 @@
         event = getCurrentNextOrPreviousRecurrence(event);
     }
     boxAppendLabeledDateTimeInterval(vbox, "tooltipDate", event);
-
+	// First try to get calendar name appearing in tooltip
+	if (event.calendar.name && event.calendar.name != "NONE") {
+      var calendarNameString = event.calendar.name;
+      boxAppendLabeledText(vbox, "tooltipCalName", calendarNameString);
+    }
+	
     if (event.status && event.status != "NONE") {
       var statusString = getEventStatusString(event);
       boxAppendLabeledText(vbox, "tooltipStatus", statusString);

Comment 7

[Graham Perrin]

Created attachment 425701 [details] [diff] [review]
diff, with eight lines of context, based on the attachment at comment 3

[macbookpro08:~/Desktop] gjp22% diff -p -U 8 current new
--- current	2010-02-07 13:34:32.000000000 +0000
+++ new	2010-02-07 13:34:25.000000000 +0000
@@ -213,17 +213,22 @@ function getPreviewForEvent( aEvent) {
       boxAppendLabeledText(vbox, "tooltipLocation", location);
     }
     if (!(event.startDate && event.endDate)) {
         // Event may be recurrent event.   If no displayed instance specified,
         // use next instance, or previous instance if no next instance.
         event = getCurrentNextOrPreviousRecurrence(event);
     }
     boxAppendLabeledDateTimeInterval(vbox, "tooltipDate", event);
-
+	// First try to get calendar name appearing in tooltip
+	if (event.calendar.name && event.calendar.name != "NONE") {
+      var calendarNameString = event.calendar.name;
+      boxAppendLabeledText(vbox, "tooltipCalName", calendarNameString);
+    }
+	
     if (event.status && event.status != "NONE") {
       var statusString = getEventStatusString(event);
       boxAppendLabeledText(vbox, "tooltipStatus", statusString);
     }
 
     var description = event.getProperty("DESCRIPTION");
     if (description) {
       boxAppendBodySeparator(vbox);

[macbookpro08:~/Desktop] gjp22% diff -p -U 8 current new > org.mozilla.bugzilla-543744-3.diff

Comment 8

[Graham Perrin]

Comment on attachment 425701 [details] [diff] [review]
diff, with eight lines of context, based on the attachment at comment 3

My thanks, to:

 * Simon, for condensing my longer bug into this bug 543744
 * Christophe, for the speedy work
 * Philipp, for steering.

(In reply to comment #3)

> I have taken only care of the event for the moment. I will do others if that is
> ok for event.

It will be nice to have comparable richness in the tip for a *task*, but I guess that we should have a separate bug (request for enhancement) for that.

Comment 9

[Christophe Humbert]

Hello Graham,

Thanks for making the diffs
I have just done the event because I wanted to have the code ok before expanding it for task...It is more than five years I have made some code.

I will try to make diffs myself but on windows it seems not so easy. SoI am setuping a linux box (time to switch no?)

Kind Regards

Comment 10

[Christophe Humbert]

Hello,

Ok I have setup HG on Windows and I don't know how to generate a diff file but here is the copy of diff for the two files taking care of both task/todo and event

diff --git a/calendar/locales/en-US/chrome/calendar/calendar.properties b/calendar/locales/en-US/chrome/calendar/calendar.properties
--- a/calendar/locales/en-US/chrome/calendar/calendar.properties
+++ b/calendar/locales/en-US/chrome/calendar/calendar.properties
@@ -163,6 +163,8 @@
 #  Date: 7:00--8:00 Thu 9 Oct 2011
 #  Date: Thu 9 Oct 2000 -- Fri 10 Oct 2000
 tooltipDate     =Date:
+# event calendar name 
+tooltipCalName	=Calendar:
 # event status: tentative, confirmed, cancelled
 tooltipStatus   =Status:
 # task/todo fields
diff --git a/calendar/resources/content/mouseoverPreviews.js b/calendar/resources/content/mouseoverPreviews.js
--- a/calendar/resources/content/mouseoverPreviews.js
+++ b/calendar/resources/content/mouseoverPreviews.js
@@ -119,6 +119,11 @@
       hasHeader = true;
     }
 
+	// First try to get calendar name appearing in tooltip
+	if (toDoItem.calendar.name && toDoItem.calendar.name != "NONE") {
+      var calendarNameString = toDoItem.calendar.name;
+      boxAppendLabeledText(vbox, "tooltipCalName", calendarNameString);
+    }
     if (toDoItem.entryDate && toDoItem.entryDate.isValid)
     {
       boxAppendLabeledDateTime(vbox, "tooltipStart", toDoItem.entryDate);
@@ -218,6 +223,12 @@
         event = getCurrentNextOrPreviousRecurrence(event);
     }
     boxAppendLabeledDateTimeInterval(vbox, "tooltipDate", event);
+	
+	// First try to get calendar name appearing in tooltip
+	if (event.calendar.name && event.calendar.name != "NONE") {
+      var calendarNameString = event.calendar.name;
+      boxAppendLabeledText(vbox, "tooltipCalName", calendarNameString);
+    }
 
     if (event.status && event.status != "NONE") {
       var statusString = getEventStatusString(event);


Hope that's ok. I will improve 
Kind Regards

Comment 11

[Christophe Humbert]

Created attachment 426492 [details] [diff] [review]
Patch Philippe if you can review

Comment 12

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

Comment on attachment 426492 [details] [diff] [review]
Patch Philippe if you can review

>diff --git a/calendar/locales/en-US/chrome/calendar/calendar.properties
>b/calendar/locales/en-US/chrome/calendar/calendar.properties
>--- a/calendar/locales/en-US/chrome/calendar/calendar.properties
>+++ b/calendar/locales/en-US/chrome/calendar/calendar.properties
>@@ -163,6 +163,8 @@
> #  Date: 7:00--8:00 Thu 9 Oct 2011
> #  Date: Thu 9 Oct 2000 -- Fri 10 Oct 2000
> tooltipDate     =Date:
>+# event calendar name 
>+tooltipCalName    =Calendar:
I realize the format of the other entries are messed up too, but since I'm proposing other changes below, I'd appreciate if you could fix the spacing here. Also, I think we should use "Calendar Name:" to align with other places in the product (unifinder, calendar properties dialog,...)

Expected spacing:

tooltipCalName=Calendar Name:


>+    // First try to get calendar name appearing in tooltip
>+    if (toDoItem.calendar.name && toDoItem.calendar.name != "NONE") {
>+      var calendarNameString = toDoItem.calendar.name;
Also probably not the case in this file, but we use "let" instead of "var" for all changes, this is a new javascript feature that allows more specific scoping.

Why are you special casing when the calendar is named "NONE" ? Although unlikely, the user may actually name the calendar "NONE".

r- for now, but only to get a new patch with the above mentioned comments fixed. I'll happily r+ then.

Comment 13

[Christophe Humbert]

Created attachment 427555 [details] [diff] [review]
Updated patch

Hope I have addressed your comments

Comment 14

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

Comment on attachment 427555 [details] [diff] [review]
Updated patch

Looks good. Next time please save the file in unix file format, I had trouble applying it. r=philipp

Comment 15

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

Thanks for taking care!

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/24b02948c319>

-> FIXED

Comment 16

[Christophe Humbert]

Hello Philipp,

Next time I will setup Hg on my linux machine

Maybe we should open a followed-up bug to have this tooltip a bit cleaner as I have noticed there is some code duplicated between Task and events and maybe we should have a common base and extend the tooltip after for event/task

Kind Regards

Comment 17

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

I originally wanted to take care of consolidation in bug 366680, I never had time to fix it though.

Comment 18

[Graham Perrin]

> Target Milestone:	1.0b2

Please, can that milestone be clarified? 

My understanding of advice elsewhere is that there will be no 1.0 release, that 1.0b1 will be as close to final as things get for Sunbird.

Comment 19

[Simon Paquet [:sipaq]]

Graham,

this fix will be in Lightning 1.0 beta2. As there will be no further Sunbird release after 1.0 beta1, Sunbird users will not benefit from this.

Comment 20

[Graham Perrin]

(In reply to comment #19)

> there will be no further Sunbird release after 1.0 beta1

> Sunbird users will not benefit from this.

A) Should I open a separate bug for the fix to Sunbird? 
Bear in mind, this bug originated from Sunbird-oriented bug 543712. 

OR

B) Might the patch in this bug 543744 be in a nightly build of Sunbird?

OR

C) Must I build Sunbird myself?

Comment 21

[Stefan Sitter [:ssitter]]

You need to build Sunbird yourself. But I still hope that Bug 549286 will be fixed. In this case you would have (unsupported) nightly builds that contain all the fixes marked as Milestone 1.0b2.