Skip to content

Commit

Permalink
Merge pull request #411 from WalletConnect/refactor/require-notificat…
Browse files Browse the repository at this point in the history
…ion-permission-to-register-device

refactor: update notification device register logics
  • Loading branch information
enesozturk authored Jan 17, 2024
2 parents 5ee26a2 + 8e0d9c7 commit a5a19c3
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 11 deletions.
4 changes: 2 additions & 2 deletions src/Modals.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { useModals } from '@/utils/hooks'
import { useNotificationPermissionState } from '@/utils/hooks/notificationHooks'
import {
checkIfNotificationModalClosed,
notificationsEnabledInBrowser
notificationsAvailableInBrowser
} from '@/utils/notifications'
import { isAppleMobile, isMobileButNotInstalledOnHomeScreen, isNonSafari } from '@/utils/pwa'
import { notificationPwaModalService, signatureModalService } from '@/utils/store'
Expand Down Expand Up @@ -44,7 +44,7 @@ export const Modals = () => {
const shouldShowPreferencesModalOpen = isPreferencesModalOpen && !shouldShowChangeBrowserModal

const shouldShowNotificationModal =
notificationsEnabledInBrowser() &&
notificationsAvailableInBrowser() &&
!explicitlyDeniedOnDesktop &&
!isMobileButNotInstalledOnHomeScreen &&
!notificationsEnabled &&
Expand Down
4 changes: 2 additions & 2 deletions src/components/settings/NotificationsSettings/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import SettingsContext from '@/contexts/SettingsContext/context'
import W3iContext from '@/contexts/W3iContext/context'
import { useNotificationPermissionState } from '@/utils/hooks/notificationHooks'
import { getDbEchoRegistrations } from '@/utils/idb'
import { notificationsEnabledInBrowser, requireNotifyPermission } from '@/utils/notifications'
import { notificationsAvailableInBrowser, requireNotifyPermission } from '@/utils/notifications'
import { showSuccessMessageToast } from '@/utils/toasts'

import SettingsHeader from '../SettingsHeader'
Expand Down Expand Up @@ -131,7 +131,7 @@ const NotificationsSettings: React.FC = () => {
setChecked={handleEnableNotifications}
icon={<NotificationIcon />}
title="Enable Push Notifications"
active={notificationsEnabledInBrowser() && !notificationsEnabled}
active={notificationsAvailableInBrowser() && !notificationsEnabled}
/>
</SettingsItem>
</div>
Expand Down
6 changes: 3 additions & 3 deletions src/utils/notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ export const checkIfNotificationModalClosed = () => {
return storageValue === 'true'
}

export const notificationsEnabledInBrowser = () => {
export const notificationsAvailableInBrowser = () => {
return 'Notification' in window
}

export const userEnabledNotification = () => {
if (notificationsEnabledInBrowser()) {
if (notificationsAvailableInBrowser()) {
return window.Notification?.permission === 'granted'
}
return false
Expand All @@ -89,7 +89,7 @@ export const userEnabledNotification = () => {
* Returns true if permissions were granted
*/
export const requireNotifyPermission = async () => {
if (!notificationsEnabledInBrowser()) {
if (!notificationsAvailableInBrowser()) {
console.warn('This browser does not support desktop push notifications')
return false
}
Expand Down
13 changes: 9 additions & 4 deletions src/w3iProxy/notifyProviders/internalNotifyProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import mixpanel from 'mixpanel-browser'
import { logError } from '@/utils/error'
import { getDbEchoRegistrations } from '@/utils/idb'
import {
notificationsEnabledInBrowser,
notificationsAvailableInBrowser,
registerWithEcho,
requireNotifyPermission,
setupSubscriptionsSymkeys,
userEnabledNotification
} from '@/utils/notifications'
Expand Down Expand Up @@ -78,10 +79,14 @@ export default class InternalNotifyProvider implements W3iNotifyProvider {
return
}

if (notificationsEnabledInBrowser()) {
await window.Notification?.requestPermission()
const notificationsAvailabe = notificationsAvailableInBrowser()

await registerWithEcho(this.notifyClient)
if (notificationsAvailabe) {
const notificationsGranted = await requireNotifyPermission()

if (notificationsGranted) {
await registerWithEcho(this.notifyClient)
}
}
}

Expand Down

1 comment on commit a5a19c3

@vercel
Copy link

@vercel vercel bot commented on a5a19c3 Jan 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.