Closed Bug 330386 Opened 19 years ago Closed 19 years ago

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

Categories

(Calendar :: Lightning Only, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: keria, Assigned: jminta)

Details

Attachments

(2 files)

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...
I would tend to agree that the requested behavior seems more intuitive than the current behavior.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Hardware: PC → All
Attached patch persist the panel (deleted) — Splinter Review
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.
Assignee: nobody → jminta
Status: NEW → ASSIGNED
Attachment #216296 - Flags: first-review?(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?
Attachment #216296 - Flags: first-review?(dmose) → first-review-
Attached patch persist the panel v2 (deleted) — Splinter Review
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.
Attachment #218420 - Flags: first-review?(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
Attachment #218420 - Flags: first-review?(dmose) → first-review+
Checked in since this is Lightning only.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: