[web] Check if notifications are supported
ClosedPublic
Actions

Authored by michal on Apr 6 2023, 8:02 AM.

Details

Reviewers

kamil
inka
• kuba

Commits

rCOMM1be814d0bf66: [web] Check if notifications are supported

Summary

Fix for ENG-3340. Mobile browsers don't all support the web Notifications API. We should add a check for that.

Test Plan

Check in safari on iOS emulator. Before the changes, website displays the error. After the diff it works fine.

Diff Detail

Repository

rCOMM Comm

Branch

michal/stuff1

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

michal created this revision.Apr 6 2023, 8:02 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptApr 6 2023, 8:02 AM

Harbormaster completed remote builds in B18051: Diff 24737.Apr 6 2023, 8:19 AM

michal requested review of this revision.Apr 6 2023, 8:19 AM

inka added inline comments.Apr 11 2023, 12:47 AM

web/push-notif/push-notifs-handler.js
162	Shouldn't this be `\|\| !supported`?

LGTM but check @inka's comment

This revision is now accepted and ready to land.Apr 11 2023, 1:33 AM

Fixed, thanks @inka

Harbormaster completed remote builds in B18214: Diff 24944.Apr 11 2023, 3:12 AM

Closed by commit rCOMM1be814d0bf66: [web] Check if notifications are supported (authored by Michal Gniadek <michal.gniadek@swmansion.com>). · Explain WhyApr 11 2023, 7:04 AM

This revision was automatically updated to reflect the committed changes.

Michal Gniadek <michal.gniadek@swmansion.com> added a commit: rCOMM1be814d0bf66: [web] Check if notifications are supported.

Revision Contents
Changeset List

Path

Size

web/

push-notif/

push-notifs-handler.js

11 lines

Diff 24737

View Options

web/push-notif/push-notifs-handler.js

[web] Check if notifications are supportedClosedPublicActions