Details

Reviewers

ashoat
tomek

Commits

rCOMM18ba2d595d52: [web] Add notification permission modal

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.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

michal created this revision.Feb 21 2023, 9:29 AM

Herald added a subscriber: atul. · View Herald TranscriptFeb 21 2023, 9:29 AM

michal added a parent revision: D6819: [web] Add hook for creating push subscription.Feb 21 2023, 9:29 AM

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!

Rebase

Harbormaster completed remote builds in B16761: Diff 22922.Feb 22 2023, 2:07 AM

michal requested review of this revision.Feb 22 2023, 2:07 AM

michal added a child revision: D6832: [web] Handle notifications in service worker.Feb 22 2023, 2:52 AM

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