From 64c24d073c0d335ba2b17293bd0f41bb4ab2e752 Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Thu, 7 Aug 2025 10:15:04 -0400 Subject: [PATCH] docs(notification-processing): [PM-19877] System Notification Implementation - Addressed more feedback. I think it's good, going to pull into other branch to test --- .../browser/src/background/main.background.ts | 3 +- .../src/platform/browser/from-chrome-event.ts | 6 +- .../browser-system-notification.service.ts | 112 ++++++++---------- .../default-server-notifications.service.ts | 2 + 4 files changed, 53 insertions(+), 70 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 4d3e3b25165..6aa2d3c2c62 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -1127,7 +1127,7 @@ export default class MainBackground { this.actionsService = new BrowserActionsService(this.logService, this.platformUtilsService); - if ("notifications" in chrome || Notification) { + if ("notifications" in chrome) { this.systemNotificationService = new BrowserSystemNotificationService( this.logService, this.platformUtilsService, @@ -1723,6 +1723,7 @@ export default class MainBackground { /** * Opens the popup to the given page + * * @default ExtensionPageUrls.Index * @deprecated Migrating to the browser actions service. */ diff --git a/apps/browser/src/platform/browser/from-chrome-event.ts b/apps/browser/src/platform/browser/from-chrome-event.ts index 28e57f58132..e0cd7a10b83 100644 --- a/apps/browser/src/platform/browser/from-chrome-event.ts +++ b/apps/browser/src/platform/browser/from-chrome-event.ts @@ -1,5 +1,3 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Observable } from "rxjs"; import { BrowserApi } from "./browser-api"; @@ -26,13 +24,13 @@ export function fromChromeEvent( event: chrome.events.Event<(...args: T) => void>, ): Observable { return new Observable((subscriber) => { - const handler = (...args: T) => { + const handler = (...args: readonly unknown[]) => { if (chrome.runtime.lastError) { subscriber.error(chrome.runtime.lastError); return; } - subscriber.next(args); + subscriber.next(args as T); }; BrowserApi.addListener(event, handler); diff --git a/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts b/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts index 1b55cc12707..e41609a50be 100644 --- a/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts +++ b/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts @@ -1,7 +1,6 @@ -import { Observable, Subject } from "rxjs"; +import { map, merge, Observable } from "rxjs"; import { v4 as uuidv4 } from "uuid"; -import { DeviceType } from "@bitwarden/common/enums"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { @@ -12,75 +11,67 @@ import { SystemNotificationsService, } from "@bitwarden/common/platform/notifications/system-notifications.service"; +import { fromChromeEvent } from "../browser/from-chrome-event"; + export class BrowserSystemNotificationService implements SystemNotificationsService { - private systemNotificationClickedSubject = new Subject(); notificationClicked$: Observable; constructor( private logService: LogService, private platformUtilsService: PlatformUtilsService, ) { - this.notificationClicked$ = this.systemNotificationClickedSubject.asObservable(); + this.notificationClicked$ = merge( + fromChromeEvent(chrome.notifications.onButtonClicked).pipe( + map(([notificationId, buttonIndex]) => ({ + id: notificationId, + buttonIdentifier: buttonIndex, + })), + ), + fromChromeEvent(chrome.notifications.onClicked).pipe( + map(([notificationId]: [string]) => ({ + id: notificationId, + buttonIdentifier: ButtonLocation.NotificationButton, + })), + ), + ); } async create(createInfo: SystemNotificationCreateInfo): Promise { try { const notificationId = createInfo.id || uuidv4(); - switch (this.platformUtilsService.getDevice()) { - case DeviceType.ChromeExtension: - chrome.notifications.create(notificationId, { - iconUrl: "https://avatars.githubusercontent.com/u/15990069?s=200", - message: createInfo.body, - type: "basic", - title: createInfo.title, - buttons: createInfo.buttons.map((value) => { - return { title: value.title }; + + chrome.notifications.create(notificationId, { + iconUrl: "https://avatars.githubusercontent.com/u/15990069?s=200", + message: createInfo.body, + type: "basic", + title: createInfo.title, + buttons: createInfo.buttons.map((value) => { + return { title: value.title }; + }), + }); + + // eslint-disable-next-line no-restricted-syntax + chrome.notifications.onButtonClicked.addListener( + (notificationId: string, buttonIndex: number) => { + this.notificationClicked$.subscribe({ + next: () => ({ + id: notificationId, + buttonIdentifier: buttonIndex, }), }); + }, + ); - // eslint-disable-next-line no-restricted-syntax - chrome.notifications.onButtonClicked.addListener( - (notificationId: string, buttonIndex: number) => { - this.systemNotificationClickedSubject.next({ - id: notificationId, - buttonIdentifier: buttonIndex, - }); - }, - ); + // eslint-disable-next-line no-restricted-syntax + chrome.notifications.onClicked.addListener((notificationId: string) => { + this.notificationClicked$.subscribe({ + next: () => ({ + id: notificationId, + buttonIdentifier: ButtonLocation.NotificationButton, + }), + }); + }); - // eslint-disable-next-line no-restricted-syntax - chrome.notifications.onClicked.addListener((notificationId: string) => { - this.systemNotificationClickedSubject.next({ - id: notificationId, - buttonIdentifier: ButtonLocation.NotificationButton, - }); - }); - - break; - case DeviceType.FirefoxExtension: - await browser.notifications.create(notificationId, { - iconUrl: "https://avatars.githubusercontent.com/u/15990069?s=200", - message: createInfo.title, - type: "basic", - title: createInfo.title, - }); - - browser.notifications.onButtonClicked.addListener( - (notificationId: string, buttonIndex: number) => { - this.systemNotificationClickedSubject.next({ - id: notificationId, - buttonIdentifier: buttonIndex, - }); - }, - ); - - browser.notifications.onClicked.addListener((notificationId: string) => { - this.systemNotificationClickedSubject.next({ - id: notificationId, - buttonIdentifier: ButtonLocation.NotificationButton, - }); - }); - } return notificationId; } catch (e) { this.logService.error( @@ -94,15 +85,6 @@ export class BrowserSystemNotificationService implements SystemNotificationsServ } isSupported(): boolean { - switch (this.platformUtilsService.getDevice()) { - case DeviceType.FirefoxExtension: - case DeviceType.EdgeExtension: - case DeviceType.VivaldiExtension: - case DeviceType.OperaExtension: - case DeviceType.ChromeExtension: - return true; - default: - return false; - } + return "notifications" in chrome; } } diff --git a/libs/common/src/platform/notifications/internal/default-server-notifications.service.ts b/libs/common/src/platform/notifications/internal/default-server-notifications.service.ts index 1118dd0e632..09657d3908c 100644 --- a/libs/common/src/platform/notifications/internal/default-server-notifications.service.ts +++ b/libs/common/src/platform/notifications/internal/default-server-notifications.service.ts @@ -213,6 +213,8 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer await this.syncService.syncDeleteSend(notification.payload as SyncSendNotification); break; case NotificationType.AuthRequest: + // create notification + this.messagingService.send("openLoginApproval", { notificationId: notification.payload.id, });