476937 - Support optional default Start and Due dates for Tasks

Status: RESOLVED FIXED

(Calendar :: Tasks, enhancement)

Description

[Adam Katz]

Most of the tasks I create use the "Today Pane" in Thunderbird's email view.  I click the bottom area to create a new task and enter something in.  It comes with no information at all, specifically regarding dates.  This is a problem for me.

I would like the creation time noted by default.

I would like to be able to set a default due date along the lines of "5pm tomorrow" or "5pm this Friday" (or if we want to be clever, that latter one should be "5pm on the Friday following tomorrow at 5:01pm" so it doesn't create <25h deadlines).

This means that with a default due date of "5pm tomorrow", when I create a task by entering something in Today Pane's "Click Here to Add a New Task," it automatically creates the task with a due date of 5pm the next day.  If I were using the "New Task" dialog, a Due Date of the following day at 5:00pm would be entered and checked automatically.

Ditto for start date.  I'd like to be able to configure the system to default to a start date like "right now" or "9am tomorrow" or "9am Monday."

As it currently stands, sorting tasks by date is completely messed up since un-dated items are regarded as ~1969 rather than ~2071.  Even if we add an option for default due dates, this will be an issue for people who don't activate them.

Comment 1

[Philipp Kewisch [:Fallen][📱📆]]

This feature needs some UI that doesn't overload the prefs pane. I don't think the core code should be too complex on this, but some defaults for due and start dates (i.e task templates) could work out.

Please search/file a separate bug for the sorting issue.

Comment 3

[Matthew Mecca [:mmecca]]

If it's OK with you Philipp I'd like to reconsider this as a regular pref, I think it complements the default event length preference, and would be really useful with the quick add feature of the task lists, independent of the event dialog and template UI discussed in Bug 361213.

Comment 4

[Matthew Mecca [:mmecca]]

Attachment: Fix v1

Adds general prefs for default start and due dates. Options include: start/end of current day, next day, or next week as defined by the view settings; a fixed offset in minutes from the current time / start date; and a fixed offset in minutes from the start of the next hour.

Comment 5

[Richard Marti (:Paenglab)]

Comment on attachment 8333537 [details] [diff] [review]
Fix v1

Review of attachment 8333537 [details] [diff] [review]:
-----------------------------------------------------------------

For me this looks good. Only one thing, you are using in offsets minutes up to 999 (a little bit more than 16 hours). Should this work in such fine granularity for tasks? Wouldn't it be better to use hours instead. I'm not using Tasks but i think tasks are for longer periods and a shortest period of 1 hour should be enough.

Again, I'm not using tasks and this is only to think about it.

Comment 6

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8333537 [details] [diff] [review]
Fix v1

Review of attachment 8333537 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine to me, r=philipp

::: calendar/base/content/calendar-item-editing.js
@@ +20,5 @@
>  
> +function setDefaultItemValues(aItem, aCalendar, aStartDate, aEndDate, aInitialDate, aForceAllday) {
> +    function endOfDay(aDate) {
> +        let eod = aDate ? aDate.clone() : cal.now();
> +        eod.hour = getPrefSafe("calendar.view.dayendhour", 19);

cal.getPrefSafe

@@ +27,5 @@
> +        return eod;
> +    }
> +    function startOfDay(aDate) {
> +        let sod = aDate ? aDate.clone() : cal.now();
> +        sod.hour = getPrefSafe("calendar.view.daystarthour", 8);

cal.getPrefSafe

@@ +43,5 @@
> +            if (aStartDate.isDate && !aForceAllday) {
> +                // This is a special case where the date is specified, but the
> +                // time is not. To take care, we setup up the time to our
> +                // default event start time.
> +                aItem.startDate = getDefaultStartDate(aItem.startDate);

cal.getDefaultStartDate

@@ +53,5 @@
> +            }
> +        } else {
> +            // If no start date was passed, then default to the next full hour
> +            // of today, but with the date of the selected day
> +            aItem.startDate = getDefaultStartDate(initialDate);

cal.getDefaultStartDate

::: calendar/base/content/calendar-task-editing.js
@@ +129,2 @@
>              if (edit.value && edit.value.length > 0) {
> +                let item = createTodo();

cal.createTodo while you are here.

::: calendar/locales/en-US/chrome/calendar/preferences/general.dtd
@@ +24,5 @@
> +<!ENTITY pref.default_todo_tomorrow.label "Tomorrow">
> +<!ENTITY pref.default_todo_next_week.label "Next Week">
> +<!ENTITY pref.default_todo_offset_current.label "Offset From Current">
> +<!ENTITY pref.default_todo_offset_start.label "Offset From Start">
> +<!ENTITY pref.default_todo_offset_next_hour.label "Offset From Next Hour">

Would everyone understand what "Offset From Current" means? Maybe something like "Relative to Start", "Relative to Next Hour" for the lower two, I'm not quite sure what "current" is? The current time? In that case "Relative to current time".

Alternatively, if things are getting too long, maybe its possible to add a tooltip? Although I think a tooltip on a menuitem won't work.

Comment 7

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Fix - v2

Hi Matt, I've picked this one up again while trying to clear bug 731723 from my review queue. Here is a debitrotted patch with review comments applied. The only thing I haven't done is the strings, waiting for your feedback there. A few further comments:

* "Offset from Current" and "Offset from next hour" are very close to each other. Do we really need both options? If I had to pick, I'd stick with only "offset from current".

* Possibly the strings in the tasks dropdown should be put in a more general location. I could imagine a new feature (for a followup bug) that gives any "new task" button a dropdown where you can select when the task being created is due (End of Day, Tomorrow, ...)

* I sometimes with JS had python-style arguments. The argument list for setDefaultItemValues could potentially get very long. Maybe it would be better to have a parameter "args" which is an object and contains all the optional properties?

These are rather FYI:

* The defaults section looks a little unordered to me. As time permits I will solve this in another bug by moving the snooze prop to the alarms pane, putting the defaults into its own groupbox and creating a dropdown connected to a deck so that events/tasks defaults can be set separately.

* Arranging these controls would probably look better in a grid. I'll take care of this in above mentioned bug.

Comment 8

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Fix - v3

Heya. I've made a few changes to your patch to directly incorporate the deck. I'd appreciate a review on this before you push.

Comment 9

[Philipp Kewisch [:Fallen][📱📆]]

Oh, one thing I forgot, I think we should use "task" instead of "todo" in more places in that patch, i.e the preferences.

Comment 10

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8393804 [details] [diff] [review]
Fix - v3

Review of attachment 8393804 [details] [diff] [review]:
-----------------------------------------------------------------

::: calendar/base/content/preferences/general.xul
@@ +86,5 @@
> +            <menulist id="defaults-itemtype-menulist" oncommand="gCalendarGeneralPane.updateItemtypeDeck()">
> +                <menupopup id="defaults-itemtype-menupopup">
> +                    <menuitem id="defaults-itemtype-event"
> +                              label="&pref.events.label;"
> +                              value="default-event-grid"/>

s/default/defaults/

Sorry for all the mistakes :)

Comment 11

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Fix - v4

v4 addresses a few of my previous comments. I decided not to move the strings into a general location for the new tasks feature just yet. I also decided to stick with the current argument format for setItemDefaults for now.

Comment 12

[Matthew Mecca [:mmecca]]

I think Richard makes a good point in comment #5, while I think we should keep the minute resolution as the default I think we should also offer the option to specify hours and days as a unit for the offsets, similarly to how the default reminders pref works. I have a version of the patch locally that makes those changes, I'll merge those with your changes and put up a new patch.

Comment 13

[Matthew Mecca [:mmecca]]

Attachment: Fix v5

This adds a dropdown to select the unit for custom offsets. I added another column to the grid to get everything lined up correctly.

Comment 14

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8394547 [details] [diff] [review]
Fix v5

Review of attachment 8394547 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, one thing we should take care of:

::: calendar/base/content/calendar-ui-utils.js
@@ +437,1 @@
>      }

Shouldn't we be using PluralForm instead?

Comment 15

[Richard Marti (:Paenglab)]

Comment on attachment 8394547 [details] [diff] [review]
Fix v5

Review of attachment 8394547 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

Comment 16

[Matthew Mecca [:mmecca]]

(In reply to Philipp Kewisch [:Fallen] from comment #14)
> Comment on attachment 8394547 [details] [diff] [review]
> Fix v5
> 
> Review of attachment 8394547 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good, one thing we should take care of:
> 
> ::: calendar/base/content/calendar-ui-utils.js
> @@ +437,1 @@
> >      }
> 
> Shouldn't we be using PluralForm instead?

Yes, I also noticed there is an almost identical function at [1] that can be consolidated here as well, but I think that's better done in a separate bug.

[1] http://mxr.mozilla.org/comm-central/source/calendar/base/content/preferences/alarms.js#33

Comment 17

[Philipp Kewisch [:Fallen][📱📆]]

(In reply to Matthew Mecca [:mmecca] from comment #16)

> Yes, I also noticed there is an almost identical function at [1] that can be
> consolidated here as well, but I think that's better done in a separate bug.

Ok, lets do that. Can you file the bug when you push?

Comment 18

[Matthew Mecca [:mmecca]]

https://hg.mozilla.org/comm-central/rev/6ebe3fce935c