Page MenuHomePhabricator

[web] Add notification permission modal
ClosedPublic

Authored by michal on Feb 21 2023, 9:29 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 20, 2:29 PM
Unknown Object (File)
Tue, Mar 5, 5:01 PM
Unknown Object (File)
Feb 21 2024, 12:50 PM
Unknown Object (File)
Feb 21 2024, 12:50 PM
Unknown Object (File)
Feb 21 2024, 12:50 PM
Unknown Object (File)
Feb 21 2024, 12:50 PM
Unknown Object (File)
Feb 21 2024, 12:50 PM
Unknown Object (File)
Feb 20 2024, 2:04 PM
Subscribers

Details

Summary

Adds a simple modal that asks for permission to display notifications. If user hasn't blocked the notifs yet we will request permission from the browser. If we get it we will then create a new subscription. The modal will be used in the laters diffs.

Test Plan

Check that the user is correctly asked for a permission and the subscription is correctly created.

image.png (454×984 px, 36 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 21 2023, 9:43 AM
Harbormaster failed remote builds in B16716: Diff 22870!

Can you add a screenshot to the Test Plan?

web/modals/push-notif-modal.react.js
1 ↗(On Diff #22922)

Nit: we always have a newline after this

11 ↗(On Diff #22922)

Should this be wrapped in React.memo? Otherwise I think it will always re-render whenever App rerenders

This revision is now accepted and ready to land.Feb 22 2023, 11:23 AM

Add newline and react memo.

ashoat added inline comments.
web/modals/push-notif-modal.react.js
57 ↗(On Diff #23061)

Usually we export default we are not planning to export something else from the file. We can always update it later