New notification permissions request bar?

What’s up with this blaring blue bar?

Just hopped on a new computer and I don’t remember seeing this before; and can’t see any recent Meta topics about adding it.

It’s an option (default off) from the push notifications plugin. We are testing it on Meta right now.

Here is the announcement:

https://meta.discourse.org/t/discourse-push-notifications/46692/236?u=falco

7 Likes

What is the difference between Desktop Notifications and Push Notifications? This is kinda confusing:

14 Likes

Yeah that should be simplified for the user…

5 Likes

My vote here is to add discourse-push-notifications to core and remove “Desktop Notifications” altogether.

Anyone know any use case that we would break by doing that?

1 Like

This does not answer my question, what is the difference between the two? Very confused right now :confounded:

3 Likes

Desktop notifications: only works when you are browsing the site (ie: with a tab open)

Push Notifications: always work.

7 Likes

One thing is Desktop displays the site logo and AFAIK only new posts, while Push displayed the reply icon.

Off-topic, does the mobile app notifications touch the same code?

Also, the push notifications plugin requires a Google app registration (on Chrome), while “desktop notifications” work without that - but only when the forum is open.

They don’t require that anymore. Both Chrome and Firefox implement VAPID.

3 Likes

Oh, then there’s no reason to have the old desktop notifications implementation!

In that case, I support moving push notifications to default / core and removing the old version.

5 Likes

:tada:

:heart_eyes:

I really think it would help a lot having this out of the box on Discourse everywhere now that the setup is so simple.

3 Likes

Sure go for it… two settings is super weird.

4 Likes

Wow, even works on the edge case.

Also, I am very disappointed in how long it has taken me to come up with edge puns.

9 Likes

Desktop notification and Push Notification are just two different implementations of showing a native notification on the device.

The desktop notification feature was implemented by having the clients subscribe to a messageBus channel and it uses the browser’s Web Notifications API to display a notification on the user’s desktop. When the tab is closed, it kills the messageBus subscription which is why the user would stop getting the native notification. This feature works across all the major desktop browser.

Push Notification on the other hand requires the client to have a service worker registered. When the client enables push notification, it uses the service worker API to create a pushManager subscription which is sent and stored on the server. That subscription object contains a unique (I think) endpoint to a push service and the encryption keys that are used to encrypt the payload.

Browsers that support web push each implement their own push service, which is a system for processing messages and routing them to the correct clients. Push messages destined to become notifications are sent from a server directly to the push service, and contain the information necessary for the push service to send it to the right client and wake up the correct service worker.

Currently one service worker can be registered per domain and that service worker runs in the background even if all the tabs to a site are closed. This is why we’re still able to display native notification even when all the tabs are closed.

When a notification is created in Discourse, we basically add a callback that would sent a payload to the respective browser’s push service which then does its magic on routing the payload to the client. Currently, only Safari lacks support for the Push API which I think may be coming soon since service workers have already been implemented in latest version.

9 Likes

I would hold off on this until we are able to support push on all the 4 major browsers. What I think we should do while waiting is to not show the Desktop Notifications button if Push Notifications is available in the browser.

5 Likes

I would rather rip off the band-aid and do this now. Having duplicate code and UI for basically identical features is a nightmare.

5 Likes

I also want to squelch notifications when a page is actively focused on. This is something in the desktop notifications that is not yet in the push plugin.

7 Likes

Until push notifications are included in core, I’d prefer to not to do anything that drastic. In the meantime, can anyone tell me if there is a better way of disabling portions of the UI from displaying?

https://github.com/discourse/discourse/pull/5759

This exposes an option that can be called to disable/remove the desktop notifications from the UI. I’ve got a feature branch I’m working on in push notifications to consume it.

I am with @codinghorror here, its mega weird to have 2 different implementations of a feature. Especially when the plugin way is the “correct way” of doing things.

My call here it to just deal with the pain and replace the old way with the new way and get rid of the plugin.

4 Likes