Last Comment Bug 405199 - First day of week is always Sunday in prefs window
: First day of week is always Sunday in prefs window
Status: VERIFIED FIXED
:
Product: Calendar
Classification: Client Software
Component: Preferences (show other bugs)
: unspecified
: All All
: -- minor (vote)
: 0.8
Assigned To: Christian Schmidt
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-11-24 04:45 PST by Christian Schmidt
Modified: 2007-12-01 12:12 PST (History)
1 user (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Dynamic sort order based on chosen first day of week (4.74 KB, patch)
2007-11-24 04:52 PST, Christian Schmidt
michael.buettner: review+
chris.j.bugzilla: ui‑review+
Details | Diff | Review

Description Christian Schmidt 2007-11-24 04:45:28 PST
I live in a country where Monday is considered the first day of the week. Even though I specify this in the preferences window (General / Start the week on : 
[ Monday ] ), the checkboxes immediately below this in the preferences window are always sorted with Sunday first (Workweek / Include these days in the workweek:
[ ] Sun  [X] Mon  [X] ... ).

This looks odd to me. I believe the checkbox order should respect the calendar.week.start setting.
Comment 1 Christian Schmidt 2007-11-24 04:52:12 PST
Created attachment 290003 [details] [diff] [review]
Dynamic sort order based on chosen first day of week

So far only tested in Lightning (I need to set up a Sunbird build environment).
Comment 2 Christian Schmidt 2007-11-24 07:35:26 PST
I have now tested in Sunbird as well.
Comment 3 Simon Paquet [:sipaq] 2007-11-24 09:08:01 PST
Comment on attachment 290003 [details] [diff] [review]
Dynamic sort order based on chosen first day of week

Matt is not active at the moment. Please request reviews from the active reviewers listed on http://wiki.mozilla.org/Calendar:Module_Ownership

In addition, this is an UI change and needs UI-review.
Comment 4 Christian Jansen 2007-11-29 02:45:18 PST
Comment on attachment 290003 [details] [diff] [review]
Dynamic sort order based on chosen first day of week

Thanks for the patch. Works fine. ui+
Comment 5 Michael Büttner (no reviews TFN) 2007-11-29 05:15:03 PST
Comment on attachment 290003 [details] [diff] [review]
Dynamic sort order based on chosen first day of week

>+    updateViewWorkDayCheckboxes: function (weekStart) {
>+        for (var i = Number(weekStart); i < Number(weekStart) + 7; i++) {
>+            var checkbox = document.getElementById("dayoff" + (i % 7));
>+            checkbox.parentNode.appendChild(checkbox);
>+        }
>+    }
This is indeed a nice and elegant solution.

I'd follow the surrounding style and avoid anonymous functions, for example:
>     updateViewWorkDayCheckboxes: function prefUpdateViewWorkDays(weekStart) {

...and pull the conversion out of the loop, such as:
>      weekStart = Number(weekStart);
>      for (var i = weekStart; i < weekStart + 7; i++) {

I'm going to change this before checking in. Thanks for the patch. r=mickey.
Comment 6 Michael Büttner (no reviews TFN) 2007-11-29 05:22:12 PST
patch checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED
Comment 7 Omar Bajraszewski 2007-12-01 12:12:07 PST
Verified with Lt 2007120104 and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.11pre) Gecko/20071201 Calendar/0.8pre

Note You need to log in before you can comment on or make changes to this bug.


Privacy Policy