feat: add Duration field to tasks - #3768
Conversation
Implements estimated duration as a new task field, addressing the feature request in issue obsidian-tasks-group#1649. ## What was added **Core storage and parsing** - New `Duration` class (`src/Task/Duration.ts`): immutable value object storing hours and minutes, with normalisation (90m → 1h30m), a `Duration.None` sentinel, and `fromText`/`toText`/`fromTotalMinutes` helpers - Serialiser support in both Tasks emoji format (⏱) and Dataview inline field format (`[duration:: ...]`) - `Task` class extended with `duration`, `durationHours`, and `durationMinutes` properties **Query language** - New `DurationField` filter supporting `has duration`, `no duration`, `duration is <value>`, `duration above <value>`, `duration below <value>` - `sort by duration` / `sort by duration reverse` — tasks with no duration sort after tasks with a duration - `group by duration` / `group by duration reverse` — tasks with no duration group as "No duration" - `hide duration` / `show duration` layout instructions **Edit modal** - New `DurationEditor.svelte` component with real-time validation and normalised preview; duration field is togglable via modal settings **Auto-suggest** - Duration symbol added to the suggestor trigger regex - Common presets suggested: 15m, 30m, 45m, 1h, 1h30m, 2h, 3h, 4h - Works in both Tasks emoji and Dataview formats **Documentation** - New standalone page: `docs/Getting Started/Duration.md` - Duration sections added to Filters, Sorting, Grouping, and Quick Reference docs ## Testing All 4703 existing tests continue to pass. New tests added: - `tests/Task/Duration.test.ts` — Duration class: parsing, normalisation, edge cases, `toText`, `totalMinutes`, `fromTotalMinutes` - `tests/Query/Filter/DurationField.test.ts` — all filter keywords, boundary conditions, sort order (including no-duration-last), grouping, and `canCreateFilterForLine` - `tests/ui/DurationEditor.test.ts` — Svelte component: empty input, valid input, normalisation (90m→1h30m), invalid input error state, initial value pre-population - `tests/ui/EditableTask.test.ts` — `parseAndValidateDuration()`: all valid formats, normalisation, and invalid input error messages - `tests/Query/Query.test.ts` — wire-up tests for all duration instructions - Approval tests updated for renderer, suggestor, and serialiser snapshots Manual testing completed on desktop and mobile (videos recorded): - Task creation and editing via the modal in both emoji and Dataview formats - All filter instructions verified against a test vault - Auto-suggest in Dataview format confirmed working - Sort and group by duration verified visually Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Please find the testing validation for the various scenarios on desktop and mobile including the emoji rendering Desktop Emoji Editobsidian-task-plugin-testing-desktop-emoji-small-edit.movDesktop Emoji Reportobsidian-task-plugin-testing-desktop-emoji-small-report.movDesktop Dataview Editobsidian-task-plugin-testing-desktop-dataview-small-edit.movDesktop Dataview Reportobsidian-task-plugin-testing-desktop-dataview-small-report.movMobile Emoji Editobsidian-task-plugin-testing-mobile-emoji-small-edit.movMobile Dataview Editobsidian-task-plugin-testing-mobile-dataview-small-edit.movMobile Emoji / Dataview Reportobsidian-task-plugin-testing-mobile-emoji-small-report.mov |
…bsidian-tasks-group#3767) PR obsidian-tasks-group#3767 (fix-mutable-dates) made Task date fields private (`_createdDate` etc.) with public getters using a `resolveDate()` helper to support safe spreading. Conflicts resolved in three files: - `src/Task/Task.ts`: kept `duration` as a public readonly field alongside the new private date fields; added `duration` to the constructor destructuring and used `this.duration = duration ?? Duration.None` in the body alongside the `resolveDate()` calls - `src/TaskSerializer/DefaultTaskSerializer.ts`: replaced the old hand-rolled match blocks (which our branch still had) with a single `this.extractField(state, durationRegex, ...)` call matching the new refactored pattern - `tests/ui/EditableTask.test.ts`: updated inline snapshot to use the new private field names (`_doneDate`, `_dueDate`, etc.) while keeping the `duration: Duration { hours: 1, minutes: 30 }` entry Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Many thanks for doing this. I'm curious - The PR seems to have been created from the old template, as it doesn't not have the How could that happen? |
|
FYI I am going to push a few changes to your branch. I will let you know when I have finished. |
I've finished pushing for now. |
|
thanks @claremacrae - I went back and made sure the PR description included everything that you outlined in #1649 (comment) . And I see the changes that you added. Is there anything else that you'd like to see and test before moving forward or is that pending a more exhaustive review? |
The PR Description
I was unclear. I have edited the description to add the missing checkboxes (docs and vault updated, and a branch has been created) - those were the things missing from the PR template... By all means list the user-visible feature changes (things like the new instructions). It's a good if a plugin user can read the description and see what is provided. But I would prefer the PR description not to list the finer points of everything that has changed. So please do not list the individual documentation and test changes. They should be visible in the code, and the code is the source of truth there. It's a waste of time maintaining and reading detailed lists of internal changes. What next(I edited this section a bit for readability)
Yes, lots. Too many for it to be feasible to record them all in one code review - it would be unmanageable for you to implement them, and it would be unmanageable for us both to track all the requests. So there won't be a "more exhaustive review". So when I am confident of my requested changes, I will pick a reasonably well-bounded initial set of feedback and ask you to address those. Then we can rinse and repeat. As a very wild guess, there might be between 5 and 10 small-ish cycles like this, as we address one topic, decision, or area of change - and iterate towards something that is reliable, robust, logical, maintainable and releasable.... |
|
Just for info, I'm going to be pairing with @ilandikov shortly, and we will have a look at the position of the Duration field in the Edit task modal. I am sure that the new location can be improved - the Duration field is nothing to do with the Status, Created, Done and Cancelled - but less sure of where exactly to move it to... So I expect I will be pushing another change to your branch soon.... 
|
|
I'm going to edit the PR description to shorten it - I appreciate the care that you or an AI took, but I want to keep the PR readable. Later, once I start doing code reviews, we will track progress on what needs to be done using GitHub's 'Resolve conversation' facility. More about that nearer the time... |
It was previously between Status and Created Date - which didn't seem logical. It is now between Priorities and Recurrence, in the Dates section... Co-Authored-By: Ilyas Landikov <93825870+ilandikov@users.noreply.github.com>
It was previously between Start and Before this - which didn't seem logical. It is now between Priorities and Recurrence, in the Dates section, consistent with the (new) order of fields in the modal itself. Co-Authored-By: Ilyas Landikov <93825870+ilandikov@users.noreply.github.com>
Done. The changes are: 
|
claremacrae
left a comment
claremacrae
left a comment
There was a problem hiding this comment.
Hi @rmartin, thank you again for working on this.
My first set of comments is around the conceptual order/positioning of the duration field throughout the code base...
Some of these changes will be noticeable to users, including the position of the new field in task lines.
Others are about consistency within the code - where Duration should not be mixed in the middle of date-related code.
Making these changes
DO NOT put all these changes in a single commit. I want to be able to review the changes in manageable chunks, one commit at a time.
So, for example, you could start by
- making the requested changes to the Serialiazer code
- get the tests to pass
- and then commit those changes
And then continue to another area, such as TaskLayout and TaskLayoutOptions (or perhaps have them as two separate commits...)
DO NOT mark conversations as resolved, please. As I review the new commits, I will resolve conversations myself.
Problem with order of fields
The original code that you copied was obviously written by:
- searching for all uses of a particular date field
- duplicating the code
- changing the duplicate to do Duration instead.
This had resulted in a bit of a mess
Requested order
Having experimented with the code, and done exploratory testing, it generally feels logical to me for the Duration to appear after the Priority.
The one exception is in Auto Suggest, where I propose moving Duration lower down the list, based on my estimate of likely frequency of use.
claremacrae
added
the
status: ready to review
|
Sounds good, I appreciate the diligence and thought going into the reviews and comments. I'll review further throughout the week to address these in the logical groupings in small commits to address your feedback. Looking forward to collaborating and learning from this process myself and getting this feature ready for myself and others. Thank you. |
claremacrae
removed
the
status: ready to review
|
Hi @rmartin, how are you getting on with this? |
d629dd5 to
dcdedd3
Compare
|
Hi @claremacrae, apologies for the delay — I had some unexpected additional commitments at home and work that took priority. I understand you're starting the larger refactor, and I'm fully committed to working with you to get this feature included. I've addressed all 17 inline review comments from your first round of feedback. Per your instructions, the changes are split into atomic commits by logical area:
All 4730 tests pass at each commit. I have not resolved any conversations — leaving that to you as requested. |
|
Many thanks. I'm a bit confused, did you force-push? |
Yes, all the commits on this PR are now dated 7th March or 8th March. May I know the reason for force-pushing please? I had expected only to have to review the differences in the commits made since my original review - but now I need to re-review all the initial commits too. As we go through later rounds of code review, please do not force-push. You do not need to worry about merging in |
|
Moving @beauraines's comments from #1649 (comment): @claremacrae @rmartin I've been following PR #3768 and the review feedback. It looks like the branch is in a difficult state after the force-push, and I noticed Clare's recent comment about progress. @rmartin — if you don't have bandwidth to continue, I could pick this up with AI assistance (GitHub Copilot). I'd reimplement on a fresh branch from main using your implementation as the foundation, but with the field ordering correct from the start (Duration after Priority, not mixed with dates). I'd structure it as incremental TDD commits so each one builds and passes tests independently. @claremacrae — would you be open to that, or would you prefer to give rmartin more time? Happy to wait either way. |
No, I actively absolutely do not want more AI used to start this again from scratch, please. In no way should reordering some lines of code require the use of an AI, and it absolutely is not worth the climate-change-inducing/water-sapping technology to do it. Not does it require restarting the whole process again.
I'll leave a few more days for @rmartin to reply. If I don't hear back, I'll force-push the previous commit to here, and fix the line ordering myself. And then I'll make some more suggestions. But the remaining issues in the originally-pushed code are requiring testing and decisions about desirable behaviour, based on actually using the feature, not things that an AI can drive forward. |
Move duration-related declarations, symbols, regex patterns, extraction logic, and return values to appear after priority (instead of mixed in with date fields) in DefaultTaskSerializer, DataviewTaskSerializer, and TaskSerializer index.
Move duration field declaration, constructor parameter, assignment, and identicalTo check to appear after priority. Move durationHours/durationMinutes getters to after urgency. Update TaskBuilder field, method, and build ordering.
Move Duration in TaskLayoutComponent enum to after Priority, which changes the rendering and serialization order. Reorder taskFieldHTMLData entries to match. Update expected test arrays, snapshots, and approval files.
Move DurationField registration to after PriorityField in the fieldCreators array.
Move duration field declarations, constructor parameters, assignments, and fromTask/applyEdits ordering to after priority in EditableTask. Move DurationEditor section from inside dates to after priority in EditTask.svelte. Reorder formIsValid conditions.
Move duration suggestion to appear after recurring (instead of after scheduled date) per frequency-of-use ordering. Update approval files.
Move Duration section after Priority in Quick Reference docs. Move duration field below priority in CustomMatchers helper functions.
dcdedd3 to
2bfc133
Compare
|
Hi @claremacrae, apologies for the delay — I had some unexpected additional commitments at home and work that took longer than expected. I've reset the branch back to d629dd5 as you asked and remade all the fix commits fresh. Each commit has been individually verified to pass the full test suite (4715 tests) and lint. Here's the updated commit history:
Every intermediate commit passes |
claremacrae
added
the
status: ready to review
A tip is not top put commit ids in backticks - as then the GitHub web ui actually links directly to the commits, which is really useful. It also shows when a commit id was mistyped (instead of copied-and-pasted) I've adjusted that message to remove the backticks, and fixed the broken link, just for convenience.... |
# Conflicts: # docs/Queries/Presets.md
|
I've merged |
This will allow me to trigger the checks to run on the branch. |
|
|
I'd appreciate any thoughts on the following: Non-support of zero-length durationsFrom reviewing the code and experimenting using the feature, the current design does not allow a user to say a duration is zero. Ignoring the implementation for now, I'm interested in user-facing thoughts about this behaviour. An example of a zero-length task might be: Currently as soon as that task is modified by Tasks, the The workaround at the moment is this: My feeling is that this is:
My inclination is to leave it as-is now, as it is already rather a large change - and then have a separate PR to enable support of 0-length durations. |
|
That's a really good point about zero-cost estimates I hadn't considered that use case. Tracking tasks where someone else does the work (costing you 0 time) is a perfectly valid scenario, and silently deleting the I agree with your inclination to leave it as-is for this PR and address it separately. Happy to raise an issue and follow-up PR for zero-duration support after this one is merged, if that works for you. |
claremacrae
left a comment
claremacrae
left a comment
There was a problem hiding this comment.
NEW_TASK_FIELD_EDIT_REQUIRED
This page in the Contributing guide records the need to search for, and review, all occurrences of NEW_TASK_FIELD_EDIT_REQUIRED throughout the code base.
I suspect this hasn't been done fully, as the file Styling.md has 3 of them, and hasn't been updated.
So please could you
- update all 3 locations in
Styling.md - find and review all the
NEW_TASK_FIELD_EDIT_REQUIREDstrings to see if anything else was missed out.
I've done some testing of the Edit Task modal changes, and made comments in this review on the appearance...
Also, a few niggles around confusing edits that are just a bit harder to review because of the moving around of code...
| priority: new TaskFieldHTMLData('task-priority', 'taskPriority', (_component, task) => { | ||
| return PriorityTools.priorityNameUsingNormal(task.priority).toLocaleLowerCase(); | ||
| }), | ||
| duration: new TaskFieldHTMLData('task-duration', 'taskDuration', (_component, task) => { | ||
| return task.duration.toText(); | ||
| }), | ||
|
|
||
| createdDate: createDateField('task-created', 'taskCreated'), | ||
| dueDate: createDateField('task-due', 'taskDue'), | ||
| startDate: createDateField('task-start', 'taskStart'), | ||
| scheduledDate: createDateField('task-scheduled', 'taskScheduled'), | ||
| dueDate: createDateField('task-due', 'taskDue'), | ||
| doneDate: createDateField('task-done', 'taskDone'), | ||
| cancelledDate: createDateField('task-cancelled', 'taskCancelled'), | ||
|
|
||
| priority: new TaskFieldHTMLData('task-priority', 'taskPriority', (_component, task) => { | ||
| return PriorityTools.priorityNameUsingNormal(task.priority).toLocaleLowerCase(); | ||
| }), | ||
|
|
||
| description: createFieldWithoutDataAttributes('task-description'), |
There was a problem hiding this comment.
There are more diffs here than I expected, for what would have been adding a new field.
Can you remember the reason why?
| /** | ||
| * Return the hours component of the task's duration, or null if no duration is set. | ||
| */ | ||
| public get durationHours(): number | null { | ||
| return this.duration === Duration.None ? null : this.duration.hours; | ||
| } | ||
|
|
||
| /** | ||
| * Return the minutes component of the task's duration, or null if no duration is set. | ||
| */ | ||
| public get durationMinutes(): number | null { | ||
| return this.duration === Duration.None ? null : this.duration.minutes; | ||
| } |
There was a problem hiding this comment.
I think it would be best to delete durationHours() and durationMinutes().
When I comment them out, both yarn lint and jest still pass - so they are unused and un-tested.
I would rather that behaviour was contained without in the Duration class.
| $: formIsValid = | ||
| isDueDateValid && | ||
| isDescriptionValid && | ||
| isDurationValid && | ||
| isRecurrenceValid && | ||
| isScheduledDateValid && | ||
| isStartDateValid && | ||
| isDescriptionValid && | ||
| isScheduledDateValid && | ||
| isDueDateValid && | ||
| isCancelledDateValid && | ||
| isCreatedDateValid && | ||
| isDoneDateValid; |
There was a problem hiding this comment.
The diff is hard to spot because of the reordering of existing lines...
| {/if} | ||
|
|
||
| <!-- --------------------------------------------------------------------------- --> | ||
| <!-- Dates and times --> |
There was a problem hiding this comment.
What was the reason for adding 'and times'? Best to leave it as just Dates for now, please.
| <!-- --------------------------------------------------------------------------- --> | ||
| {#if isShownInEditModal.duration} | ||
| <DurationEditor {editableTask} bind:isDurationValid /> | ||
| {/if} |
There was a problem hiding this comment.
- It would be really helpful if the Duration Edit field could align with the date fields below it.
- And it would be good for it to have a keyboard accelerator. There is a comment block in
EditTask.sveltethat lists all the available and used characters - looking at that list, there are no obvious ones, so I suggest usingQ- arbitrarily - as it is the one least likely to be useful for other new fields... However the(x)and(-)are displayed onDoneandCancelledshould show how to do this - and please do update the comment listing characters that are used, as well.
claremacrae
removed
the
status: ready to review
|
This functionality would be super useful for me! Thank you guys for working on it. If progress goes stale for a while, I can try to pick it up later to finish it up |
|
@rmartin Thanks for what you have done so far. How are you feeling about responding to the second set of feedback above? If you are unable to continue, that's fine, please do say. I would then pick it up myself, when I have time - rather than having to start again with someone else on a third branch... |
@theluxaz Hi, thanks for the offer, but please don't spend your time on it... |
|
Hi Clare,
Thanks for the feedback, I will look into this in the coming days and
address this. I want to hand-review this to ensure I understand the
details. I will get back to you with my findings over the next few days.
Thank you!
Cheers,
Roy
…On Wed, Apr 29, 2026 at 9:39 AM Clare Macrae ***@***.***> wrote:
*claremacrae* left a comment (obsidian-tasks-group/obsidian-tasks#3768)
<#3768 (comment)>
This functionality would be super useful for me! Thank you guys for
working on it. If progress goes stale for a while, I can try to pick it up
later to finish it up
@theluxaz <https://github.com/theluxaz> Hi, thanks for the offer, but
please don't spend your time on it...
—
Reply to this email directly, view it on GitHub
<#3768 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA5C34DUXOBWJ63HCBJOYL4YIV2PAVCNFSM6AAAAACV4FDCBSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DGNBVGY3DOMZUG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@rmartin, Hi, I hope you are OK. Please may I have an update on the likelihood of your having enough time over the next few weeks, realistically, to be able to work with me to finish this off? |
3 participants
Types of changes
feat- non-breaking change which adds functionality)docs- improvements to any documentation content for users)vault- improvements to the [Tasks-Demo sample vault](https://github.com/obsidian-tasks-group/obsidian-tasks/tree/main/resources/sample_vaults/Tasks-Demo))Internal changes:
test- additions and improvements to unit tests and the smoke tests)Description
Implements an estimated duration field for tasks, as requested in issue #1649.
This PR is a continuation of the work started in #3464, rebased onto the current main branch and fully rounded out per the requirements listed in #1649 (comment).
What was added
Core storage and parsing
Durationclass (src/Task/Duration.ts) — immutable value object with normalisation (e.g.90m→1h30m), aDuration.Nonesentinel, andfromText/toText/fromTotalMinuteshelpers⏱) and Dataview inline field format ([duration:: ...])Taskclass extended withduration,durationHours, anddurationMinutespropertiesQuery language
DurationFieldsupporting:has duration,no duration,duration is <value>,duration above <value>,duration below <value>sort by duration/sort by duration reverse— tasks with no duration sort after tasks with a durationgroup by duration/group by duration reverse— tasks with no duration group asNo durationhide duration/show durationlayout instructionshide_non_date_fieldspresetEdit modal
DurationEditor.sveltecomponent with real-time validation and normalised previewAuto-suggest
15m,30m,45m,1h,1h30m,2h,3h,4hDocumentation
docs/Getting Started/Duration.mdHow has this been tested?
New automated tests added.
Manual testing completed on Desktop (macOS) and Mobile (iOS) — videos to follow:
has duration,no duration,duration is,duration above,duration below) verified against a test vault with boundary and normalisation tasksNo durationgroup) verified visually0h1200mconfirmed to normalise to20hScreenshots / Videos
Checklist
mainbranch.yarn run lintTerms