Closed Bug 253396 Opened 20 years ago Closed 17 years ago

Task Mode: Full Task Window

Categories

(Calendar :: Calendar Frontend, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: bugzilla.mozilla.org, Assigned: michael.buettner)

References

Details

Attachments

(3 files, 4 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040626 Firefox/0.8
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040626 Firefox/0.8

Please add a task list view which reduces the functionality to simply a task
viewer. 

This would be like the day or month view.

This would make for an amazing application given that it already allowys for
multiuser remote ics editing, unlike any other application.

Reproducible: Always
Steps to Reproduce:
1.
2.
3.
OS: Windows XP → All
Hardware: PC → All
Attached image Mockup screenshot
I hacked up a quick idea of what this interface might look like. Please let me
know if you think this option would be useful to users. 

You can ignore the "Administer this site option" :-)
QA Contact: gurganbl → sunbird
Reassigning all automatically assigned bugs from Mostafa to nobody@m.o

Bugspam filter: TorontoMostafaMove
Assignee: mostafah → nobody
Wanted for 0.9 as per Roadmap. CCing UI people to come up with some view ideas.
Component: Sunbird Only → Calendar Views
QA Contact: sunbird → views
Summary: Task List View requested → Task Mode: Full Task Window
Target Milestone: --- → 0.9
See also existing proposal on <http://wiki.mozilla.org/Calendar:Task_View>
Target Milestone: 0.9 → 0.8
Assignee: nobody → michael.buettner
Flags: wanted-calendar0.8+
This is one of the main three goals for version 0.8 [1]. A release shouldn't
happen without this bug fixed. Requesting blocking stauts.

[1] http://wiki.mozilla.org/Calendar:Status_Meetings:2007-10-31
Flags: blocking-calendar0.8?
It looks like there is a broad disagreement with my point of view. Removing blocking request.
Flags: blocking-calendar0.8?
Status: NEW → ASSIGNED
Flags: blocking-calendar0.8+
We will not release 0.8 without this.
Attached patch patch v1 (obsolete) — Splinter Review
This patch introduces the task mode with all the necessary bits and pieces. In general, it can be seen as the first step towards the final result as it establishes the necessary infrastructure as well as the task list control and the task details view. Everything else will come as separate patches as this one is already well beyond what can be considered manageable... :-)

Some technical details regarding the patch: As the task view was most naturally modeled as a tree control it doesn't make sense to introduce another custom view tree control besides the one we already have, namely the unifinder todo. So I decided to make the existing code into a new binding, which ended up as a total re-write to be honest. This patch introduces a new binding 'calendar-task-tree' which functionally is equivalent to the previous unifinder todo. This binding then has been used for the unifinder todo and the task list.

Here's a rundown of the files the patch actually touches and why:

### base/content

calendar-common-sets.xul -> shared popup menues now live here
calendar-multiday-view.xml -> small fix

calendar-task-tree.css -> unifinder todo as binding
calendar-task-tree.js
calendar-task-tree.xml

calendar-task-view.js -> the new task view (handlers & logic)
calendar-task-view.xul

calendar-unifinder-todo.js -> old unifinder todo (handlers & logic)
calendar-unifinder-todo.xul

today-pane.js -> make the today pane disappear in task mode

### base/themes/*instripe

calendar-task-tree.css -> new styles for the calendar task tree binding
calendar-task-view.css -> styles for the task mode
calendar-unifinder-todo.css -> removed as it's no longer used

### base

jar.mn -> new files

### lightning/content

messenger-overlay-sidebar.js -> introduced task mode, adapted switching behavior
messenger-overlay-sidebar.xul
messenger-overlay-toolbar.js

### lightning

chrome.manifest -> overlay new files (i.e. task mode) into thunderbird

### locales/en-US/chrome/calendar

calendar.dtd -> new strings for task details view

### resources/content

calendar.js -> modifications due to task tree binding, calling finishCalendarToDoUnifinder() is no longer necessary.
Attachment #288979 - Flags: ui-review?(christian.jansen)
Attachment #288979 - Flags: review?(philipp)
(In reply to comment #11)
Will it be possible to display the Task Mode in Sunbird too?
In my opinion it could be added as a top level view next to day/week/month view.
(In reply to comment #12)
> Will it be possible to display the Task Mode in Sunbird too?
Sure. From a technically point of view this is certainly possible. From an UI perspective I would like to hear Christians opinion on that.
Comment on attachment 288979 [details] [diff] [review]
patch v1

First pass comments. Style nits are optional since much code was copied from other locations, but of course it would be nice to fix. I'll do a second pass when I have time otherwise maybe berend can continue the review?

>+                    oncommand="contextChangeProgress( event, 0 );"/>
>+                    oncommand="contextChangeProgress( event, 25 );"/>
>+                    oncommand="contextChangeProgress( event, 50 );"/>
>+                    oncommand="contextChangeProgress( event, 75 );"/>
>+                    oncommand="contextChangeProgress( event, 100 );"/>
spaces
>+                    oncommand="contextChangePriority( event, 0 );"/>
>+                    oncommand="contextChangePriority( event, 9 );"/>
>+                    oncommand="contextChangePriority( event, 5 );"/>
>+                    oncommand="contextChangePriority( event, 1 );"/>
spaces

>+++ mozilla/calendar/base/content/calendar-multiday-view.xml	Fri Nov 16 09:38:35 2007
>+          if (!startDate) {
>+              return null;
>+          }
return columns here instead, since an array is expected and columns is still empty

>+calendar-task-tree { 
>+  -moz-binding: url(chrome://calendar/content/calendar-task-tree.xml#calendar-task-tree);
>+}
I see why you created this file, but maybe we can rename calendar-view-bindings to calendar-bindings, so that a new file is not needed for just this one binding.

>+      if (document.getElementById("percent-"+task.percentComplete+"-menuitem")) {
>+         document.getElementById("percent-"+task.percentComplete+"-menuitem")
>+      if (document.getElementById("priority-"+task.priority+"-menuitem")) {
>+         document.getElementById("priority-"+task.priority+"-menuitem")
spaces

>+   if(numRanges == 0)
>+      return;
>+   if(numRanges == 0)
>+      return;

brackets
>+   for (var t=0; t<numRanges; t++) {
>+      for (var v=start.value; v<=end.value; v++) {

spaces
>+          // back reference to the binding
>+          mBinding: this,
>+          tree: null,
>+          treebox: null,
>+          selectedColumn: null,
>+          sortDirection: null,
mBinding, but everything else is not prefixed. All or none.

>+          findIndex: function tTV_findIndex(aItem) {
>+              for (var i=0; i<this.mBinding.mTaskArray.length; i++) {
>+                  if (this.mBinding.mTaskArray[i].hashId == aItem.hashId) {
>+                      return i;
>+                  }
>+              }
>+              return -1;
>+          },
This could get expensive if we have a lot of rows. Maybe we should create an object to map hashId's to row indexes?

>+              var aserv = Components.classes["@mozilla.org/atom-service;1"]
>+                  .getService(Components.interfaces.nsIAtomService);
>+                      aProps.AppendElement(aserv.getAtom("highpriority"));
>+                      aProps.AppendElement(aserv.getAtom("lowpriority"));
calUtils has getAtomFromService(). It might make sense to cache the atom service in calUtils, but thats not strictly needed. Go ahead and use the util func though.
>+                  for (var i=0; i<treecols.length; i++) {
style

>+          // The text for a given cell. If a column consists only of an
>+          // image, then the empty string is returned.
>+          getCellText: function mTV_getCellText(aRow, aCol) {
I'd just use return ""; for the default case, and maybe to make it clear let the image cols fall through.

>+          // Specifies if there is currently a sort on any column.
>+          // Used mostly by dragdrop to affect drop feedback.
>+          isSorted: function mTV_isSorted(aRow) {
>+              return false;
>+          },
We do sorting, right? A comment as to why this always returns false would be good.

>+                      if (this.tree.currentIndex > -1 ) {
style
>+          onCalendarAdded: function tTO_onCalendarAdded(aDeletedItem) {
>+          onCalendarRemoved: function tTO_onCalendarRemoved(aDeletedItem) {
>+              if (!this.mInBatch) {
>+                  this.mBinding.refresh();
>+              }
>+          },
I'm pretty sure you just copied this from somewhere, but we should have a spin-off bug to handle this more efficiently. I think especially in a tree view, it should be quite easy to filter out events by calendar or to just add events from the calendar.

>+          onDefaultCalendarChanged: function tTO_onDefaultCalendarChanged(aNewDefaultCalendar) {}
onDefaultCalendarChanged is not used anywhere. We might want to get rid of it. Spin off bug of course.

>+              (composite.ITEM_FILTER_TYPE_TODO|composite.ITEM_FILTER_COMPLETED_ALL);
>+          composite.getItems(filter,0,null,null,refreshListener);
style

>+          function compareString(a, b) {
>+            a = nullToEmpty(a);
>+            b = nullToEmpty(b);
>+            return ((a < b) ? -1 :
>+                    (a > b) ?  1 : 0);
>+          }
>+
>+          function nullToEmpty(value) {
>+            return value == null? "" : value;
>+          }
>+
>+          function compareNumber(a, b) {
>+            // Number converts a date to msecs since 1970, and converts a null to 0.
>+            a = Number(a); 
>+            b = Number(b);
>+            return ((a < b) ? -1 :      // avoid underflow problems of subtraction
>+                    (a > b) ?  1 : 0); 
>+          }
indent

>+          tree.boxObject.invalidateRange(0,this.mTaskArray.length-1);
style

>\ No newline at end of file
I'm not totally sure why diff is so verbose with this, but I usually don't see this in patches.


>+.calendar-task-tree {
>+  -moz-appearance:  none;
>+  background-color: -moz-Field;
>+  color: -moz-FieldText;
>+  border: none;
>+}
indent

>+.calendar-task-tree > treechildren::-moz-tree-image(calendar-task-tree-col-completed, completed),
>+.calendar-task-tree-col-completed {
>+   list-style-image      : url("chrome://calendar/skin/unifinder/checkbox_checked.png");
>+}
spaces

>+.todo-due-image-class {
>+   list-style-image      : url( "chrome://calendar/skin/unifinder/priority_header.png" )
>+}
spaces

>+
>+.todo-due-image-class[highpriority="true"] {
>+   list-style-image      : url( "chrome://calendar/skin/unifinder/priority_high.png" )
>+}
>+
>+.todo-due-image-class[lowpriority="true"] {
>+   list-style-image      : url( "chrome://calendar/skin/unifinder/priority_low.png" )
>+}
spaces

>+.task-details-name {
>+  font-weight: bold;
>+  text-align: right;
>+  background-color: transparent;
>+}
>+
>+.task-details-value {
>+  text-align: left;
>+  background-color: transparent;
>+}
indent
>+    if (gCurrentMode != 'mail') {
>+        if (id != "calendar-view-box" && id != "calendar-task-box") {
>+            ltnSwitch2Mail();
>+        }
one if()

> function ltnSwitch2Task() {
>   if (gCurrentMode != 'task') {
> ... 
>+
>+    window.setCursor("auto");
Why does this belong to task switching?
Comment on attachment 288979 [details] [diff] [review]
patch v1

Philipp, thanks for the first pass review. I'm going to address these issues with the next iteration of the patch. I find it a good idea to also let Berend have an additional review pass since it's a huge patch. Moving review request to berend to make him aware.
Attachment #288979 - Flags: review?(philipp) → review?(Berend.Cornelius)
Attached patch patch v2 (obsolete) — Splinter Review
Patch with all comments addressed. Besides the usual style changes I changed the following issues:

- calendar-decorated-views.css has been renamed to calendar-bindings.css which now also holds the binding statement for the task tree.
- the task tree now has an additional hash table for fast reverse lookup (item to index).
- adding/removing a calendar doesn't trigger refresh() buts handles the operation much more efficiently.
Attachment #288979 - Attachment is obsolete: true
Attachment #289315 - Flags: ui-review?(christian.jansen)
Attachment #289315 - Flags: review?(Berend.Cornelius)
Attachment #288979 - Flags: ui-review?(christian.jansen)
Attachment #288979 - Flags: review?(Berend.Cornelius)
Attached patch patch v3 (obsolete) — Splinter Review
Tested the previously submitted on Mac. Everything works fine and well, but I discovered that I forgot to add the task mode button to the default set of the mode toolbar. This updated version of the patch incorporates the necessary change.
Attachment #289315 - Attachment is obsolete: true
Attachment #289318 - Flags: ui-review?(christian.jansen)
Attachment #289318 - Flags: review?(Berend.Cornelius)
Attachment #289315 - Flags: ui-review?(christian.jansen)
Attachment #289315 - Flags: review?(Berend.Cornelius)
When I reviewed and tested the patch I noticed the following issues:
-The checked menuitems in the context menu should be radios.
-pending jobs should be canceled similar to the agenda pane.

>+                  createTodoWithDialog();

The todo-Dialog should be called with parameters, especially a due date that I think should be defined in the task pane.

-I noticed that sorting the titles does not work properly with capitalized and non capitalized tasks titles involved.


Besides these minor issues the patch works as advertised. I know that mickey is aware that it is not yet fully discussed in which way the task mode is going to be integrated in the system (see also the comment #12 and comment#13). Consequently the minimonth left to the task view does not yet correspond to the task mode that is currently implemented "next to the calendar mode". These issues should be worked in in follow-up bugs.
Attachment #289318 - Flags: review?(Berend.Cornelius) → review+
Attached patch patch v4 (obsolete) — Splinter Review
Updated patch that incorporates the review comments.

I also took the liberty to add the following bits and pieces:

- fixed typo in event handler, i.e. onOperationComplete()
- using calDateTime throughout the patch instead of mixing jsDate/calDateTime
- canceling previously submitted request operations if appropriate

As far as I am concerned this patch is the final version that is going to checked in as soon as the ui review has been granted.
Attachment #289318 - Attachment is obsolete: true
Attachment #289493 - Flags: ui-review?(christian.jansen)
Attachment #289493 - Flags: review+
Attachment #289318 - Flags: ui-review?(christian.jansen)
Comment on attachment 289493 [details] [diff] [review]
patch v4

Thanks for the patch, it works really fine. I'll give this a ui+. It would be great if you could change the following within that patch.

Mode Toolbar
	* Remove space between Calendar Icon & Task Icon in Mode Toolbar
	* Rename "Task" to "Tasks
	
	
Task List
	* Display by Default the following Categories
	  * Done
	  * Priority
	  * Title
	  * Start
	  * Due
	* Remove the horizontal dotted line and replace them by vertical dotted lines (see attachment)

Summary Pane (Mac only)
	* If possible, reuse the styles of the Message Pane header box
Attachment #289493 - Flags: ui-review?(christian.jansen) → ui-review+
(In reply to comment #20)
>   * Remove the horizontal dotted line and replace them by vertical dotted
> lines (see attachment)

While the vertical dotted lines look nice, I don't think that they're consistent with Thunderbird's theme (e.g. see the message-list pane (Sender/Subject/etc)).  I think that the vertical lines are not necessary because the text will form a natural left boundary in each column because the text is left-aligned.

If the vertical lines remain, please give us a way to hide them in UserChrome.css.

I agree that horizontal lines would also be undesirable.  I also don't like the horizontal striping of background color in the rows, but at least that is consistent with Thunderbird's theme (FYI I quickly hid that striping when it become a standard part of Thunderbird's theme).
p.s. The vertical lines would also not be consistent with the unifinder.
We can change this ;-)
Attached patch patch v5Splinter Review
Final iteration of the patch with all comments addressed. Carrying r+ and ui-r+ forward.

> Remove space between Calendar Icon & Task Icon in Mode Toolbar
The additional space has probably been introduced when your toolbar has been customized. This patch just adds another button to the toolbar, try the 'restore default set' option.

> Rename "Task" to "Tasks
Done.

> Display by Default the following Categories...
Done. The new 'calendar-task-tree' binding now has a configurable set of default columns.

> Remove the horizontal dotted line.
The horizontal striped background appears only on Mac, therefore I decided to have a different set of rules for Mac and other platforms. I removed the dotted horizontal line on Mac but left it in on the other platforms. I added the vertical lines between columns on all platforms.

Pete, if you don't like those lines, you can remove them by adding these rules to you userchrome.css:

/* horizontal line */
#calendar-task-tree > .calendar-task-tree > treechildren::-moz-tree-row {
    border-bottom: none;
}

/* vertical line */
#calendar-task-tree > .calendar-task-tree > treechildren::-moz-tree-cell {
    border-bottom: none;
}

or combine them:

#calendar-task-tree > .calendar-task-tree > treechildren::-moz-tree-row,
#calendar-task-tree > .calendar-task-tree > treechildren::-moz-tree-cell {
    border-bottom: none;
}

'calendar-task-tree' is the id of the tree control in the new task view. You can use similar rules applied to 'unifinder-todo-tree' for the task list in the today pane.
Attachment #289493 - Attachment is obsolete: true
Attachment #289800 - Flags: ui-review+
Attachment #289800 - Flags: review+
patch checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment on attachment 289800 [details] [diff] [review]
patch v5

>-<!ENTITY lightning.toolbar.task.label         "Task">
>+<!ENTITY lightning.toolbar.task.label         "Tasks">

Please rename the entity to make the change visible to the l10n teams.
Blocks: 388433
When I look at concept section in http://wiki.mozilla.org/Calendar:Task_Mode I just wonder what happened with other features like "click here to add a new task" or "sort tasks" pane. Are you planning to add them? And  the preview pane looks very simple- just title and description field. What about other details like 'start date', 'due date', 'timezone', etc.?

Verified with lightning 2007112211 ant Tb 2.0.0.9
Status: RESOLVED → VERIFIED
Blocks: 405033
(In reply to comment #28)
> When I look at concept section in http://wiki.mozilla.org/Calendar:Task_Mode I
> just wonder what happened with other features like "click here to add a new
> task" or "sort tasks" pane. Are you planning to add them?
All these features will be addressed in separate patches. The reason was simply that this patch already grew extremely large (> 150 kb). That's why I made the decision to land it now and provide the rest in other patches which will be naturally smaller as the groundwork has been laid with this one.
Depends on: 405034
(In reply to comment #25)
> Pete, if you don't like those lines, you can remove them by adding these rules
> to you userchrome.css:

Thanks for the instructions, Michael, and for Tasks Mode.  :)
This checkin regressed Bug 405034 and Bug 405499.
Depends on: 405499
Depends on: 405502
Depends on: 405111
You need to log in before you can comment on or make changes to this bug.