mattermost
mattermost copied to clipboard
mattermost
MM-52219 Add announcement bar to prompt users for notification permission
Summary
This is a fix for the issue where some browsers prevent us from prompting for notification permission because the app tries to notify the user when the browser isn't focused.
My original plan was for something fancier where we'd wait until the user could've received a notification and then we prompt them for permission when the app next gets focus, but that ended up getting overly complicated, and it changed the existing code which had me worried that something might break.
Instead, I went for the super simple solution of just showing a "give us permission" banner as soon as the app opens like other apps do. This might be mildly annoying to people, but it's simple and hopefully effective enough to do the trick. It also leaves the existing code path in place so, if someone ignores the banner, they'll still get the same permission prompt that we previously trigger.
Ticket Link
MM-52219
Release Note
Added banner to prompt users to give desktop notification permission when opening the app
E2E tests not automatically triggered, because the PR is not in a mergeable state. Please update the branch with the base branch and resolve outstanding conflicts.
@hmhealey: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.
I understand the commands that are listed here
@hmhealey: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.
I understand the commands that are listed here
@hmhealey: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.
I understand the commands that are listed here
@hmhealey: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.
I understand the commands that are listed here
@abhijit-singh Let me know if there's any changes you'd want to see from a user perspective, particularly around the timing the bar appears, the text on the bar, and whether or not we should have a "don't ask me again" button. I didn't add that last one because it seemed redundant with just denying permission, but I'm 0/5 on that.
Mattermost test server created! :tada:
Access here: https://mattermost-pr-26849.test.mattermost.cloud
| Account Type | Username | Password |
|---|---|---|
| Admin | sysadmin | Sys@dmin123 |
| User | user-1 | User-1@123 |
Your Spinwick's installation ID is: q93h84dg5384ucoiw18fi3chgo
To access the logs, please click here
@hmhealey if I reject the permission, i.e, disallow it, the notification banner never shows up. I just want to confirm this is intentional.
@hmhealey if I reject the permission, i.e, disallow it, the notification banner never shows up. I just want to confirm this is intentional.
Yeah, that's intended. The browser also prevents you from showing the permission prompt if the user previously denied it as well, so there's no point in prompting the user at the moment. This PR is more to ensure that users see the initial prompt rather than to get them to give us permission after they've denied it.
I think we plan to show that permission was denied in the account settings in the future, but that's separate from this
Oh right. Sorry, I totally forgot about that case since you definitely mentioned that before
@abhijit-singh I ended up just removing the z-index from that component entirely since, as far as I can tell, that hasn't been needed for a long time. I think that's a remnant of when the announcement bar used to sometimes overlap the app, and it's no longer needed ever since Caleb made the whole app use CSS grid a couple years ago
@hmhealey If user blocks this and reloads browser. The banner won't show again. Is this expected?
- If we allow it from here. The notifications are on, but the banner persists. Unless we reload.
Yeah, those are both expected.
For the first one, we can't prompt the user to change their permission if they've previously blocked it, so I don't bother showing the bar.
For the second one, we can't detect that the user changed their permissions from outside the web app, so it's normal that the bar doesn't disappear unless the user specifically clicks on "Enable notifications" on it. From my testing, that also matches how Slack behaves
New commit detected. SpinWick will upgrade if the updated docker image is available.
Mattermost test server updated with git commit 19744de862eb68bbf2d0fe53b92e4a11bb8cd642.
Access here: https://mattermost-pr-26849.test.mattermost.cloud