feat: add alarms (reminders)

[jaylinski]

Description

This PR adds an alarms (reminder) feature to the tasks app.

Tasks

• Test on my own instance
• Gather visual feedback and improve UX
• Gather code feedback and improve code (it's a lot of code, maybe some parts can be removed or stripped down)
• Add German translations on Transifex as soon as merged

Screenshots

Fixes #154.

[raimund-schluessler]

Wow, thanks a lot for your contribution. I can see that this took a lot of work. From a first glance it looks great, both code and screenshots. I will give it a test as soon as I can.

[raimund-schluessler]

nickvergessen jancborchardt Could you please invite jaylinski to the Nextcloud org (if he likes)? Collaborating would be easier then. Thanks a lot!

[raimund-schluessler]

@jaylinski I only had a bit of time yet and played around with the feature a little. It seems to work nicely in general, although I didn't check yet if the reminders really trigger a notification on the server or mobile devices. So this is not an extensive review yet and I only collect some things I noticed.

• When editing an alarm (Reminder dropdown menu -> Edit time) one can select "Empty" in the datepicker dropdown. Saving the changes to the reminder via "Update reminder" then triggers this error:

Uncaught (in promise) TypeError: e is undefined
    fromDateTimeString http://nextcloud.local/apps-extra/tasks/js/store-C5HPSdT2.chunk.mjs:493
    decorate http://nextcloud.local/apps-extra/tasks/js/store-C5HPSdT2.chunk.mjs:497
    _decorate http://nextcloud.local/apps-extra/tasks/js/store-C5HPSdT2.chunk.mjs:498
    _setDecoratedValue http://nextcloud.local/apps-extra/tasks/js/store-C5HPSdT2.chunk.mjs:498
    setValue http://nextcloud.local/apps-extra/tasks/js/store-C5HPSdT2.chunk.mjs:499
    updatePropertyWithValue http://nextcloud.local/apps-extra/tasks/js/store-C5HPSdT2.chunk.mjs:499
    updateAlarm http://nextcloud.local/apps-extra/tasks/js/store-C5HPSdT2.chunk.mjs:500
    updateAlarm http://nextcloud.local/apps-extra/tasks/js/store-C5HPSdT2.chunk.mjs:4355
    J$/< http://nextcloud.local/apps-extra/tasks/js/store-C5HPSdT2.chunk.mjs:490
    commit http://nextcloud.local/apps-extra/tasks/js/store-C5HPSdT2.chunk.mjs:490
store-C5HPSdT2.chunk.mjs:493:282418

• If the DUE/DURATION or DTSTART date are removed, reminders related to END or START need to be removed as well.

[raimund-schluessler]

In general it seems to work really nicely with most cases already thought out well. It will require a bit more testing as this is a huge feature with many corner cases. However, if the above things are fixed, I am fine with merging it already and proceed in follow-up PRs.

Some minor design nitpicks can also be fixed later.

[nickvergessen]

Could you please invite jaylinski to the Nextcloud org (if he likes)? Collaborating would be easier then. Thanks a lot!

Done

[jaylinski]

@raimund-schluessler Thanks for the review. I'll fix the JS error.

Regarding the removal of reminders that depend on the start/end-date. I thought about it and I agree.
But I would somehow tell the user about it and I can't decide between those options:

• Prompt the user with a pop-up if the date should really be deleted, as this will also delete the reminders
  • Pros: always a clean state, no dangling alarms
• Prompt the user with a pop-up and tell the user that existing reminders will not work anymore (and don't delete reminders)
  • Pros: user can just create a new date and the existing reminders will work again
• Prompt the user with a pop-up and let the user decide if only the date or the date including reminders should be deleted
  • Pros: user can decide what to do

Maybe there are other good options.

[raimund-schluessler]

@jaylinski We should stay in accordance to the CalDAV specification, which states that relative alarms without the date the alarm is related to are not allowed. See the description in https://icalendar.org/iCalendar-RFC-5545/3-6-6-alarm-component.html:

In an alarm set to trigger on the "START" of an event or to-do,
      the "DTSTART" property MUST be present in the associated event or
      to-do.  In an alarm in a "VEVENT" calendar component set to
      trigger on the "END" of the event, either the "DTEND" property
      MUST be present, or the "DTSTART" and "DURATION" properties MUST
      both be present.  In an alarm in a "VTODO" calendar component set
      to trigger on the "END" of the to-do, either the "DUE" property
      MUST be present, or the "DTSTART" and "DURATION" properties MUST
      both be present.

So, the options 2 and 3 in your list are not possible by the specification alone. Furthermore, the user would still see the alarms being present, and might falsely assume that they are still working, where in reality they won't.

However, we are free to convert relative alarms to absolute alarms, if the related date is deleted. We could present the user with a popup to let him decide whether he wants to:

• cancel the deletion of due or start date
• convert relative alarms related to the deleted date to absolute alarms
• delete the relative alarms related to the deleted date

The other option would be to not confront the user with this problem, and simply convert relative to absolute alarms. This is what we mostly do, to prevent pop-ups and dialogs as they often interfere with the workflow.

@jancborchardt and @nimishavijay might also have an opinion here.

[nimishavijay]

Completely agreed! Relative dates make more sense anyway, and for the deletion popup I would suggest that we show it but simplify the wording to

This task has 2 alarms. Would you like to keep them?
[Keep alarms] [Discard alarms] and [Cancel]

That way the user won't be confused when the alarm that they had set with the due date in mind goes off without any warning :)

[jaylinski]

I just pushed the fix for the select "Empty" in the datepicker-issue: https://github.com/nextcloud/tasks/compare/767cba3814155dac286c40a63fdbf6abb7730671..fede77d198fb325184f07a2865d6d47a55da074c

Since it is an edge-case, I tried to implement it as simple as possible and used the browser-native validation. If that's ok for you, I'll try to get the custom CSS for the NcDateTimePickerNative-component into the component-lib.

As for the start/end-date deletion issue, I'll try to implement it as suggested by @nimishavijay.