Page MenuHomePhabricator

[web] Add notification permission modal
ClosedPublic

Authored by michal on Feb 21 2023, 9:29 AM.
Tags
None
Referenced Files
F3509735: D6820.diff
Sat, Dec 21, 7:46 AM
Unknown Object (File)
Sat, Dec 14, 2:29 AM
Unknown Object (File)
Nov 13 2024, 2:11 AM
Unknown Object (File)
Nov 6 2024, 2:36 AM
Unknown Object (File)
Nov 6 2024, 2:36 AM
Unknown Object (File)
Nov 6 2024, 2:35 AM
Unknown Object (File)
Nov 6 2024, 2:35 AM
Unknown Object (File)
Nov 6 2024, 2:35 AM
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
No Lint Coverage
Unit
No Test Coverage

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

Nit: we always have a newline after this

11

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