sakai
sakai copied to clipboard
SAK-49287 Notifications. Improved push permissions workflow
https://sakaiproject.atlassian.net/browse/SAK-49287
Tasty - I was wondering how we were going to handle this. Folks say 'no' to the "accept notifications" for all sites - so for an LMS they need a second chance. We will need to see if browsers kind of beat us up - they probably have some trick to keep sites from asking over and over.
which path names in particular?
I've updated this to change the ROOT sakai maven module to just be called "root" and not "sakai-root". Also, I've added a button in the notifications window to make it explicit that the user is opting into Sakai notifications.
I've now updated this maven module to be called "serviceworker". No more root.
@adrianfish I think I can take a look at the PR sometime next week, if it's still needed. But just to get some context, when the user does not accept the push notifications, there will be no notifications within the portal neither?
@stetsche Exactly that. Our challenge is inform the user that Sakai notifications are good. Push is so powerful and it is a shame that companies have abused it so heavily.
@adrianfish, okay I understand what the PR is for. But when I enable push notifications, will notifications display only in the portal, or like other push notifications as OS notifications (What a lot of websites are abusing)?
Depending on the browser, both. Chrome and firefox will popup a OS notification as well as adding a notification to the portal area.
Okay, and if you want to have only portal notifications you need to enable them in the browser and block browser notifications on the OS level?
I suppose so. Or we could just disable them in the js. They are explicitly triggered, the os notifications.
Done with this now. I've removed sakai-message-broker.js. Most of the logic moved nicely into sakai-push.utils.js and the remainder I just tacked onto portal.init.js.
This seems to consistently break Cypress tests. I saw lots of errors in my console around theme switching. Maybe theme switcher was depending on something that got moved?
Thanks a ton @stetsche, for the review. And anybody else who reviewed this PR. Much appreciated :)
I think the husky command is failing for windows