Add calendar name to event tooltip information

RESOLVED FIXED in 1.0b2

Status

--
trivial
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: sipaq, Assigned: chrissc.humbert)

Tracking

unspecified
1.0b2

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment, 5 obsolete attachments)

(Reporter)

Description

9 years ago
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.
(Reporter)

Updated

9 years ago
Whiteboard: [good first bug]
(Assignee)

Comment 1

9 years ago
Hello Simon,

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

Kind Regards
(Assignee)

Comment 3

9 years ago
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
(Assignee)

Comment 4

9 years ago
Created attachment 425432 [details]
the properties file to go with previous attachment
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

9 years ago
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

9 years ago
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
Attachment #425700 - Attachment is obsolete: true

Updated

9 years ago
Attachment #425701 - Attachment is patch: true
Attachment #425701 - Attachment mime type: application/octet-stream → text/plain

Comment 8

9 years ago
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.
Attachment #425701 - Flags: review?(philipp)
(Assignee)

Comment 9

9 years ago
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
Assignee: nobody → grahamperrin
Status: NEW → ASSIGNED
(Assignee)

Comment 10

9 years ago
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
(Assignee)

Comment 11

9 years ago
Created attachment 426492 [details] [diff] [review]
Patch Philippe if you can review
Attachment #425430 - Attachment is obsolete: true
Attachment #425432 - Attachment is obsolete: true
Attachment #426492 - Flags: review?(philipp)
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.
Attachment #426492 - Flags: review?(philipp) → review-
Attachment #425701 - Attachment is obsolete: true
Attachment #425701 - Flags: review?(philipp)
(Assignee)

Comment 13

9 years ago
Created attachment 427555 [details] [diff] [review]
Updated patch

Hope I have addressed your comments
Attachment #426492 - Attachment is obsolete: true
Attachment #427555 - Flags: review?(philipp)
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
Attachment #427555 - Flags: review?(philipp) → review+
Thanks for taking care!

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

-> FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0b2
(Assignee)

Comment 16

9 years ago
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
I originally wanted to take care of consolidation in bug 366680, I never had time to fix it though.
Assignee: grahamperrin → c.humbert

Comment 18

9 years ago
> 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.
(Reporter)

Comment 19

9 years ago
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

9 years ago
(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?
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.
You need to log in before you can comment on or make changes to this bug.