316791 - multiday-view events remain collapsed when view is switched to

Status: RESOLVED FIXED

(Calendar :: Lightning Only, defect)

Description

[Dan Mosedale (:dmosedale, :dmose)]

Reasonably often, but not always, when switching to the week view, the events all remain collapsed at the top of the screen.  They do seem to be colored correctly, indicating that the CSS is being applied.  The workaround is to simply select the same view again.

Comment 1

[Joey Minta]

Attachment: fix collapse calls

This bug is caused directly by a race between LtnObserveDisplayDeckChange reaching the uncollapse call and showCalendar reaching the setDateRange call, which explains the "Reasonably often, but not always" quote.  The multiday-view hardcodes the heights of everything in the view, based on the height of its container box when it is told to setDateRange, meaning that if setDateRange wins the race, all the heights are the minimum 1px.

In trying to fix this bug, though, I realized that the current collapse/uncollapse structure was getting a bit weird/difficult to follow, so this includes some simplification on that point as well.  The new way things work is:

When switching FROM mailview TO calendarview, we collapse inside showCalendar().  This is the central function which dispatches all view switching, etc, so it should never be missed regardless of whether the view is shown via clicking the minimonth, clicking a calendar, or one of the menu-options.

When FROM calendarview TO mailview, we have no choice except to wait for the actual deck-switching event (although bug 316790 might give us an alternative in some cases).  Currently, TB doesn't worry about height for anything in its boxes, so there is no race.  When we do get this event, we uncollapse all of the mail stuff we collapsed previously, and collapse the calendar box.

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 203560 [details] [diff] [review]
fix collapse calls

This looks great; thanks!

>+
>+function collapseElement(element) {
>+    element.style.visibility = "collapse";
>+}
>+
>+function uncollapseElement(element) {
>+    element.style.visibility = "";
>+}

Just for future reference, can you add a comment here explaining that this seems to work reliably and element.collapsed doesn't, and that's why it's written this way?

>+    deck.selectedPanel = document.getElementById("calendar-view-box");
>+
>+    switchView('week');
> }

In some ideal world, we'd switch back to whatever view they happened to be looking at last.  If that's easy to do (ie if |switchView(currentView())| would always work here), I'd suggest doing that not.  

Either way, r+ with or without that change.

Comment 3

[Joey Minta]

patch checked in

(In reply to comment #2)
> In some ideal world, we'd switch back to whatever view they happened to be
> looking at last.  If that's easy to do (ie if |switchView(currentView())| would

currentView() returns a panel of the deck, which makes 'day' and 'week' indistinguishable without testing more specific features of the view.  For simplicity's sake, I did not add this change.