Closed
Bug 543744
Opened 15 years ago
Closed 15 years ago
Add calendar name to event tooltip information
Categories
(Calendar :: Calendar Frontend, defect)
Calendar
Calendar Frontend
Tracking
(Not tracked)
RESOLVED
FIXED
1.0b2
People
(Reporter: sipaq, Assigned: chrissc.humbert)
Details
(Whiteboard: [good first bug])
Attachments
(1 file, 5 obsolete files)
1.66 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
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•15 years ago
|
Whiteboard: [good first bug]
Assignee | ||
Comment 1•15 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
Reporter | ||
Comment 2•15 years ago
|
||
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
Assignee | ||
Comment 3•15 years ago
|
||
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•15 years ago
|
||
Comment 5•15 years ago
|
||
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•15 years ago
|
||
[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•15 years ago
|
||
[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•15 years ago
|
Attachment #425701 -
Attachment is patch: true
Attachment #425701 -
Attachment mime type: application/octet-stream → text/plain
Comment 8•15 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•15 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
Updated•15 years ago
|
Assignee: nobody → grahamperrin
Status: NEW → ASSIGNED
Assignee | ||
Comment 10•15 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•15 years ago
|
||
Attachment #425430 -
Attachment is obsolete: true
Attachment #425432 -
Attachment is obsolete: true
Attachment #426492 -
Flags: review?(philipp)
Comment 12•15 years ago
|
||
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-
Updated•15 years ago
|
Attachment #425701 -
Attachment is obsolete: true
Attachment #425701 -
Flags: review?(philipp)
Assignee | ||
Comment 13•15 years ago
|
||
Hope I have addressed your comments
Attachment #426492 -
Attachment is obsolete: true
Attachment #427555 -
Flags: review?(philipp)
Comment 14•15 years ago
|
||
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+
Comment 15•15 years ago
|
||
Thanks for taking care!
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/24b02948c319>
-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0b2
Assignee | ||
Comment 16•15 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
Comment 17•15 years ago
|
||
I originally wanted to take care of consolidation in bug 366680, I never had time to fix it though.
Updated•15 years ago
|
Assignee: grahamperrin → c.humbert
Comment 18•15 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•15 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•15 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?
Comment 21•15 years ago
|
||
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.
Description
•