298352 - multiday view needs to have pref control for start/end of day, and start/end of week

Status: RESOLVED FIXED

(Calendar :: Lightning Only, defect, P2)

Description

[Mike Shaver (:shaver emeritus)]

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: start/end time setting, no pref yet

initial patch for this, just adds central control of the start/end time.  Need
to do some pref lookups in the view constructor and maybe attach a pref
observer for changes.

Comment 2

[Mike Shaver (:shaver emeritus)]

Comment on attachment 187443 [details] [diff] [review]
start/end time setting, no pref yet

r=shaver

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

First part of this checked in; leaving bug open until I do the pref hookup in a
second patch.

Comment 4

[Stuart Parmenter]

Sunbird supports:

calendar.event.defaultstarthour
calendar.event.defaultendhour

with a value of 0-23.  We have some stuff in the lightning prefs dialog for
these, although its kind of ugly.  Use these prefs and default to whatever we do
currently.

Comment 5

[Joey Minta]

Attachment: pref observer

After the last checkin, all that was left to do was simply watch the prefs and
update the multiday view if they changed.  This patch does that.  I have a
slight concern the ltnFinish() isn't getting called, but in principle, it
should be.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 197338 [details] [diff] [review]
pref observer

Looks good; just a few minor nits. 

In general, prefer coding an explicit |return| at the end of functions rather
than falling off the end.  It makes it clear to the reader that there was
intent to return and not an inadvertant error.

>Index: mozilla/calendar/lightning/content/messenger-overlay-sidebar.js
>===================================================================
>RCS file: /cvsroot/mozilla/calendar/lightning/content/messenger-overlay-sidebar.js,v
>retrieving revision 1.22
>diff -p -U8 -r1.22 messenger-overlay-sidebar.js
>--- mozilla/calendar/lightning/content/messenger-overlay-sidebar.js	23 Sep 2005 01:09:47 -0000	1.22
>+++ mozilla/calendar/lightning/content/messenger-overlay-sidebar.js	25 Sep 2005 15:57:50 -0000
>@@ -65,16 +65,30 @@ function ltnOnLoad(event)
> 
>     document.getElementById("ltnMinimonth").value = today;
> 
>     gMiniMonthLoading = false;
> 
>     // nuke the onload, or we get called every time there's
>     // any load that occurs
>     document.removeEventListener("load", ltnOnLoad, true);
>+
>+    // Start observing preferences
>+    var prefService = Components.classes["@mozilla.org/preferences-service;1"]
>+                            .getService(Components.interfaces.nsIPrefService);
>+    var rootPrefBranch = prefService.getBranch("");
>+    ltnPrefObserver.rootPrefBranch = rootPrefBranch;
>+    var pbi = rootPrefBranch.QueryInterface(
>+        Components.interfaces.nsIPrefBranch2);
>+    pbi.addObserver("calendar.", ltnPrefObserver, false);

Maybe just call this "pb" or "pb2" since the interface is no longer called
nsIPrefBranchInternal?

>+    ltnPrefObserver.observe(null, null, "");
>+
>+    // Add an unload function to the window so we don't leak the pref observer
>+    var win = document.getElementById("messengerWindow");
>+    win.setAttribute("onunload", "ltnFinish(); "+win.getAttribute("onunload"));

Does this really want to be |win.addEventListener()|?

Comment 7

[Joey Minta]

Attachment: pref observer v2

Version 2 fixes the comments from the previous review.	It also explicitly sets
the start/end hours each startup, since the views have no way to persist these
things on their own.

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 197640 [details] [diff] [review]
pref observer v2

r=dmose.  Thanks for the patch!

Comment 9

[Joey Minta]

Comment on attachment 197640 [details] [diff] [review]
pref observer v2

patch checked in, leaving bug open to sort out the start/end of week issues

Comment 10

[Joey Minta]

Attachment: pref for start of week

This patch wires in a preference for the day that the week should start on by default.  Setting it currently won't have any effect, but with the decorated week-view it works.  (This bug doesn't say anything about getting a default starting day of the week for the month view.  I'm guessing that should be handled separately?)

This patch also creates a default preferences file (like any good extension should have) that gets shipped in the lightning.xpi and is automatically placed in the right location during the xpinstall process.  I can break this part off into a separate bug if you prefer.

Comment 11

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 205443 [details] [diff] [review]
pref for start of week

Two requests: make the menulist you're adding be accessible, and post a screenshot.

Comment 12

[Joey Minta]

Attachment: screenshot

Screenshot of the relevant tab of the lightning preferences with this patch applied.  (Note that the code for this is taken directly from Sunbird, so testing is easily available there.)

Comment 13

[Joey Minta]

Attachment: pref for start of week v2

Same as before, but with align="center" on the row, so that the label lines up nicely.  As I mentioned in IRC, xul:menuitem is already accessible by default.

Comment 14

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 206418 [details] [diff] [review]
pref for start of week v2

r=dmose

Comment 15

[Joey Minta]

patch checked in.  Closing bug since bug 319701 ought to finish this up.