Add UI for assigning a duration to a task



9 years ago
a year ago


(Reporter: martinschroeder, Unassigned)




(3 attachments)

Follow-up from bug 328216: Implement the UI to allow to set a duration for tasks.

Comment 1

9 years ago
Created attachment 399692 [details] [diff] [review]
[medium,test] UI implementation for Duration

here is a UI proposal for setting a duration on a task. The UI will not allow setting both, a due date and a duration at the same time. 

However, one observed bug: The getter-method for a due date will consult a duration field in ICS file in case it cannot find a DUE DATE (see calTodo.js). Thus, storing a duration and a start date will result in a start date and a due date being displayed the next time the task-dialog is opened. As this does not represent the ICS file, the dialog recognizes a change and will ask the user to save the data even though no change has been made by the user.

Also, setting duration does not work for other formats than ICS. This needs to be implemented in the backend as well!

Happy to hear comments on the posted patch!
Hey Tom, thanks for the patch. Could you provide a screenshot for easier ui-review? You can request ui-review from

Comment 3

9 years ago
Created attachment 399705 [details]
screenshot, duration patch

here is a screenshot. Sry, I forgot!
Attachment #399705 - Flags: ui-review?(clarkbw)
Comment on attachment 399692 [details] [diff] [review]
[medium,test] UI implementation for Duration

I'll take care of this after the beta to reduce the risk of regressions.
Attachment #399692 - Attachment description: UI implementation for Duration → [after beta] UI implementation for Duration
Attachment #399692 - Flags: review?(philipp)


9 years ago
Assignee: nobody → odor
Comment on attachment 399705 [details]
screenshot, duration patch

This looks good.

I don't recall if 'h' is the correct abbreviation to use for hours; I would have assumed it was 'hr' but I'll have to look it up.

Also are their specs on the duration that we should be looking at?  I noticed there is a 24 hour limit in the code and wasn't sure if that was the correct behavior.
Attachment #399692 - Attachment description: [after beta] UI implementation for Duration → [medium,test] UI implementation for Duration
Comment on attachment 399692 [details] [diff] [review]
[medium,test] UI implementation for Duration

First review iteration is just a shallow code review, to give you an idea whats needed.

Also, regarding the UI, I'd personally prefer just one textbox that allows entering numbers in the format hh:mm, and the up/down arrows incrementing/decrementing the minute. Maybe there is a standard format for xx days, hh:mm or we can do some simple parsing in case the user enters "days". We might want to create a new binding for this similar to the time picker binding. Bryan, what do you think?

>+        if (gItemDuration)
>+            updateDurationElements();
Use brackets even for one-line if()s, here and elsewhere.

>+ * checkbox event handler
>+ */
>+function updateDurationCheckbox() {
>+    durationControl(getElementValue("todo-has-duration","checked"));
>+ * enables or disables duration fields in dialog
I'd appreciate some spacing, i.e one empty line between } and /**

>     // calculate the duration if possible
>     var hasEntryDate = getElementValue("todo-has-entrydate", "checked");
>     var hasDueDate = getElementValue("todo-has-duedate", "checked");
>+    let hasDuration = getElementValue("todo-has-duration", "checked");
Go ahead and change the remaining var's to lets too.

>+        date1.hour=0;
>+        date2.minute=0;
>+        date2.hour=getElementValue("durationHourTextBox");
>+        date2.minute=getElementValue("durationMinTextBox");
>+        gItemDuration=date2.subtractDate(date1);
Spaces around operators, here and elsewhere.

>+        if(getElementValue("todo-has-duration", "checked")) {
getElementValue returns a string, I'd suggest comparing with string "true" here.

>+                <vbox>
>+                    <hbox id="todo-grid-duration-picker-box"
>+                          align="center">
Please give this vbox an id, so extensions can hook into it, if they need to.

>+                        <textbox id="durationHourTextBox" 
>+                                 class="todo-only" 
>+                                 type="number" 
>+                                 size="2" 
>+                                 min="0" 
>+                                 max="24" 
>+                                 value="0" 
>+                                 disabled="true"/>
Yes, I don't think max=24 is right here. Tasks can take as long as they want. See above suggestion with the textbox.

>+                        <textbox id="durationMinTextBox" 
>+                                 class="todo-only" 
>+                                 type="number" 
>+                                 size="2" 
>+                                 min="0" 
>+                                 max="60" 
>+                                 value = "15" 
>+                                 increment="5" 
>+                                 disabled="true"/>
If we do use these textboxes, then max="59" should be right.

r- for now, I'm happy to discuss the above comments.
Attachment #399692 - Flags: review?(philipp) → review-
Duplicate of this bug: 274362
Duplicate of this bug: 427402
Comment on attachment 399705 [details]
screenshot, duration patch

handing this over to andreas who is helping me catch up with my queue
Attachment #399705 - Flags: ui-review?(clarkbw) → ui-review?(nisses.mail)
(In reply to comment #5)
> Comment on attachment 399705 [details]
> screenshot, duration patch
> I don't recall if 'h' is the correct abbreviation to use for hours; I would
> have assumed it was 'hr' but I'll have to look it up.

According to Wikipedia, it's both
I'm slightly worried about the fact that the patch have checkboxes that turn each other off. Investigating if there could be a better control for this.
Just wondering, if a start date is set to 16:00, duration is 2 hours, would that mean that Due date should become 18:00?
Comment on attachment 399705 [details]
screenshot, duration patch

I'm stealing the UI review from Andreas.
ui-r- because the Duration line breaks the "Keep Duration" button which has landed after this ui-r request.
This makes it also questionable if this bug is still desired.

What I also saw during test was, when a start and a due date are entered and I activate the duration checkmark, then the duration is default to 15 minutes. Better would be when the difference between start and due would be taken.
Attachment #399705 - Flags: ui-review?(bugs) → ui-review-
Philipp, what do you think, is this bug still needed together with the "Keep duration" button? Both are doing almost the same. This bug's advantage is the easier entering of a duration instead of the manual calculation of end time.

Maybe when the "Keep duration" button is activated a duration field becomes active behind the button to enter the duration. But this could make the dialog wider than we want.
Flags: needinfo?(philipp)
Hmm looking at this again, I think the extra row will clutter the dialog, but I do think there should be some way to enter a duration. Putting it on the right side would indeed waste horizontal space. I'm not really sure how to continue on this, I think the best we can do now is to wait for a new proposal.
Flags: needinfo?(philipp)
Since duration and due date can't be active at the same time, we could avoid the extra row by collapsing the inactive one. We'd need another control to switch between the two, but I'm not sure what that should look like. I like the idea of replacing the "keep duration" button with something like this, I think the current button is too easy to miss if you don't know it's there.

From and implementation standpoint I'd suggest mapping the duration to the DUE property as an offset to the start date (or current date/time if there isn't one) rather than the DURATION property. I know that gets away from the original intent of the bug, but I fear that implementing duration in the storage provider will have a perf overhead when selecting tasks by date range.
Flags: needinfo?(philipp)

Comment 17

3 years ago
I lack the exact interpretation of "duration" and "due".
RFC5545 says:
                  ; Either 'due' or 'duration' MAY appear in
                  ; a 'todoprop', but 'due' and 'duration'
                  ; MUST NOT occur in the same 'todoprop'.
                  ; If 'duration' appear in a 'todoprop',
                  ; then 'dtstart' MUST also appear in
                  ; the same 'todoprop'.
                  due / duration /

It si not clear for me from what requirement this rule come. 
Why "dtstart"is required only together with "duration" but not with "due".
And why "dtstart" is required in case of "VTODO" but not in case of "VEVENT":

                  ; Either 'dtend' or 'duration' MAY appear in
                  ; a 'eventprop', but 'dtend' and 'duration'
                  ; MUST NOT occur in the same 'eventprop'.
                  dtend / duration /

Can someone to explain this or to give some links where it is explained?
(In reply to Ján Koštial from comment #17)
> It si not clear for me from what requirement this rule come. 
> Why "dtstart"is required only together with "duration" but not with "due".
> And why "dtstart" is required in case of "VTODO" but not in case of "VEVENT":

The idea is that you can define the optional end/due date of a task either as an explicit date-time value (using the due property), or as an offset from the start date (using the duration property). If the duration property is used, then the start date must be defined for that duration to be meaningful. 

The start date for a task is optional, so that it can represent something that needs to be done, without needing to be tied to a particular date range. This differs from an event, which will always have a defined starting and ending time.

Comment 19

3 years ago
Created attachment 8528353 [details]
Sceenshot - proposed UI

Comment 20

3 years ago
Thanks for explanation. So my idea for UI is:

For the task the label "Due Date:" will be replaced by menulist with two menuitems:
"Due Date:" and "Duration:" - se the attachment 8528353 [details].

When menulist will be changed - controls for "Due Date" and "Duration" will be hidden/unhidden depend on chosen item in menulist.


a year ago
Assignee: odor → nobody
You need to log in before you can comment on or make changes to this bug.