Closed Bug 377041 Opened 16 years ago Closed 12 years ago

Sorting should put tasks without date alway at the bottom


(Calendar :: Tasks, enhancement)

Not set


(Not tracked)



(Reporter: aryx, Assigned: julo)




(1 file, 1 obsolete file)

The most effective sort order for tasks by date is from past to future. Tasks without a specified date should be at the end of the list, even if the list is sorted from future to the past, because the user wants to see the tasks which need to be finished soon.
Dupe of Bug 372879? See Bug 372879 Comment #8 for information on current sort order, see Bug 307137 Comment #7 for reasons that lead to decision.
See "each successive calendar date" in comment 7 and Bug 307137 Comment #8 , it is not specified that now() should be used. Furthermore, tasks without date are expected to be done when the user has some time left, so they are voluntary tasks (compared to the "forced" tasks with dates which can't be seen if you have many voluntary tasks because the lists gets scrollable).
Whiteboard: [qa discussion needed]
Normally, we would not revisit decisions made in other bugs, but we are making an exception here since we are discussing re-designing the entire task UI.  Therefore, we are taking this discussion to the newsgroup please join the discussion there.  This bug can be used to implement part of whatever the outcome of that discussion is.
Whiteboard: [qa discussion needed]
Could this possibly be made a preference?
Attached patch Work in Progress Patch (obsolete) — Splinter Review
Hi All,

I am attaching the work in progress patch. It changes the order based on due date only (and due in since it's the same). The patch seems to work pretty well for me.

I have also added the time how much the task is overdue if it was not already completed. Please, have a look and provide some feedback.

Should the completed tasks be treated as without a due date? Since it does not make much sense anymore.

What other date do you think should be sorted in a same way? The ones without a date are always put at the end.

Appreciate any feedback.


(In reply to comment #7)
> Created an attachment (id=388103) [details]
> Work in Progress Patch

Thanks for looking into this, and writing a first patch. Maybe you want to have a look at for more details on the review process of patches for Calendar code.
Assignee: nobody → julo
OS: Windows XP → All
Hardware: x86 → All
I have done minor cleanups in the patch and have changed the order of all dates of the task to the new way (putting entries without the date specified always at the end) as requested by the bug.

I have also added the code that shows how long the task is overdue for overdue tasks. Please, provide feedback.
Attachment #388103 - Attachment is obsolete: true
Comment on attachment 388160 [details] [diff] [review]
[medium,l10n] Final Patch for Review

Adding a "Requestee" does not work for me.

Normally, I do not see any text field below "Requestee:" caption.

The text boxes come up only if I disable javascript which then does not add the request for review (I do not see it listed in "My Requests").

Philipp, could you please review this patch?

Attachment #388160 - Flags: review?(philipp)
Comment on attachment 388160 [details] [diff] [review]
[medium,l10n] Final Patch for Review

To request review select "?" in the review flag dropdown list and enter the requestee name.
Comment on attachment 388160 [details] [diff] [review]
[medium,l10n] Final Patch for Review

Due to the fact we are in string freeze I will postpone the review until after the beta.
Attachment #388160 - Attachment description: Final Patch for Review → [after beta] Final Patch for Review
Attachment #388160 - Attachment description: [after beta] Final Patch for Review → [medium,l10n] Final Patch for Review
Comment on attachment 388160 [details] [diff] [review]
[medium,l10n] Final Patch for Review

>diff -r 53f9c5558d5a calendar/base/content/calendar-task-tree.xml
>--- a/calendar/base/content/calendar-task-tree.xml	Sun Jul 12 17:36:54 2009 +0200
>+++ b/calendar/base/content/calendar-task-tree.xml	Sun Jul 12 23:32:11 2009 +0200
>@@ -319,8 +319,23 @@
>                       // Less than one hour
>                       return calGetString("calendar", "dueInLessThanOneHour");
>                   }
>+              } else if (!aTask.completedDate || !aTask.completedDate.isValid) {
>+                  // Overdue task
>+                  var minutes = Math.ceil( -dur.inSeconds / 60);
let vs var, here and elsewhere in this patch

>+    compareNativeTimeFilledAsc: function cal_compareNativeTimeFilledAsc(a, b) {
>+      if (a == b)
>+        return 0;
>+      if (a == -62168601600000000) // value for (0000/00/00 00:00:00)
>+        return 1;
>+      if (b == -62168601600000000) // value for (0000/00/00 00:00:00)
>+        return -1;
>+      return (a < b ? -1 : 1);
>+    },
>+    compareNativeTimeFilledDesc: function cal_compareNativeTimeFilledDesc(a, b) {
>+      if (a == b)
>+        return 0;
>+      if (a == -62168601600000000) // value for (0000/00/00 00:00:00)
>+        return 1;
>+      if (b == -62168601600000000) // value for (0000/00/00 00:00:00)
>+        return -1;
>+      return (a < b ? 1 : -1);
>+    },
I'd appreciate some comments on these functions. I initially thought that in compareNativeTimeFilledDesc, you could just return -compareNativeTimeFilledAsc(a,b), but if the time is 0000/00/00 00:00:00, then the same result is offered. Is this supposed to be that way?

>+            if (modifier == 1)
>+              return cal.compareNativeTimeFilledAsc(nsA, nsB);
>+            else
>+              return cal.compareNativeTimeFilledDesc(nsA, nsB);
use brackets {} with this if-else block.

r=philipp with that fixed, feel free to check in anytime.
Attachment #388160 - Flags: review?(philipp) → review+
Whiteboard: [needs updated patch]
I've decided to leave away the string change here, I don't think we should show more granularity here, otherwise we'd need a timer to keep it up to date.

I've also taken care of the review comments.

Pushed to comm-central <>
Backported to comm-1.9.2 <>

Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0b3
Whiteboard: [needs updated patch]
You need to log in before you can comment on or make changes to this bug.