From 8b926e30426078b7139b6a19211991b08b73a4b6 Mon Sep 17 00:00:00 2001 From: katspaugh Date: Mon, 20 Nov 2023 11:18:00 +0100 Subject: [PATCH 1/3] Fix: delay the notifications banner --- .../PushNotifications/PushNotificationsBanner/index.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx b/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx index 9bf3c4cff8..e7ae9ccbff 100644 --- a/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx +++ b/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx @@ -27,7 +27,7 @@ import type { PushNotificationPreferences } from '@/services/push-notifications/ import type { NotifiableSafes } from '../logic' import useWallet from '@/hooks/wallets/useWallet' import CircularProgress from '@mui/material/CircularProgress' - +import useDebounce from '@/hooks/useDebounce' import css from './styles.module.css' const DISMISS_PUSH_NOTIFICATIONS_KEY = 'dismissPushNotifications' @@ -119,8 +119,10 @@ export const PushNotificationsBanner = ({ children }: { children: ReactElement } const isSafeAdded = !!addedSafesOnChain?.[safeAddress] const isSafeRegistered = getPreferences(safe.chainId, safeAddress) - const shouldShowBanner = - isNotificationFeatureEnabled && !isPushNotificationBannerDismissed && isSafeAdded && !isSafeRegistered && !!wallet + const shouldShowBanner = useDebounce( + isNotificationFeatureEnabled && !isPushNotificationBannerDismissed && isSafeAdded && !isSafeRegistered && !!wallet, + 3000, + ) const { registerNotifications } = useNotificationRegistrations() From fcfedb54188c0d6f5eefa8a91ca7daeeef2d892c Mon Sep 17 00:00:00 2001 From: katspaugh Date: Mon, 20 Nov 2023 12:12:53 +0100 Subject: [PATCH 2/3] Fix tests --- .../PushNotificationsBanner.test.tsx | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/components/settings/PushNotifications/PushNotificationsBanner/PushNotificationsBanner.test.tsx b/src/components/settings/PushNotifications/PushNotificationsBanner/PushNotificationsBanner.test.tsx index 6ea40fb386..4d8a2ffb19 100644 --- a/src/components/settings/PushNotifications/PushNotificationsBanner/PushNotificationsBanner.test.tsx +++ b/src/components/settings/PushNotifications/PushNotificationsBanner/PushNotificationsBanner.test.tsx @@ -7,7 +7,7 @@ import type { ChainInfo, SafeInfo } from '@safe-global/safe-gateway-typescript-s import { PushNotificationsBanner, _getSafesToRegister } from '.' import { createPushNotificationPrefsIndexedDb } from '@/services/push-notifications/preferences' -import { render } from '@/tests/test-utils' +import { act, render } from '@/tests/test-utils' import type { AddedSafesOnChain } from '@/store/addedSafesSlice' import type { PushNotificationPreferences } from '@/services/push-notifications/preferences' import * as useWallet from '@/hooks/wallets/useWallet' @@ -28,6 +28,10 @@ jest.spyOn(useWallet, 'default').mockImplementation(() => ({ })) describe('PushNotificationsBanner', () => { + beforeAll(() => { + jest.useFakeTimers() + }) + describe('getSafesToRegister', () => { it('should return all added safes if no preferences exist', () => { const addedSafesOnChain = { @@ -152,6 +156,7 @@ describe('PushNotificationsBanner', () => { expect(tracking.trackEvent).toHaveBeenCalledTimes(1) }) + it('should display the banner', () => { const result = render( @@ -193,6 +198,8 @@ describe('PushNotificationsBanner', () => { }, ) + jest.advanceTimersByTime(3000) + expect(result.getByText('Get notified about pending signatures', { exact: false })).toBeInTheDocument() }) @@ -240,7 +247,7 @@ describe('PushNotificationsBanner', () => { expect(result.queryByText('Get notified about pending signatures', { exact: false })).not.toBeInTheDocument() }) - it('should not show the banner if the user has dismissed it', () => { + it('should not show the banner if the user has dismissed it', async () => { window.localStorage.setItem( 'SAFE_v2__dismissPushNotifications', JSON.stringify({ '1': { [hexZeroPad('0x123', 20)]: true } }), @@ -281,6 +288,11 @@ describe('PushNotificationsBanner', () => { }, ) + await act(() => { + jest.advanceTimersByTime(3000) + return Promise.resolve() + }) + expect(result.queryByText('Get notified about pending signatures', { exact: false })).not.toBeInTheDocument() }) From 3165deb2a4eeaf70c45c831c0911c67f8b6f314a Mon Sep 17 00:00:00 2001 From: katspaugh Date: Mon, 20 Nov 2023 20:03:43 +0100 Subject: [PATCH 3/3] Fix: close immediately --- .../PushNotifications/PushNotificationsBanner/index.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx b/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx index e7ae9ccbff..0380e3e5d6 100644 --- a/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx +++ b/src/components/settings/PushNotifications/PushNotificationsBanner/index.tsx @@ -31,6 +31,7 @@ import useDebounce from '@/hooks/useDebounce' import css from './styles.module.css' const DISMISS_PUSH_NOTIFICATIONS_KEY = 'dismissPushNotifications' +const BANNER_DELAY = 3000 export const useDismissPushNotificationsBanner = () => { const addedSafes = useAppSelector(selectAllAddedSafes) @@ -121,7 +122,7 @@ export const PushNotificationsBanner = ({ children }: { children: ReactElement } const isSafeRegistered = getPreferences(safe.chainId, safeAddress) const shouldShowBanner = useDebounce( isNotificationFeatureEnabled && !isPushNotificationBannerDismissed && isSafeAdded && !isSafeRegistered && !!wallet, - 3000, + BANNER_DELAY, ) const { registerNotifications } = useNotificationRegistrations() @@ -166,7 +167,7 @@ export const PushNotificationsBanner = ({ children }: { children: ReactElement } dismissBanner() } - if (!shouldShowBanner) { + if (!shouldShowBanner || isPushNotificationBannerDismissed) { return children }