353696 - View/Number-of-weeks is not updated/synced with Option weeks-to-show

Status: VERIFIED FIXED

(Calendar :: Sunbird Only, defect)

Description

[Damian Szczepanik]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.8.0.7) Gecko/20060909 Firefox/1.5.0.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060921 Calendar/0.3a2+

When I change Numbers of week in main view in option still see old value

Reproducible: Always

Steps to Reproduce:
1. create new profile
2. switch to multiweek view (by default 4 weeks is displayed)
3. from menu view -> Numbers of week select 2 weeks
4. To go tools -> options -> views -> multiweek view
Actual Results:  
Default weeks to show is not updated - still 4 weeks

Expected Results:  
options should be updated as well

by default in Numbers of week no value is selected (when you create new profile)

more: when you change Numbers of week in option in main view you still can see old value so this is as well problem with refreshing (both side)

Comment 1

[Joey Minta]

This is by design, whether a good idea or not.  The preference is the default (on startup), toggling the option in the view is a temporary change.  WONTFIX?

Comment 2

[Damian Szczepanik]

like rotate? when you switch between views you always go to horizontal days and vertical hours.

I don't use rotate but it's strange. It's like starting up always with default toolbar even if you switched it before...

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

I suspect we want to fix this.  I found this behavior surprising and annoying as well.

Comment 4

[Joey Minta]

How about we kill the pref and just persist the menu-value?

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Sounds like the right thing to me.

Comment 6

[Damian Szczepanik]

(In reply to comment #4)
> How about we kill the pref and just persist the menu-value?
but in option you can set more than just number of weeks - also "Prevous weeks to show"
you can delete this option as well (what is not good idea) or migrate both to menu (which is even worse) or have one in menu and second in options which is not user friendly

I would drop it from menu, because generaly you set it once and don't change any more (I think so...). 

Before you kill any variant think about users - they should not be suprised that they lost famous option (from menu or options)

Eg me, I was confused because of bug 348765 because since then I can not see events with from date < 6:00h

Comment 7

[Joey Minta]

Looking for some UI-review goodness on killing the pref.  I think everyone's agreed that we don't need both the pref and the menu-item.  The question is only which should go and which should stay.

Comment 8

[Martin Schröder [:mschroeder]]

Christian, what is your opinion on this problem from an UI expert POV?

Comment 9

[gekacheka]

Additional problems:
1. Option pane allows one week, view menu does not.
2. On startup, current number of weeks is not selected in view menu.
3. If changed by view menu, then changed by option, wrong number of weeks is selected in view menu.

Easy to fix in view menu, but requires UI text change (l10n).
View menu/number-of-weeks is currently only in Sunbird, not Lightning.

Comment 10

[gekacheka]

Attachment: patch v1: set via view menu sets pref; add "1 week"; init view menu to pref

(patch -l -p 1 -i file.patch)

Changes:
calendar-decorated-multiweek-view.xml:
  changing weeksInView property now sets preference calendar.weeks.inview
calendar.dtd:
  Add calendar.menu.numberofweeks.1 "1 Week" so it can appear in view menu.
views.dtd:
  Change "Default weeks to show" to "Number of weeks to show" so
  it matches view menu (it is not a 'default' anymore).
calendar-menubar.inc:
  Add onpopupshowing code to initialize selected number of weeks.
  Add menuitem for 1 week.

With this patch, the view menu number-of-weeks and the option pane number-of-weeks now stay in sync after either is updated.

(Since this patch changes UI text it is not suitable for checking in during string freeze, so not asking for review yet.)

Comment 11

[Philipp Kewisch [:Fallen][📱📆]]

(In reply to comment #10)
> (Since this patch changes UI text it is not suitable for checking in during
> string freeze, so not asking for review yet.) 
Go ahead and ask for ui-r/r for your patches, even if they have strings. We'll eventually review and then use the checkin-needed/[checkin needed after 0.9] combination.

Comment 12

[Martin Schröder [:mschroeder]]

Comment on attachment 335269 [details] [diff] [review]
patch v1: set via view menu sets pref; add "1 week"; init view menu to pref

Requesting review for gekacheka's patch.

Comment 13

[Christian Jansen]

Comment on attachment 335269 [details] [diff] [review]
patch v1: set via view menu sets pref; add "1 week"; init view menu to pref

looks good for me. ui=christian

Comment 14

[Berend Cornelius]

Martin, I applied the patch and found that the "numberofweek.." resources have been shifted from calendar.dtd. Maybe you want to unbitrot the patch. I did not have a closer look if this is a dramatic change - most probably not.

Comment 15

[Berend Cornelius]

Comment on attachment 335269 [details] [diff] [review]
patch v1: set via view menu sets pref; add "1 week"; init view menu to pref

patch works as advertised and looks good.
Maybe it is an option to use the pref-"numberofWeeks" resources for the menulist, although they are used in a different context. I would be interested about Simon's opinion about this. On the fly you could also assign accesskeys for the menuitems ("1,2,3..").
As I said already please be aware that the numberofWeeks resources have been moved to Sunbird.dtd.
By the way is there any reason why we should not offer this menu-item in Lightning , too?. I think we should decide whether we like it or not, but in this case we should then either skip the feature completely or implement it for both applications.
Christian, do you have an opinion about it?
r=berend.

Comment 16

[Berend Cornelius]

Attachment: updated patch

checked in updated patch 
http://hg.mozilla.org/comm-central/rev/081c4c69d34f

where I added the Accesskeys

Comment 17

[Andreas Treumann]

Checked in sunbird 20081201 -> VERIFIED.