429687 - Follow-up features for the mode dependent today-pane

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[Berend Cornelius]

in bug 389150 comment #67 Christian made the following notes to improve the display of the today-pane, that is now available in all three modes:

He suggested to initially set the today-pane collapsed in all modes:
...
Yes, this is OK. OFF would be nicer of course, because it ruduces complexity.
Anyhow, I'll r+ the bug, but I'd like to see the following things changed in
spin-off bugs.

General:
Store Today Pane width Mode dependent

Calendar Mode:
- Show Tasks only by default.
- Hide the Mini Day

Task Mode:
- Show Events only by default.
- Hide the Mini Day

After talking to Christian we came up with some other ideas: 

- don't display the task pane of the today-pane in task-mode as it is just a redundant display. Consequently we also have to hide the buttons "< >" at the todaypane header that allow controlling the display of the subpanes( agenda-pane and task-pane".

Comment 1

[Bas van den Bosch]

I think we should show the calendar-list in task-mode (duplicate the relevant part of the todaypane in calendar-mode). 

Comment 2

[Sven Giermann]

@Bas:
I don't know what you mean... Could you attach a screenshot of your proposal?

@Christian:
Why should we always hide the (nice!) Mini Day in the agenda pane?
It is the only way to see the Agenda for a specific date...

But I would agree to force synchronization between the selected day in calendar view and the agenda pane.

(I recognize much more issues displaying the Today Pane in Calendar mode than I expected while working on bug 389150 ...)

Comment 3

[Bas van den Bosch]

@sven, please disregard my comment. I thought the calendar-selection didn't exist in taskmode but it does already. 

Comment 4

[Decathlon]

Attachment: Today pane less large

I hope this is the right place to do a request about Today pane.
With today pane in calendar and tasks views it's important to consider space optimization because with 1024x768 resolution monitor, events in month and week views with today pane are a bit narrow.
It would be nice if today pane could be less large (at its minimum width) and this should be possible modifying mini-day layout e.g. like shown in the attachment image.

To gain space I also set padding and margin of agenda and task list to zero but I see it's personal taste and anyway it could be done easily modifying userChrome.css file.

Thanks for your attention.

Comment 5

[Sven Giermann]

Comment on attachment 319164 [details]
Today pane less large

Reducing the font-size for 'weekdayNameContainer' to 14px instead of 18px would also bring the intended smaller width and furthermore save up some vertical space (if the background is cropped to 40x40px).

I would also reduce the size of '.dateValue' from 36px to 24px.

I also recognized Lightning to be very large on smaller display resolutions - maybe one should pay attention to that when designing new UIs...

Comment 6

[Berend Cornelius]

Attachment: [checked in] patch to remove tasktree in Taskmode

This is a first step to resolve the bug and adresses the last part of the bug description. This part is an important prerequisite to fix bug 429685 – "Toggling of 'Delete' - toolbarbutton in calendar-mode and task-mode does not work" as described in the comments of that bug.

Comment 7

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 323726 [details] [diff] [review]
[checked in] patch to remove tasktree in Taskmode

>Index: base/content/today-pane.js
>+      agendaPanel.setVisible((index != 1) && agendaPanel.isVisibleInMode(currentMode));
>+      todoPanel.setVisible((index != 2) && todoPanel.isVisibleInMode(currentMode));
Extra () not needed.

>+        <modehbox mode="mail,calendar" broadcaster="modeBroadcaster">
>+          <toolbarbutton id="folderview-cycler-left"  class="folderview-cycler"
>+                         oncommand="TodayPane.cyclePaneView(-1);"/>
>+          <toolbarbutton id="folderview-cycler-right" class="folderview-cycler"
>+                         oncommand="TodayPane.cyclePaneView(1);"/>
>+        </modehbox>
While you are at it, please wrap the class attribute into a new line.


r=philipp

Comment 8

[Berend Cornelius]

"patch to remove tasktree in Taskmode" checked in on trunk and MOZILLA_1_8_BRANCH

Comment 9

[Berend Cornelius]

Attachment: patch with minimonth in today-pane

In this patch I added the minimonth to the today-pane. It follows the ideas that I summarized in the description of this issue with the choice of a miniday or minimonth or none in mail mode and none of them in the other modes. I have not yet considered Sven's comment #2 but adding a miniday to the other modes should not be problem if it is desired. 
A remaining problem is the synchronisation of the selected day and the today-pane in calendar mode. This would mean that we possibly have several different displayed mode depenndent dates in the today-pane. This is not yet feasible with modeboxes and therefor the current patch does not yet offer any option to change the date in calendar and task mode.
For the same reason the mode-dependent width for the today-pane is not yet implemented either and I would like to do this in a follow-up bug.
Before I will ask for a review I would like to discuss the open ui issues on this issue. Any feedback is welcome. Further interesting ideas to this topic can be found under Bug 387420 – Today Pane: Show MiniMonth instead of MiniDay

Comment 11

[Berend Cornelius]

Attachment: minimonth-patch v. #2

I unbitrotted the patch. Yesterday I had a talk with Christian about the topic "minimnonth in Todaypane" and this is what came out of it:
- The minimonth should be displayed only together with the agenda-listbox because the tasklist is not bound to a date. If the today-pane only displays the tasklist we consequently don't display a minimonth or miniday. The disadvantage of this approach is that we then do not provide such a promininent date display in this case.
- We consequently also offer a minimonth/miniday in calendar/task mode to control the agenda Listbox, although this leads to a redundant display of two minimonths.
- The today-pane menu is located at the bottom of the View-menu:

Today Pane
           Show Today Pane F11
           ------------------
           * Display Miniday
             Display Minimonth

- The default display should be a "Display Miniday" instead of the "Display Minimonth".
- If the agenda-pane is not displayed the "Display Miniday/Display Minimonth" menuitems consequently have to be disabled.
- We stick to the name "Miniday" because it's somehow "sweet" compared to "Date-Navigator" or something like that.

We hope to get some more input from the community around this question. The patch attached does not yet reflect the described behaviour because we want to get some more feedback before I will continue with the implementation.

Comment 12

[Berend Cornelius]

Attachment: Screenshot of the current patch

Comment 13

[Christian Jansen]

Please find my comments below.

(In reply to comment #11)
> Created an attachment (id=325396) [details]
> minimonth-patch v. #2
> 
> I unbitrotted the patch. Yesterday I had a talk with Christian about the topic
> "minimnonth in Todaypane" and this is what came out of it:
> - The minimonth should be displayed only together with the agenda-listbox
> because the tasklist is not bound to a date.

The minimonth or the miniday.

[...]

> - The today-pane menu is located at the bottom of the View-menu:
> 
> Today Pane
>            Show Today Pane F11
>            ------------------
>            * Display Miniday
>              Display Minimonth

Please also add the "Modes" of the Pane to the sub-menu. I've also replaced Display by show.


Today Pane >  Show Today Pane   F11  *)
              ---------------------
              o  Mini Day
                 Mini Month
              ---------------------
              o  Tasks and Events
                 Events
                 Tasks 
              
*) Show Today Pane should context sensitive, if the Today Pane is visible "Show" should be replaced by "Hide".

[...]
> - We stick to the name "Miniday" because it's somehow "sweet" compared to
> "Date-Navigator" or something like that.

Maybe it is more correct write Miniday in two words. But this should be decided by a native English speaker.

Comment 14

[Pete Riley]

For people that show a minimonth in their Google/Linux/Vista sidebars, it would be nice if there was an option in Lightning to show neither the minimonth nor the miniday, but it's okay with me to not offer this feature as long as I can hide the MiniMonth/MiniDay via my userchrome.css.

FWIW, IMO in English it works equally well to use Mini Month & Mini Day, or MiniMonth & MiniDay.  Personally I prefer MiniMonth/MiniDay.

Comment 15

[Berend Cornelius]

Another comment to this topic from Jason Van Vliet:

...I notice that one of the goals for Lightning 0.9 is:

" Allow users to display the Mini Month instead of the Mini Day" [quote from wiki].

In my opinion, the goal might just as well be: "Allow users to display the Mini Month in addition to the Mini Day."

Let me explain why.  When I'm in Mail Mode, and I'm either reading emails or composing emails, there are numerous occasions when I just want to glance over at a Mini Month and see "What day is June 23rd?"  or "If Bob says next week Friday, what date is that?"

At present, in order to do this, I need to click the little "down triangle" in the Mini Day and make a Mini Month appear.  I don't mind doing one click to make that happen, but the problem is, as soon as I click back to my "Compose Email" window, the Mini Month disappears.  Or if I click anywhere in the Thunderbird window, such as scrolling around in "Display message" frame of the Thunderbird window, then the Mini Month also disappears.  It's this disappearing thing that greatly reduces the usability / functionality.

My suggestion: when a user clicks on the little "down triangle" make Mini Month stick in the Today Pane, and automatically slide the Event (& Task, too?) List down, so that the Mini Month does not cover the event list.  At this point, also change the little "down triangle" into a little "up triangle".  In this way, the Mini Month is pinned into position below the Mini Day.  The user can click around and do whatever he needs to do in the Thunderbird window (read emails, composing emails, etc, etc) and that Mini Month stays right there, just a glance away.

However, when the user does not want the Mini Month there anymore, because he wants to have more room for his event & task lists, then he clicks the little "up triangle" and the Mini Month disappears.  At this point the "up triangle" changes back to a "down triangle." The user can pull it down against into view whenever he needs it next...

Comment 16

[Sven Giermann]

I would also prefer Jason's suggestion compared to toggling via menu!

But I would still leave the toggling rather than displaying the Mini Month additionally - I would just leave the little triangle in the upper right corner to switch between Mini Month and Mini Day.

Comment 17

[Berend Cornelius]

Attachment: patch v. #3

I modified my patch basically following the ideas in comment #11 and #13. 

>Today Pane >  Show Today Pane   F11  *)
>              ---------------------
>              o  Mini Day
>                 Mini Month
>              ---------------------
>              o  Tasks and Events
>                 Events
>                 Tasks 
I did not provide the second part of this this menu: As the minimonth/miniday is a subpane of the agendapane we found that it is not correct to display this next to the menu of the Task and Event pane. Instead the "Mini" menu is disabled when the todaypane or the agendapane is not visible. 
We followed Pete's recommendation and offered a menu "None" to show neither the minimonth nor miniday. 
To display both minimonth and miniday did not seem to make sense to us because both panes offere a lot redundant information then.
I would like Andreas to test this patch before I ask for a review. This issue is String relevant so we hopefully can check in these beforehand.

Comment 18

[Berend Cornelius]

Attachment: patch v. #4

Andreas found a bug with the pesistent settings of new profiles. I fixed that and ask for review.
@philipp: Maybe it's time that i move the today-pane into an overlay. Today-Pane.xul is getting a bit crowded. I can handle this in a follow-up bug.

Comment 19

[Christian Jansen]

Comment on attachment 329399 [details] [diff] [review]
patch v. #4

ui=christian

One minor (but important bug). We should stick to the mini day as default.  After applying the patch the mini month shows up as default.

Comment 20

[Christian Jansen]

Comment on attachment 325396 [details] [diff] [review]
minimonth-patch v. #2

review done in minimonth-patch v. #4

Comment 21

[Berend Cornelius]

Attachment: patch v. #5

I slightly modified the last patch to meet Christian's demand in comment #20, so that now the miniday is the default pane to be displayed.

Comment 22

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 329399 [details] [diff] [review]
patch v. #4

>+    <command id="calendar_toggle_todaypane_command" 
>+             oncommand="document.getElementById('today-pane-panel').togglePane(event);TodayPane.setTodayHeader()"/>
Space after ;
>+ * applies an attribute value to the children of a DOM parent node
Kind of hard to parse. Maybe "Sets the given attribute on all children of the passed node". The "children of ... parent" part was a bit confusing.

>+ *
>+ * @param aParent  the parent node
>+ * @param aValue   the value of the radio control bound to be checked.
>+ * @return         true or false depending on if the a 'radio control' with the
>+ *                  given value could be checked.
>+ */
The parameter description is wrong here, mixed up with another function.

>+            if (compValue == aFilterValue) {
>+                if (aValue == "false") {
To conform to our other functions like setElementValue, you should use boolean false, and check === false here. Actually, you could probably use setElementValue here instead :-)

>       if (agendaIsVisible && todoIsVisible) {
>           var index = 0;
>       } else if (!agendaIsVisible && (todoIsVisible)) {
>           var index = 1;
>       } else if (agendaIsVisible && (!todoIsVisible)) {
>           var index = 2;
>+      } else { // agendaIsVisible == false && todoIsVisible == false:
>+          // In this case something must have gone wrong 
>+          // - probably in the previous session - and no pane is displayed.
>+          // We set a default by only displaying agenda-pane.
>+          agendaIsVisible = true;
>+          document.getElementById("agenda-panel").setVisible(agendaIsVisible);
>+          index = 2;
>       }
While you are here, could you put var index; before the if block and then set accordingly?

>+      return aMonthLabel.value = getDateFormatter().shortMonthName(aIndex)
>               + " " + aYear +  ", " + this.cwlabel + " " +  aCalWeek;
Is this ok for localizations?

>+      this.setMonthDescription(selMonthPanel, this.start.month,
Wrap parameter

>+      if (aDontUpdateMinimonth == null || aDontUpdateMinimonth != true) {
Since you are not doing full type checks here, wouldn't
if (!aDontUpdateMinimonth || !aDontUpdateMinimonth)
work? I think its easier to parse.


> window.addEventListener("load", loadTodayPane, false);
>\ No newline at end of file
Please add newlines at the ends of your files, I get a modified file every time I save files, even if I didn't change anything.


>+               <menuitem label="&calendar.context.modifyitem.label;"
>+                 accesskey="&calendar.context.modifyitem.accesskey;"
>+                 observes="agenda_edit_event_command"/>
>+               <menuitem label="&calendar.context.deleteitem.label;"
>+                 accesskey="&calendar.context.deleteitem.accesskey;"
>+                 observes="agenda_delete_event_command"/>
Align attributes

>+                <agenda-checkbox-richlist-item id="today-header-hidden" title="&calendar.today.button.label;" checked="true" persist="checked"/>
>+                <agenda-checkbox-richlist-item id="tomorrow-header-hidden" title="&calendar.tomorrow.button.label;" checked="false" persist="checked"/>
>+                <agenda-checkbox-richlist-item id="nextweek-header-hidden" title="&calendar.soon.button.label;" checked="false" persist="checked"/>
Wrap attributes


>+<!ENTITY todaypane.showTodayPane.label "Show Today Pane">
>+<!ENTITY todaypane.showTodayPane.accesskey "o">
>+
Remove last (empty) line.

Comment 23

[Berend Cornelius]

I addressd philipp's comments especially the new function "setAttributeToChildren" and checked in on trunk and MOZILLA_1_8_BRANCH
I think it is a good time to close the bug in order to give the qa a chance to verify it. For the outstanding feature "mode-dependent width of the today-pane I will set up a new issue.

-> issue is fixed

Comment 24

[Berend Cornelius]

I posted Bug 445304 – Today-pane: width should be set mode-dependently 
as a follow-up to this issue.

Comment 25

[Rimas Kudelis [use needinfo to ping]]

Is it intentional that we now have 

todaypane.showTodaypane.label "Show Today Pane"
todaypane.showTodaypane.accesskey "o"

and

todaypane.showTodayPane.label "Show Today Pane"
todaypane.showTodayPane.accesskey "o"

?

Comment 26

[Berend Cornelius]

Attachment: patch v. #6

@Rimas: nice spot.
The patch attached removes one resource

Comment 27

[Berend Cornelius]

checked in last patch on trunk and MOZILLA_1_8_BRANCH

Comment 28

[Decathlon]

There is a problem with tasks-only view in today pane: neither minimonth nor miniday are displayed, and three items "Show ..." in submenu "Today pane" become inactive.
Lightning 0.9pre 2008071518.

I also have a little request, maybe to file another bug. Since day selected in new minimonth changes events showed in agenda, could be useful show busy day as bold, like minimonth in calendar and task mode, to check easily events on a specific day (at least in mail mode).

Comment 29

[Berend Cornelius]

>There is a problem with tasks-only view in today pane: neither minimonth nor
>miniday are displayed, and three items "Show ..." in submenu "Today pane"
>become inactive.

The thing with the new miniday/minimonth is that we decided to make it part of the agenda-pane because the task pane is not bound to a date and we did not unnecessarily have two redundant minimonths in calendar mode and task mode (see comment #11 and comment #13.So when the agenda-pane is not visible in a specific mode we consequently have to disable the miniday/minimonth menuitems. Technical wise a cleaner solution would have been to add a further sub menu, so that it all looks like this:

Today Pane
    o Show Today Pane
    Agenda Pane
        o Show Miniday
        o Show Minimonth
        o None

But we preferred to keep the menu-hierarchy flat. 

>I also have a little request, maybe to file another bug. Since day selected in
>new minimonth changes events showed in agenda, could be useful show busy day as
>bold, like minimonth in calendar and task mode, to check easily events on a
>specific day (at least in mail mode).

Sure you can file a bug on this. This is a useful hint, but I looking at our bug queue I think it's rather not so probably somebody will fix it for the 0.9 release.

Comment 30

[Ton [:Tonnes]]

Attachment: [checked in] Mini Month->Mini-Month (calendar.dtd)

Even though I’m not sure whether "Mini-Month" and "Mini-Day" will remain as they are (as most comments in this bug refer to "Miniday" and "Minimonth", this takes care of the current dash-lacking "Mini Month" occurrence in calendar.dtd.

Comment 31

[Berend Cornelius]

In reply to comment #28:
>I also have a little request, maybe to file another bug. Since day selected in
>new minimonth changes events showed in agenda, could be useful show busy day as
>bold, like minimonth in calendar and task mode, to check easily events on a
>specific day (at least in mail mode).

I had a look at the code in the minimonthbusylistener. Currently this listener only refers to one minimonth namely the one in the left pane. To change the implementation to be more flexible and be able to handle several registered minimonths is basically not a big deal, but we have to accept a certain performance loss due to an additional getItems() call and I am not sure if we want this, especially as the minimonth is not the default pane within the today-pane. Making the "getItems()" call dependent on the visibility of the minimonth strikes me as a rather complicate solution at this point.

Comment 32

[Christian Jansen]

Comment on attachment 330378 [details] [diff] [review]
[checked in] Mini Month->Mini-Month (calendar.dtd)

ui=christian

Comment 33

[Berend Cornelius]

We are currently in a state after string freeze, so I currently don't want to check this in for the 0.9 release. I will wait until we open a new branch.

Comment 35

[Simon Paquet [:sipaq]]

Comment on attachment 330378 [details] [diff] [review]
[checked in] Mini Month->Mini-Month (calendar.dtd)

r=sipaq