330386 - Clicking on day on calendar pane brings always to month view instead of last view

Status: RESOLVED FIXED

(Calendar :: Lightning Only, defect)

Description

[Andras Keri]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1
Build Identifier: Lightning 0.1 RC2 (build: 2006031011)

When clicking on a day on the calendar pane, Ligthning brings always to month view, instead of bringing back to the last view I used.

Reproducible: Always

Steps to Reproduce:
1. Select a view (for example week view)
2. Go reading mails.
3. click on day in calendar pane.
4. Observe the month view.


Expected Results:  
Remember the last view that was used.

Quite an annoying as I usually use week view to organise myself...

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

I would tend to agree that the requested behavior seems more intuitive than the current behavior.

Comment 2

[Joey Minta]

Attachment: persist the panel

With this patch, we first check to see if a view has been shown before by asking for the selected day, which will throw if it hasn't been shown.  If we do get the selected day, then it's just a matter of stripping out the view type from the selectedPanel's id.

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 216296 [details] [diff] [review]
persist the panel

>Index: lightning/content/messenger-overlay-sidebar.js
>===================================================================
>RCS file: /cvsroot/mozilla/calendar/lightning/content/messenger-overlay-sidebar.js,v
>retrieving revision 1.39
>diff -p -U8 -r1.39 messenger-overlay-sidebar.js>--- lightning/content/messenger-overlay-sidebar.js	1 Mar 2006 04:42:00 -0000	1.39
>+++ lightning/content/messenger-overlay-sidebar.js	26 Mar 2006 08:38:03 -0000
>@@ -63,17 +63,28 @@ function ltnMinimonthPick(minimonth)
> 
>     var cdt = new CalDateTime();
>     cdt.jsDate = minimonth.value;
>     cdt = cdt.getInTimezone(calendarDefaultTimezone());
>     cdt.isDate = true;
> 
>     if (document.getElementById("displayDeck").selectedPanel != 
>         document.getElementById("calendar-view-box")) {
>-        showCalendarView('month');
>+        var view = currentView();
>+        try {
>+            view.selectedDay;

This depends on all views throwing an exception if the view hasnt been shown.  I think if we want to do this, we need to define calICalendarView.selectedDay somewhat more crisply.  In particular: we 
should document when exactly that method may or may not throw an exception in the IDL comments.  This may also depend on us deciding if we want to allow someone to write a view which could conceivably have no selected day (I imagine a unifinder-like view might be such a case).

>+        } catch(ex) {
>+            // Selected day will fail if the view hasn't been shown.
>+            showCalendarView('month');
>+            view.goToDay(cdt);
>+            return;
>+        }
>+        var viewID = view.getAttribute("id");
>+        viewID = viewID.substring(0, viewID.indexOf('-'));
>+        showCalendarView(viewID);

So this depends on a specific naming structure for the DOM id used for a view.  This might be ok, or we might want to solve this in some more generic way when we make views fully pluggable (e.g. category manager registration).  Id be ok with punting on this until we do the "fully-pluggable" thing.  Thoughts?

Comment 4

[Joey Minta]

Attachment: persist the panel v2

Version 2 relies on the fact that the displayCalendar will be null if the views haven't been shown.  I feel this is a much more intuitive assumption to make, and I've included a bit of documentation to support it.  I don't think we can *require* that someone setting the displayCalendar must immediately 'show' the view, in some sense of the word (since some views, ie the unifinder are always shown), but this seems like a reasonable approach, at least to me.

I also have no problem with relying on the id-naming structure.  Even for pluggable views, expecting someone to id their element as foo-view seems pretty reasonable.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 218420 [details] [diff] [review]
persist the panel v2

Looks like a reasonable strategy to me.  Just a couple of nits:

>-   * The displayCalendar of the calICalendarView that is embedded
>+   * The displayCalendar of the calICalendarView that is embedded.  This *must*
>+   * be set prior to calling goToDay the first time.

As long as you're here, s/calICalendarView that is embedded/embedded calICalendarView/ would be clearer.


>+        var viewID = view.getAttribute("id");
>+        viewID = viewID.substring(0, viewID.indexOf('-'));
>+        showCalendarView(viewID);

Can you add a comment before this clause explaining what exactly is being done here?

With those tweaks, r=dmose

Comment 6

[Joey Minta]

Checked in since this is Lightning only.