Closed Bug 378270 Opened 17 years ago Closed 17 years ago

Remove 'Rotate' button and move the rotate option to the views menu

Categories

(Calendar :: Calendar Frontend, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: sipaq, Assigned: Fallen)

Details

Attachments

(3 files, 4 obsolete files)

In the discussion on the Hamburg F2F meeting, it was decided, that the current 'Rotate' button is bad UI and should be removed. 

The option to rotate the view should be moved to the 'views' menu instead.
These are the relevant places in the code, that need to be changed:

- mozilla/calendar/locales/en-US/chrome/calendar/calendar.properties, line 208
- mozilla/calendar/base/content/calendar-multiday-view.xml, line 1783
- mozilla/calendar/base/content/calendar-multiday-view.xml, line 1817
- mozilla/calendar/base/content/calendar-multiday-view.xml, line 1818
Please state the reasons for your decisions. Otherwise, this bug is either invalid or wontfix.
The reasons as per bug 358692 comment 13:

- In general it makes sense to have a "Rotate" option, but we should not 
  display this feature top-level.
- The button consumes much space and is located at a very prominent place. 
  The calendar layout should be as clean as possible.
- Moving the Rotate button into the "View" or "Calendar" menu should be ok. 
  We could also offer a toolbar item, but this should be turned off by default.
- View-related settings (e.g. number of weeks in multiweeks, workweek days 
  only, etc.) should be in the views menu, not spread around the UI.
Milestone 0.7 Consolidation. Filter on PinkyANDBrain to get rid of bugspam.
Target Milestone: Lightning 0.7 → Sunbird 0.7
Attached patch Remove rotate button and put in menu (obsolete) — — Splinter Review
Mickey, how far are you on this? I hope its ok that I'm taking this one, but I only saw that the bug was assigned to you after creating the patch. Note it only adds the menu items correctly for Sunbird at the moment. I would like some input on how best to solve this for lightning, since the calendar/view menu is quite different there. Requesting ui review for that reason.
Assignee: michael.buettner → bugzilla
Status: NEW → ASSIGNED
Attachment #263654 - Flags: ui-review?(christian.jansen)
Attachment #263654 - Flags: review?(michael.buettner)
Attached image Screenshot of Sunbird with the new menu items (obsolete) —
Menu only enabled in multiday views.
Drive by comment:

In my opinion this two items doesn't justify a sub menu and bloat the UI. I think a simple menu item (e.g. Horizontal Orientation) with on/off state like the "Task in View" menu item below would be better. This also reduces the required mouse action to change the orientation.

I also think the user decision about the view orientation / menu item state should be persistent between view switches and application restarts (similar to the "Task in View" item again).
Just some more random thoughts about the wording that might be more descriptive:
"Arrange as Timescale" or "View as Timescale" or
"Arrange View by Date" vs. "Arrange View by Time" or
"Orient View by Date" vs. "Orient View by Time" or ...
I agree to Stefan. Having one entry with a checkbox should be enough.

...
-------------------
   Workweek days only
   Tasks in View
   Number of Weeks >
[ ] Rotate View
-------------------
...

Regarding the string:
From my point of view the string "Rotate View" would be somehow self-explaining.
I agree to Stefan. Having one entry with a checkbox should be enough.

...
-------------------
   Workweek days only
   Tasks in View
   Number of Weeks >
[ ] Rotate View
-------------------
...

Regarding the string:
From my point of view the string "Rotate View" would be somehow self-explaining.
The checkbox solution would also work better for Lightning
There we would have

Calendar -> View -> 

    Workweek days only
    Tasks in View
[ ] Rotate View
Attached patch Remove rotate button and put in menu (obsolete) — — Splinter Review
Ok, this should do it, both for sunbird and lightning.
Attachment #263654 - Attachment is obsolete: true
Attachment #263745 - Flags: ui-review?(christian.jansen)
Attachment #263745 - Flags: review?(michael.buettner)
Attachment #263654 - Flags: ui-review?(christian.jansen)
Attachment #263654 - Flags: review?(michael.buettner)
Screenshot, also includes lightning UI in various modes. Horizontal (Rotated) view is always enabled here.
Attachment #263655 - Attachment is obsolete: true
Looks good to me. Thanks for submitting the patch.
A small side note: It is really confusing to see a disabled menu item, with an enabled checkbox. But I guess that this needs to be fixed somewhere else.
Attachment #263745 - Flags: ui-review?(christian.jansen) → ui-review+
Yes, I was thinking the same thing, but disabling the checkbox manually will make problems when you go into month view and then quit the application. The persist property will keep the item from being checked after restart, and the multiday views will start in vertical mode every startup.
Sorry for jumping in here late, but why exists a calendar/view menu while there's a top-level view menu in thunderbird? Shouldn't we put all the calendar-view related options under view/calendar? Of course this has basically nothing to do with this patch, but it could be worth a spin-off bug? Christian, any comments?
Thanks for jumping in :-) You are right we have to define that. We should create one spin-off bug for the whole calendar menu restructuring. With this issue we move the rotate to entry to its final position.
Comment on attachment 263745 [details] [diff] [review]
Remove rotate button and put in menu

>   /**
>+   * The orientation of the view. Common values are "horizontal" and "vertical"
>+   */
>+  attribute AUTF8String orient;
Wouldn't it be better to make this a boolean attribute? A string can be set to anything and is generally less restrictive than a boolean switch. Or do we need to take any other layout options into account? Phillip, I'm not strongly against a string here, but is there any particular reason for not making it a boolean?

Furthermore, it feels strange to move options that are specific to a particular type of view to the most general interface. But as far as I see, we don't have any other option here. At least we'd have the theoretical option to construct a horizontal month view :-)

Besides what I mentioned above, it bothers me that the "rotate view" option gets disabled but the checkbox does not. Wouldn't it be much better to not disable the menu entry at all? Is it wrong to toggle the option while not being in the week view? At least, the "workweek days only" and "tasks in view" options also don't get disabled while not being in the calendar. Christian, any comments on this? Otherwise, I'd prefer to not disable the option at all.
I use 125% sized fonts in Windows, which makes the "Rotate" button too wide, so that the cells in week view next to the button are narrower than the columns below them, which looks ugly. Before it is decided what to do with the menu, a limit on the size of the button would be helpful. I don't want to file it as a new bug but don't know where to send the snapshot. 
(In reply to comment #18)
> (From update of attachment 263745 [details] [diff] [review])
> >   /**
> >+   * The orientation of the view. Common values are "horizontal" and "vertical"
> >+   */
> >+  attribute AUTF8String orient;
> Wouldn't it be better to make this a boolean attribute? A string can be set to
> anything and is generally less restrictive than a boolean switch. Or do we need
> to take any other layout options into account? Phillip, I'm not strongly
> against a string here, but is there any particular reason for not making it a
> boolean?
I changed to a boolean, at least in calIDecoratedView. It sets either horizontal or vertical in the underlying calendar view.

> 
> Furthermore, it feels strange to move options that are specific to a particular
> type of view to the most general interface. But as far as I see, we don't have
> any other option here. At least we'd have the theoretical option to construct a
> horizontal month view :-)
Yes, I didn't enjoy putting the rotated attribute in calIDecoratedView. But as you said, there is no way around this.

> Besides what I mentioned above, it bothers me that the "rotate view" option
> gets disabled but the checkbox does not. Wouldn't it be much better to not
> disable the menu entry at all? Is it wrong to toggle the option while not being
> in the week view? At least, the "workweek days only" and "tasks in view"
> options also don't get disabled while not being in the calendar. Christian, any
> comments on this? Otherwise, I'd prefer to not disable the option at all.

I think it would be wrong to have a menu option that doesn't do anything visible in a view. More specifically, If I as a user would select "rotate view" in month view and see that nothing happens, I might think this is a bug and file something like "month view doesn't rotate". But I'd also like a decision from Christian regarding this.


I'll upload a new patch as soon as this decision is made.
I'll could better live with the combination of a disabled label with an always enabled checkbox instead of an entry which is alway enabled. An always enabled menu item would confuse users if there no direct feedback.
Attached patch Remove rotate button and put in menu - v2 (obsolete) — — Splinter Review
Non-bitroted patch, fixes remaining issues.
Attachment #263745 - Attachment is obsolete: true
Attachment #272386 - Flags: review?(michael.buettner)
Attachment #263745 - Flags: review?(michael.buettner)
Philipp, you add the new menu strings in calendar.dtd, but do not remove the obsolete string in calendar.properties. 

Therefore please remove the string in line 213 of mozilla/calendar/locales/en-US/chrome/calendar/calendar.properties
Ah, yes. Nice catch Simon! This patch also removes the locale string.
Attachment #272386 - Attachment is obsolete: true
Attachment #274147 - Flags: review?(michael.buettner)
Attachment #272386 - Flags: review?(michael.buettner)
Philipp, you should use |this.viewElem.orient| instead of |document.getAnonymousElementByAttribute(this, "anonid", "view-element").orient|.
I'll keep that in mind when uploading patch after the review, thanks for the tipp!
Comment on attachment 274147 [details] [diff] [review]
Remove rotate button and put in menu - v3

The patch looks fine to me, r=mickey with Martin's previous comment being addressed.
Attachment #274147 - Flags: review?(michael.buettner) → review+
Checked in on HEAD and MOZILLA_1_8_BRANCH

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
I checked this with the latest Sunbird build (20070731), the task is fixed in sunbird.

But this doesn't work in lightning (build 2007073103) correct. If you choose 'Calendar->View->Rotate view" nothing happens. You have to restart thunderbird to get the rotated view.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch Fix regression — — Splinter Review
This is actually a regression from bug 386714. Lightning's updateOrientation() relied on the id of that deck.

Quick one line fix :)
Attachment #274834 - Flags: review?(michael.buettner)
Comment on attachment 274834 [details] [diff] [review]
Fix regression

>+    var deck = document.getElementById("view-deck");
Better use getViewDeck() here.

r=mickey with this change. thanks for taking care.
Attachment #274834 - Flags: review?(michael.buettner) → review+
Regression fix checked in on HEAD and MOZILLA_1_8_BRANCH

-> FIXED
Status: REOPENED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
Verified in lightning build (2007080603) -> task is fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: