feat: Add Reminder Field Support

[stevendkwtz]

Description

This teaches Tasks to read and write a Reminder field, which is expected to be compatible with the Reminders plugin.

Problems found in exploratory testing

EditTask modal

• Because of muscle memory, consider putting Reminder after the existing 3 date fields
• Any time on the reminder is discarded/deleted when the edits are saved
• Use a different access key than C
• Figure out how to show the access key to users, when it is a letter not in the field name, so cannot just underline one of the visible characters

Recurrence

• When a recurring task's Reminder has a time, and the task is toggled, the Reminder date is updated but the time is discarded

Search filters

• happens 2024-04-05 does not match this task - [ ] #task reminder ⏰ 2024-04-05
  • See Filters > Happens
  • Tests that need updating first:
    • https://github.com/stevendkwtz/obsidian-tasks/blob/ec23842cc1dfbd1e79ef7ffdbdc3639721ffad2d/tests/Task/Task.test.ts#L507-L530
    • tests/Query/Filter/HappensDateField.test.ts
  • Fix will likely be to add the Reminder field to Task.happensDates

Postpone mechanism

• The Postpone mechanism needs to be taught to recognise Reminder dates
• Needs discussion as to where to add it in the priority order

Questions from a comment elsewhere to be resolved:

Things I've noted from reading through the code, if/when this is picked up:

• There was an earlier, incomplete PR that may already have answered some of the following: feat: Add Reminders fields and Notifications  #1925
• A significant concern - and area in need of thorough exploratory and automated testing - with this and similar implementations is how things like before and after in date searches interact with values that have times on - in ReminderDateField.ts - and whether this will give confusing results... as the entire search mechanism is built around times being at midnight...
• In Recurrence.ts, what was the thinking behind putting Reminder before Scheduled and Start in the priority order? This would be quite a significant change that would need clear documentation....
• With Reminder having been added to Recurrence, I think it should probably be added to the happens query, and to Task.happensDates()
• There is also type HappensDate in Postponer.ts - how should Reminder interact with the Postpone facility - or vice versa?
• The order of fields in Task lines needs review - I'm not sure where in the line Reminder should go, but Done date should definitely still be the last field on Task lines (which it looks like it might have done, from one of the tests)
• The following code could be put behind a helper function somewhere, to avoid repetition and the risk of inconsistency....

task.reminderDate?.format(
                    isDateTime(task.reminderDate)
                        ? TaskRegularExpressions.dateTimeFormat
                        : TaskRegularExpressions.dateFormat,
                ),

• withAllRepresentativeReminderDates() would benefit from having some values with times in too

@stevendkwtz notes from our pairing sessions

https://gist.github.com/stevendkwtz/9b2758642f151219b8a3f3753eee9e9d

---

Motivation and Context

Related PRs:

• Native reminder support #1854

How has this been tested?

Screenshots (if appropriate)

Types of changes

Changes visible to users:

• New feature (prefix: feat - non-breaking change which adds functionality)
• Documentation (prefix: docs - improvements to any documentation content for users)
• Sample vault (prefix: vault - improvements to the Tasks-Demo sample vault)

Internal changes:

• Tests (prefix: test - additions and improvements to unit tests and the smoke tests)

Checklist

• My code follows the code style of this project and passes yarn run lint.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• My change has adequate Unit Test coverage.

Terms

• My contribution follow this project's contributing guide
• I agree to follow this project's Code of Conduct

[stevendkwtz]

😅 if you're wondering where this came from. git rebase --continue wouldn't work during an ongoing massive rebase because it kept saying no file changed. so I had to make a small edit to get it to continue.

[claremacrae]

This is a little tricky...
The class accesskey-first says 'mark the first letter as the shortcut'..
However, as all the letters in Reminder are already used as shortcuts, the key c is chosen...

Which leaves two questions:

• How do we convey to users that c is the access key?
• How do we ensure that we do not mark the wrong character...

[claremacrae]

There is now a pattern for doing this on main, for the Done and Created fields IIRC.

[claremacrae]

This pattern of isDateTime() ? ... : ... is going to grow and grow....

we could save our time by introducing a helper function that does the check and the formatting...

[claremacrae]

I am about to use C on main for Created Date, so this will need to be changed...

[claremacrae]

This looks like an accidental edit.

[claremacrae]

This needs to do parseDateTime()

[stevendkwtz]

I'm such a n00b but I have no idea how to start a conversation on my latest pushed commit to my forked reminders branch.

Here is what we've done today

There are likely a lot of follow ups on your mind @claremacrae, for me I want to note some down so I don't lose track.

Remind @ilandikov about my ApprovalsJS PR

Priority: High
High priority for me, and timely since you'll be pairing with him today. My change is quite small, but would allow me to run the approvals testsuite in obsidian-tasks repo without hacking around it and also should be a fairly simple update.

TasksDate - refactor class formatter instance method logic into Moment formatting utils file

Priority: High
Note: I'm referring to functions like formatAsDate and my newly created formatAsDateAndTimeOrDate.

My opinion:

1. there is no reason to couple this logic to TasksDate. At their core, they're Moment format helpers.
2. The reusable value outside of a TasksDate instance is becoming very clear to me, such as in the above commits we've worked on today.
3. We can still keep the TasksDate class instance formatter functions, but call newly created shared utils to handle the formatting logic itself. DRYs code, and doesn't require changes elsewhere, so it feels like a huge win win to me.

stronger string formatting

1. Priority: Low, outside of scope of this PR- "YYYY-MM-DD" and "YYYY-MM-DD HH:mm" should be constants and not used directly throughout the code. Reuse these consts and replace all other direct usage of "YYY-MM-DD" etc throughout the project.
2. Priority: Low (and out of scope of this PR) - Use typed enums for specified strings passed as function args like this. I like to avoid ever using strings directly in code if we can make them enums/consts because, typos.

chrono - remove dependency from project

Priority: Low

1. chrono has only 2 use cases in the entire repo plus 1 test mock.
2. chrono is used only for a single function in the repo, chrono.parseDate()
3. I strongly believe we can reimplement this logic using moment and vanilla JS, and the code that hits this path is very well tested.
4. Where chrono.parseDate() is used, we lose date/date time format from the input string, which is critical now that we're introducing new behavior to handle both formats.
5. chrono.parseDate() returns a JS Date object, which we then reformat back to Moment. It is objectively an inefficient transformation layer, and I believe, can become an unnecessary one once we reimplement the behavior we need from this function. If that requires lifting some internal chrono code, we can! The project has an MIT License.

Consider remotely configurable feature flags in this project

Priority: Low, out of scope
Just from discussion today, we learned we had different ideas about the concept of feature flags (experimental user settings toggles vs ability to remotely remediate bugs we may accidentally introduce). Just introducing an open question about the future thinking of the project here :)

[claremacrae]

Remind @ilandikov about my ApprovalsJS PR

Priority: High High priority for me, and timely since you'll be pairing with him today. My change is quite small, but would allow me to run the approvals testsuite in obsidian-tasks repo without hacking around it and also should be a fairly simple update.

Thanks - good guess, but when I speak with @isidore later - Llewellyn Falco - I'll remind him about that PR... :-)