Add notification support to timers

[christianlupus]

For the RecipeTimer components, we could enable the desktop notifications to notify the user of end of timer.

---

Depends on #1067

[MarcelRobitaille]

I started poking at this issue. I'm not sure if you intended to use browser notifications or integrate with the Nextcloud Notification app. I looked at their docs, and I don't see any example to trigger it from the browser. Their example is from the backend, is overly complicated for this IMO. I think normal browser notifications would be better, at least for now.

Edit: I wonder if this replaces alert() or do we keep both? We could use alert() only if we don't have notification permission, or we could keep both.

https://github.com/MarcelRobitaille/cookbook/tree/1064-notifications

[christianlupus]

I think the desktop notifications are better for now than the nextcloud notifications as a first step.

The nextcloud notifications would have the benefit that all devices get the notification and the original does not need to be online. I am thinking of a slow cooking fish which takes hours or even days. Your tablet/computer will not run all the time without closing the page. I would postpone that however for now. It will Twitter also a new API backend. We could also ask in a poll if this feature was even considered helpful.

Regarding the desktop notifications, i think it makes sense to encapsulate things in its own js file. There is already a stub pre-implemented in

cookbook/src/js/helper.js

Lines 128 to 149 in a96c973

	function notify(title, options) {
	if (!("Notification" in window)) {
	return
	}
	if (Notification.permission === "granted") {
	// eslint-disable-next-line no-unused-vars
	const notification = new Notification(title, options)
	} else if (Notification.permission !== "denied") {
	Notification.requestPermission((permission) => {
	if (!("permission" in Notification)) {
	Notification.permission = permission
	}
	if (permission === "granted") {
	// eslint-disable-next-line no-unused-vars
	const notification = new Notification(title, options)
	} else {
	// eslint-disable-next-line no-alert
	alert(title)
	}
	})
	}
	}

. you can add further files there for simpler code structure 😉.

[MarcelRobitaille]

I didn't know about that function. Sorry. Isn't it better to request notification permissions before starting the timer though? If you go to a different page, you might not see the request to allow notifications when the timer goes off

[christianlupus]

Sure, the function was never used and is not finished. I would suggest to break up the functionality into multiple parts (request notification permission, send notification, and optionally create notification timer). The request for permission should be triggered while starting the timer. Then the user has a real chance to consent.

If you go to a different page, you might not see [...]

We have however a different understanding of what should be implemented: A page can be unloaded for various reasons (user closes tab, browser is closed by OS's energy saving options, user uses multiple tabs and old tabs get shut down, ...). As soon as that happens, and frontend JS code is aborted including all timers. So, no notifications can be generated anymore.

Are you intending to create a ServiceWorker for the app to keep running in the background or something in this direction?

[MarcelRobitaille]

We have however a different understanding of what should be implemented: A page can be unloaded for various reasons (user closes tab, browser is closed by OS's energy saving options, user uses multiple tabs and old tabs get shut down, ...). As soon as that happens, and frontend JS code is aborted including all timers. So, no notifications can be generated anymore.

I meant go to another tab, but leave the nextcloud tab running in the background. I think it's reasonable to expect a tab to stay open if there's a timer running. Alternatively, we could integrate with the Nextcloud Notification app and try to send push notifications, but we'd need some backend and Nextcloud notifications are flaky in my experience.

[MarcelRobitaille]

Sure, the function was never used and is not finished. I would suggest to break up the functionality into multiple parts (request notification permission, send notification, and optionally create notification timer). The request for permission should be triggered while starting the timer. Then the user has a real chance to consent.

Maybe we could do a sort of factory function that returns alert if the notification API is not available or if the request to allow them is declined, and new Notification if everything is good.

[christianlupus]

I meant go to another tab, but leave the nextcloud tab running in the background. I think it's reasonable to expect a tab to stay open if there's a timer running. Alternatively, we could integrate with the Nextcloud Notification app and try to send push notifications, but we'd need some backend and Nextcloud notifications are flaky in my experience.

Yes, but at least on my Android Firefox, tabs in the background tend to get stopped soon. Unfortunately, you will not get notified or see the effect. I just wanted to bring that to the mind.

For long-running processes (!) the NC notifications might work. Sending out a notification will alert all connected devices. However, they might take some time as there is some polling involved, internally as far as I know. So, this is not useful for time spans < 30 or even < 60 min. Starting at a certain duration, the jitter might be neglectable.

With "we'd need some backend" you mean an API endpoint plus some PHP code within the app, am I right? Yes, that is true but should be doable.

Maybe we could do a sort of factory function that returns alert if the notification API is not available or if the request to allow them is declined, and new Notification if everything is good.

True, that will encapsulate the critical part (notification API) completely. Just a remark to consider when defining the API of the module: Keep #842 in mind. It might be feasible to pass some context parameters around.

[christianlupus]

Depends on #1067

This must be in the initial post of the issue, @MarcelRobitaille. Otherwise, it will not work. Can you edit the posts in general? In this post I edited myself

[MarcelRobitaille]

@christianlupus Sorry, I didn't know how that bot worked. Is the following needed as well like you have it?

---

I do have permission to edit initial posts. In the future, do you mind if I edit comments to add things like that? I am a bit hesitant to edit somebody else's comment.

[christianlupus]

Yes, that was my concern as well. The three dashes are not needed. There must just be a line matching a regex (line is depends on # or Blocked by #). The three dashes are there, so the altering of the comment is not too invasive. We could also as a manual line by us (added by @).

[christianlupus]

As this is dependent on another PR to be merged, let's postpone it to the next feature release.

[github-actions]

This PR/issue depends on:

• Create an app configuration #1067
  By Dependent Issues (🤖). Happy coding!