Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: iOS notification counts and sums are wacky #1796

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion components/serviceworker.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,10 @@ export const ServiceWorkerProvider = ({ children }) => {
// since (a lot of) browsers don't support the pushsubscriptionchange event,
// we sync with server manually by checking on every page reload if the push subscription changed.
// see https://medium.com/@madridserginho/how-to-handle-webpush-api-pushsubscriptionchange-event-in-modern-browsers-6e47840d756f
navigator?.serviceWorker?.controller?.postMessage?.({ action: STORE_OS, os: detectOS() })
logger.info('sent STORE_OS to service worker: ', detectOS())
navigator?.serviceWorker?.controller?.postMessage?.({ action: SYNC_SUBSCRIPTION })
logger.info('sent SYNC_SUBSCRIPTION to service worker')
navigator?.serviceWorker?.controller?.postMessage?.({ action: STORE_OS, os: detectOS() })
}, [registration, permission.notification])

const contextValue = useMemo(() => ({
Expand Down
24 changes: 16 additions & 8 deletions sw/eventListener.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@ let actionChannelPort

// operating system. the value will be received via a STORE_OS message from app since service workers don't have access to window.navigator
let os = ''
const iOS = () => os === 'iOS'
async function getOS () {
if (!os) {
os = await storage.getItem('os') || ''
}
return os
}

// current push notification count for badge purposes
let activeCount = 0
Expand All @@ -28,6 +33,7 @@ export function onPush (sw) {
if (!payload) return
const { tag } = payload.options
event.waitUntil((async () => {
const iOS = await getOS() === 'iOS'
// generate random ID for every incoming push for better tracing in logs
const nid = crypto.randomUUID()
log(`[sw:push] ${nid} - received notification with tag ${tag}`)
Expand Down Expand Up @@ -56,7 +62,7 @@ export function onPush (sw) {
// close them and then we display the notification.
const notifications = await sw.registration.getNotifications({ tag })
// we only close notifications manually on iOS because we don't want to degrade android UX just because iOS is behind in their support.
if (iOS()) {
if (iOS) {
log(`[sw:push] ${nid} - closing existing notifications`)
notifications.filter(({ tag: nTag }) => nTag === tag).forEach(n => n.close())
}
Expand Down Expand Up @@ -84,7 +90,7 @@ export function onPush (sw) {
// return null
}

return await mergeAndShowNotification(sw, payload, notifications, tag, nid)
return await mergeAndShowNotification(sw, payload, notifications, tag, nid, iOS)
})())
}
}
Expand All @@ -94,7 +100,7 @@ export function onPush (sw) {
const immediatelyShowNotification = (tag) =>
!tag || ['TIP', 'FORWARDEDTIP', 'EARN', 'STREAK', 'TERRITORY_TRANSFER'].includes(tag.split('-')[0])

const mergeAndShowNotification = async (sw, payload, currentNotifications, tag, nid) => {
const mergeAndShowNotification = async (sw, payload, currentNotifications, tag, nid, iOS) => {
// sanity check
const otherTagNotifications = currentNotifications.filter(({ tag: nTag }) => nTag !== tag)
if (otherTagNotifications.length > 0) {
Expand All @@ -119,7 +125,7 @@ const mergeAndShowNotification = async (sw, payload, currentNotifications, tag,
const SUM_SATS_TAGS = ['DEPOSIT', 'WITHDRAWAL']
// this should reflect the amount of notifications that were already merged before
let initialAmount = currentNotifications[0]?.data?.amount || 1
if (iOS()) initialAmount = 1
if (iOS) initialAmount = 1
log(`[sw:push] ${nid} - initial amount: ${initialAmount}`)
const mergedPayload = currentNotifications.reduce((acc, { data }) => {
let newAmount, newSats
Expand Down Expand Up @@ -165,7 +171,7 @@ const mergeAndShowNotification = async (sw, payload, currentNotifications, tag,

// close all current notifications before showing new one to "merge" notifications
// we only do this on iOS because we don't want to degrade android UX just because iOS is behind in their support.
if (iOS()) {
if (iOS) {
log(`[sw:push] ${nid} - closing existing notifications`)
currentNotifications.forEach(n => n.close())
}
Expand Down Expand Up @@ -249,19 +255,21 @@ export function onPushSubscriptionChange (sw) {
}

export function onMessage (sw) {
return (event) => {
return async (event) => {
if (event.data.action === ACTION_PORT) {
actionChannelPort = event.ports[0]
return
}
if (event.data.action === STORE_OS) {
os = event.data.os
event.waitUntil(storage.setItem('os', event.data.os))
Comment on lines -258 to +264
Copy link
Member

Choose a reason for hiding this comment

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

Oh, did simply assigning it to os not work because the service worker does not run in a single scope?

Copy link
Member Author

Choose a reason for hiding this comment

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

You guessed it! But only in iOS (drove me nuts for a whole day)

return
}
if (event.data.action === MESSAGE_PORT) {
messageChannelPort = event.ports[0]
}
log('[sw:message] received message', 'info', { action: event.data.action })
const currentOS = await getOS()
log('[sw:message] stored os: ' + currentOS, 'info', { action: event.data.action })
if (event.data.action === STORE_SUBSCRIPTION) {
log('[sw:message] storing subscription in IndexedDB', 'info', { endpoint: event.data.subscription.endpoint })
return event.waitUntil(storage.setItem('subscription', { ...event.data.subscription, swVersion: 2 }))
Expand Down
Loading