Closed Bug 298352 Opened 15 years ago Closed 14 years ago

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

Categories

(Calendar :: Lightning Only, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: shaver, Assigned: jminta)

References

Details

Attachments

(4 files, 2 obsolete files)

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.
Attachment #187443 - Flags: first-review?(shaver)
Comment on attachment 187443 [details] [diff] [review]
start/end time setting, no pref yet

r=shaver
Attachment #187443 - Flags: first-review?(shaver) → first-review+
First part of this checked in; leaving bug open until I do the pref hookup in a
second patch.
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.
Priority: -- → P2
Target Milestone: --- → Lightning 0.8
Attached patch pref observer (obsolete) — Splinter Review
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.
Attachment #197338 - Flags: second-review?(shaver)
Attachment #197338 - Flags: first-review?(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()|?
Attachment #197338 - Flags: first-review?(dmose) → first-review-
Attached patch pref observer v2Splinter Review
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.
Attachment #197338 - Attachment is obsolete: true
Attachment #197640 - Flags: second-review?(shaver)
Attachment #197640 - Flags: first-review?(dmose)
Attachment #197338 - Flags: second-review?(shaver)
Comment on attachment 197640 [details] [diff] [review]
pref observer v2

r=dmose.  Thanks for the patch!
Attachment #197640 - Flags: first-review?(dmose) → first-review+
Assignee: vladimir → jminta
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
Attachment #197640 - Flags: second-review?(shaver)
No longer blocks: lightning-0.1
Target Milestone: Lightning 0.1 → ---
Blocks: 319701
Attached patch pref for start of week (obsolete) — Splinter Review
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.
Attachment #205443 - Flags: first-review?(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.
Attachment #205443 - Flags: first-review?(dmose) → first-review-
Attached image 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.)
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.
Attachment #205443 - Attachment is obsolete: true
Attachment #206418 - Flags: first-review?(dmose)
Comment on attachment 206418 [details] [diff] [review]
pref for start of week v2

r=dmose
Attachment #206418 - Flags: first-review?(dmose) → first-review+
patch checked in.  Closing bug since bug 319701 ought to finish this up.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
QA Contact: shaver → lightning
You need to log in before you can comment on or make changes to this bug.