Closed Bug 386479 Opened 13 years ago Closed 12 years ago

Switch to Calendar mode don't work properly using buttons, Calendar menu or keyboard shortcut

Categories

(Calendar :: Lightning Only, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: omarb.public, Assigned: giermann)

References

Details

Attachments

(2 files)

After update 0.5--->0.7 (lightning 0.7pre 2007063017) I noticed that toolbar buttons don't work very well with new switch mode


1) Install lightning 0.5
2) Add calendar buttons to toolbar: day view, month view and week view.
3) Install lightning 0.7pre 2007063017
4) While you are in mail view click month day button

Result:
1) The tab in lower left corner still shows mail mode


I think that after updating from 0.5 to 0.7 the calendar buttons should be removed from toolbar or selecting them should change view mode to calendar mode
Summary: Buttons added to toolbar with lightning 0.5 don't work very well with switch mode → Buttons added to toolbar with lightning 0.5 don't work very well with new switch mode
Same happens if the Calendar buttons are added to the menu bar (like in my setup). I think the switch to calendar view should also switch the sidebar to calendar mode.
Using the keyboard shortcut for calendar mode (Ctrl+3) has the same issue. The view is switched to calendar but the sidebar stays in mail mode. I think this is all the same issue and this bug should handle all the incidents.
OS: Windows XP → All
Hardware: PC → All
Summary: Buttons added to toolbar with lightning 0.5 don't work very well with new switch mode → Switch to Calendar mode don't work properly using buttons in mail toolbar or menubar
Version: unspecified → Trunk
Summary: Switch to Calendar mode don't work properly using buttons in mail toolbar or menubar → Switch to Calendar mode don't work properly using buttons in mail toolbar or menubar or keyboard shortcut
confirming this
with 0.5 i had adding a "this month" button in the mail toolbar

after upgrading to 0.7, if i click on this button, it switch to the calendar, but button in mode toolbar stay in "mail" and not calendar

--> notice that it is no more possible to add a "this month" button to the mail toolbar, so i remove it ... but yes, it should be done automatically
Same with the Calendar menu, e.g. Calendar -> Month.
Summary: Switch to Calendar mode don't work properly using buttons in mail toolbar or menubar or keyboard shortcut → Switch to Calendar mode don't work properly using buttons, Calendar menu or keyboard shortcut
Flags: blocking-calendar0.7?
This one calls ltnSwitch2Calendar, if not already in Calendar-Mode.
I also had to slightly modify ltnSwitch2Calendar() to set the current mode before calling ShowCalendarView() to avoid recursion errors.
Attachment #277683 - Flags: review?(michael.buettner)
Assignee: nobody → giermann
Status: NEW → ASSIGNED
Comment on attachment 277683 [details] [diff] [review]
Patch to assure Calendar-Mode in ShowCalendarView

>+    // assure CalendarMode first (we get called by this again)
>+    if (ltnCurrentMode() != 'calendar') {
>+      ltnSwitch2Calendar();
>+      return;
>+    }
These lines are indeed missing and are perfectly correct (although the indentation is wrong), but they are just half of the story. If we should find toolbar buttons configured in the mail toolbar that belong to the calendar mode they should be removed. These buttons all have an attribute called 'mode' that is set to 'calendar'. If we find such an element to be a child of the 'mail-bar2' it should be removed (delete the element from the tree and remove it from the current set of the mail toolbar). We shouldn't allow functions to be available in the wrong mode, although the above lines are indeed missing.
Attachment #277683 - Flags: review?(michael.buettner) → review-
Mickey,

thanks for your commendation! Indeed it should be discussed, whether the old (and no more available) buttons should be removed - but I would suggest to file a spin-off bug for that for two reasons:

1. The removal of buttons is more cosmetical while the subject of this bug is strict functional.

2. I don't know whether it is right to just remove the buttons and do not make them available for mail mode...
Why don't we offer functions for the toolbar, that are still accessible through the menu?

Of course it makes sense to have 2 different toolbars (as in 3-pane-view and message-view), because we probably don't need functions like 'Reply' or 'Forward' in mail-mode. There are really much more of both modes, that are not required or even not functional in the other mode.

But I would still like to have the following features in future versions:
Mail-mode: 'New Event', 'New Task', 'GoToToday'
Calendar-mode: 'Compose', 'Addressbook'

What do you think about that? Would it be possible to have 'dual-mode' buttons?
But as mentioned above, this should go into another spin-off bug.
So, please reconsider your review or tell me your opinion regarding the above.
Mickey, can you answer comment#7?
In addition to the patch Sven proposed, this patch takes care of allowing to use Ctrl+1 and Ctrl+3 to switch between modes. This patch is independent of Sven's patch though.

I'd like to have this patch for 0.7!
Attachment #280382 - Flags: review?(michael.buettner)
(In reply to comment #7)
> What do you think about that? Would it be possible to have 'dual-mode'
> buttons? But as mentioned above, this should go into another spin-off bug.
The initial idea was to have separate toolbars for the different modes. Commands are tied to one and only one mode, which suggests to only allow commands (toolbar buttons, that is) to be available in the mode that they apply to. Some commands make sense in more than one mode (e.g. delete), but have different semantics (in mail mode delete a mail, in calendar mode delete an event). Allowing buttons to be available in a mode that their commands doesn't belong to goes against the initial idea (and adds to the confusion in case of commands with semantics depending on the mode).

In essence this is the reason why allowing calendar-buttons in the mail-mode or visa versa should be avoided. At least that's my opinion, but as always I'll obey to Christian's final decision. Christian, any comment on this?
Comment on attachment 280382 [details] [diff] [review]
lightning mode switch keys

>+toMessengerWindow = function ltnToMessengerWindow() {
Please add a comment that you're overriding the function from mail\base\content\mailCore.js.

>+     window.open(uri, "_blank", ...
Please add
> var uri = "chrome://messenger/content/messenger.xul";
otherwise window.open() will throw since uri won't be defined.

>    gLastShownCalendarView = type;
>
>+    if (gCurrentMode != 'calendar') {
>+      ltnSwitch2Calendar();
>+      return;
>+    }
>
>    // If we got this call while a mail-view...
>    // hide all of the mail stuff so we have...
>    var calendarViewBox = ...
Please also add the above lines marked with '+' to messenger-overlay-sidebar.js into function showCalendarView(). This ensures that if a calendar button happens to be available in mail mode, we're at least switching back to calendar mode instead of ending up with showing a calendar view while still being in mail mode.

r=mickey with the above comments being addressed.
Attachment #280382 - Flags: review?(michael.buettner) → review+
(In reply to comment #10)
> (In reply to comment #7)
[...]
> 

I totally agree with Mickey.
Philipp, this needs to land before code freeze. Any reason why this still waits to be checked in?
Checked in on HEAD and MOZILLA_1_8_BRANCH

-> FIXED
Blocks: 390508
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
No longer depends on: 390508
Flags: blocking-calendar0.7? → blocking-calendar0.7+
Resolution: --- → FIXED
Target Milestone: --- → 0.7
(In reply to comment #10)
> In essence this is the reason why allowing calendar-buttons in the mail-mode or
> visa versa should be avoided.

Filed spin-off bug 396144, as proposed in comment #7.
Verified with 2007092104
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.