diff --git a/apps/calendar-macos/CompassUI/Sources/CompassUI/TimeGrid/TimeGridView.swift b/apps/calendar-macos/CompassUI/Sources/CompassUI/TimeGrid/TimeGridView.swift index 0db9177ca..240baa6c7 100644 --- a/apps/calendar-macos/CompassUI/Sources/CompassUI/TimeGrid/TimeGridView.swift +++ b/apps/calendar-macos/CompassUI/Sources/CompassUI/TimeGrid/TimeGridView.swift @@ -311,7 +311,7 @@ public final class TimeGridView: NSView { timedContentView.frame = NSRect(x: 0, y: 0, width: contentWidth, height: gridHeight) syncPinnedHeaderScrollX() - renderHourGutter(snapshot: snapshot, hourHeight: hourHeight, gridHeight: gridHeight) + renderHourGutter(snapshot: snapshot, hourHeight: hourHeight) renderHeaders(snapshot: snapshot, headerHeight: headerHeight) renderAllDayColumns(snapshot: snapshot, allDayHeight: allDayHeight, headerHeight: headerHeight) renderTimedGrid(snapshot: snapshot, hourHeight: hourHeight, gridHeight: gridHeight) @@ -321,8 +321,7 @@ public final class TimeGridView: NSView { private func renderHourGutter( snapshot: GridLayoutSnapshot, - hourHeight: Double, - gridHeight: Double + hourHeight: Double ) { hourGutterView.subviews.forEach { $0.removeFromSuperview() } let palette = themePalette @@ -356,7 +355,6 @@ public final class TimeGridView: NSView { hourHeight: hourHeight, palette: palette, marginWidth: margin) - _ = gridHeight } private func renderHourColumn( diff --git a/apps/calendar-web/src/notifications/NotificationPermissionBannerGate.test.tsx b/apps/calendar-web/src/notifications/NotificationPermissionBannerGate.test.tsx new file mode 100644 index 000000000..385e86005 --- /dev/null +++ b/apps/calendar-web/src/notifications/NotificationPermissionBannerGate.test.tsx @@ -0,0 +1,97 @@ +import { cleanup, render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { createTestNotificationPort } from "@web/__tests__/helpers/web-test-seams"; +import { STORAGE_KEYS } from "@web/common/constants/storage.constants"; +import { persistentBrowserStore } from "@web/common/storage/browser-key-value.store"; +import { NotificationPermissionBannerGate } from "@web/notifications/NotificationPermissionBannerGate"; +import { + registerNotificationPort, + resetNotificationPort, +} from "@web/notifications/notification.port"; +import { + recordNotificationShowFailure, + resetNotificationShowFailureCount, +} from "@web/notifications/notification-diagnostics"; +import { afterEach, beforeEach, describe, expect, it } from "bun:test"; +import "@testing-library/jest-dom"; + +const banner = () => + screen.queryByRole("region", { name: "Event notifications blocked" }); + +describe("NotificationPermissionBannerGate", () => { + beforeEach(() => { + persistentBrowserStore.remove(STORAGE_KEYS.NOTIFICATIONS_ENABLED); + persistentBrowserStore.remove( + STORAGE_KEYS.NOTIFICATIONS_BLOCKED_BANNER_DISMISSED, + ); + resetNotificationShowFailureCount(); + resetNotificationPort(); + }); + + afterEach(() => { + cleanup(); + resetNotificationPort(); + resetNotificationShowFailureCount(); + persistentBrowserStore.remove(STORAGE_KEYS.NOTIFICATIONS_ENABLED); + persistentBrowserStore.remove( + STORAGE_KEYS.NOTIFICATIONS_BLOCKED_BANNER_DISMISSED, + ); + }); + + it("stays hidden when notifications are not opted in", () => { + registerNotificationPort( + createTestNotificationPort({ permission: "granted" }).port, + ); + + render(); + + expect(banner()).not.toBeInTheDocument(); + }); + + it("prompts for permission when the pref is on but the grant is missing", () => { + persistentBrowserStore.set(STORAGE_KEYS.NOTIFICATIONS_ENABLED, "true"); + registerNotificationPort( + createTestNotificationPort({ permission: "default" }).port, + ); + + render(); + + expect(banner()).toHaveTextContent("has not granted permission"); + expect( + screen.getByRole("button", { name: "Allow notifications" }), + ).toBeInTheDocument(); + }); + + it("hides after dismiss and stays hidden on remount", async () => { + const user = userEvent.setup(); + persistentBrowserStore.set(STORAGE_KEYS.NOTIFICATIONS_ENABLED, "true"); + registerNotificationPort( + createTestNotificationPort({ permission: "default" }).port, + ); + + const { unmount } = render(); + await user.click(screen.getByRole("button", { name: "Dismiss" })); + + expect(banner()).not.toBeInTheDocument(); + unmount(); + + render(); + expect(banner()).not.toBeInTheDocument(); + }); + + it("warns when the browser keeps dropping constructed notifications", () => { + persistentBrowserStore.set(STORAGE_KEYS.NOTIFICATIONS_ENABLED, "true"); + registerNotificationPort( + createTestNotificationPort({ permission: "granted" }).port, + ); + recordNotificationShowFailure(); + recordNotificationShowFailure(); + + render(); + + expect(banner()).toHaveTextContent("is not showing them"); + expect( + screen.queryByRole("button", { name: "Allow notifications" }), + ).not.toBeInTheDocument(); + }); +}); diff --git a/apps/calendar-web/src/notifications/NotificationPermissionBannerGate.tsx b/apps/calendar-web/src/notifications/NotificationPermissionBannerGate.tsx index fbb4e7421..d747d3a96 100644 --- a/apps/calendar-web/src/notifications/NotificationPermissionBannerGate.tsx +++ b/apps/calendar-web/src/notifications/NotificationPermissionBannerGate.tsx @@ -1,52 +1,31 @@ -import { useCallback, useSyncExternalStore } from "react"; -import { STORAGE_KEYS } from "@web/common/constants/storage.constants"; -import { subscribeToStorageKey } from "@web/common/utils/external-store.util"; +import { useState, useSyncExternalStore } from "react"; import { NotificationPermissionBanner } from "@web/notifications/NotificationPermissionBanner"; -import { getNotificationPort } from "@web/notifications/notification.port"; +import { subscribeToNotificationState } from "@web/notifications/notification.state"; import { dismissNotificationBlockedBanner, + hasDismissedNotificationBlockedBanner, pickNotificationBlockedBannerKind, } from "@web/notifications/notification-blocked-banner.util"; -const subscribe = (onChange: () => void) => { - const unstorage = subscribeToStorageKey( - STORAGE_KEYS.NOTIFICATIONS_ENABLED, - onChange, - ); - const unblocked = subscribeToStorageKey( - STORAGE_KEYS.NOTIFICATIONS_BLOCKED_BANNER_DISMISSED, - onChange, - ); - const unobserve = getNotificationPort().observePermission(onChange); - const onVisibilityChange = () => { - if (document.visibilityState === "visible") onChange(); - }; - document.addEventListener("visibilitychange", onVisibilityChange); - window.addEventListener("focus", onChange); - window.addEventListener("storage", onChange); - - return () => { - unstorage(); - unblocked(); - unobserve(); - document.removeEventListener("visibilitychange", onVisibilityChange); - window.removeEventListener("focus", onChange); - window.removeEventListener("storage", onChange); - }; -}; - export function NotificationPermissionBannerGate() { const kind = useSyncExternalStore( - subscribe, + subscribeToNotificationState, pickNotificationBlockedBannerKind, () => null, ); + const [dismissed, setDismissed] = useState( + hasDismissedNotificationBlockedBanner, + ); - const handleDismiss = useCallback(() => { - dismissNotificationBlockedBanner(); - }, []); - - if (!kind) return null; + if (!kind || dismissed) return null; - return ; + return ( + { + dismissNotificationBlockedBanner(); + setDismissed(true); + }} + /> + ); } diff --git a/apps/calendar-web/src/notifications/notification-blocked-banner.util.test.ts b/apps/calendar-web/src/notifications/notification-blocked-banner.util.test.ts index b7c96166c..06b0799dd 100644 --- a/apps/calendar-web/src/notifications/notification-blocked-banner.util.test.ts +++ b/apps/calendar-web/src/notifications/notification-blocked-banner.util.test.ts @@ -22,7 +22,6 @@ describe("pickNotificationBlockedBannerKind", () => { STORAGE_KEYS.NOTIFICATIONS_BLOCKED_BANNER_DISMISSED, ); resetNotificationShowFailureCount(); - if (typeof sessionStorage !== "undefined") sessionStorage.clear(); resetNotificationPort(); }); diff --git a/apps/calendar-web/src/notifications/notification-diagnostics.ts b/apps/calendar-web/src/notifications/notification-diagnostics.ts index fa0b5ed75..ada4a03f0 100644 --- a/apps/calendar-web/src/notifications/notification-diagnostics.ts +++ b/apps/calendar-web/src/notifications/notification-diagnostics.ts @@ -1,60 +1,21 @@ -import { track } from "@web/auth/posthog/track"; -import { STORAGE_KEYS } from "@web/common/constants/storage.constants"; -import { persistentBrowserStore } from "@web/common/storage/browser-key-value.store"; -import { getNotificationPort } from "@web/notifications/notification.port"; - -const isNotificationsPrefEnabled = (): boolean => - persistentBrowserStore.get(STORAGE_KEYS.NOTIFICATIONS_ENABLED) === "true"; - -export type NotificationSkipReason = - | "not_effective" - | "no_reminders" - | "already_fired" - | "not_due" - | "show_failed"; +import { sessionBrowserStore } from "@web/common/storage/browser-key-value.store"; const SHOW_FAILURE_SESSION_KEY = "compass.notifications.session-show-failures"; export function readNotificationShowFailureCount(): number { - if (typeof sessionStorage === "undefined") return 0; - const raw = sessionStorage.getItem(SHOW_FAILURE_SESSION_KEY); - const parsed = raw ? Number.parseInt(raw, 10) : 0; + const parsed = Number.parseInt( + sessionBrowserStore.get(SHOW_FAILURE_SESSION_KEY) ?? "", + 10, + ); return Number.isFinite(parsed) && parsed > 0 ? parsed : 0; } export function recordNotificationShowFailure(): number { - if (typeof sessionStorage === "undefined") return 0; const next = readNotificationShowFailureCount() + 1; - sessionStorage.setItem(SHOW_FAILURE_SESSION_KEY, String(next)); + sessionBrowserStore.set(SHOW_FAILURE_SESSION_KEY, String(next)); return next; } export function resetNotificationShowFailureCount(): void { - if (typeof sessionStorage === "undefined") return; - sessionStorage.removeItem(SHOW_FAILURE_SESSION_KEY); -} - -export function trackNotificationPermissionState( - source: "palette" | "runtime", -): void { - const port = getNotificationPort(); - track("notifications_permission", { - source, - pref_enabled: isNotificationsPrefEnabled(), - permission: port.getPermission(), - supported: port.isSupported(), - effectively_on: - isNotificationsPrefEnabled() && port.getPermission() === "granted", - }); -} - -export function trackNotificationSkipped( - reason: NotificationSkipReason, - extra?: Record, -): void { - track("notifications_skipped", { reason, ...extra }); -} - -export function trackNotificationScheduled(reminderMinutes: number): void { - track("notifications_scheduled", { reminder_minutes: reminderMinutes }); + sessionBrowserStore.remove(SHOW_FAILURE_SESSION_KEY); } diff --git a/apps/calendar-web/src/notifications/notification.state.ts b/apps/calendar-web/src/notifications/notification.state.ts index 567b74003..e33a5fb1e 100644 --- a/apps/calendar-web/src/notifications/notification.state.ts +++ b/apps/calendar-web/src/notifications/notification.state.ts @@ -9,7 +9,6 @@ import { getNotificationPort, type NotificationPort, } from "@web/notifications/notification.port"; -import { trackNotificationPermissionState } from "@web/notifications/notification-diagnostics"; /** Where a toggle came from for analytics. */ export type NotificationToggleSource = "palette"; @@ -82,6 +81,10 @@ const startExternalSubscriptions = (): (() => void) => { STORAGE_KEYS.NOTIFICATIONS_ENABLED, emit, ); + const unblocked = subscribeToStorageKey( + STORAGE_KEYS.NOTIFICATIONS_BLOCKED_BANNER_DISMISSED, + emit, + ); const onVisibilityChange = () => { if (document.visibilityState === "visible") emit(); }; @@ -91,12 +94,15 @@ const startExternalSubscriptions = (): (() => void) => { return () => { unobserve(); unstorage(); + unblocked(); document.removeEventListener("visibilitychange", onVisibilityChange); window.removeEventListener("focus", emit); }; }; -const subscribe = (onChange: () => void): (() => void) => { +export const subscribeToNotificationState = ( + onChange: () => void, +): (() => void) => { listeners.add(onChange); if (subscriberCount === 0) { stopExternal = startExternalSubscriptions(); @@ -113,9 +119,22 @@ const subscribe = (onChange: () => void): (() => void) => { }; }; +export function trackNotificationPermissionState( + source: NotificationToggleSource | "runtime", +): void { + const port = getNotificationPort(); + track("notifications_permission", { + source, + pref_enabled: isNotificationsPrefEnabled(), + permission: port.getPermission(), + supported: port.isSupported(), + effectively_on: areNotificationsEffectivelyOn(), + }); +} + export function useNotificationsEffectivelyOn(): boolean { return useSyncExternalStore( - subscribe, + subscribeToNotificationState, areNotificationsEffectivelyOn, () => false, ); diff --git a/apps/calendar-web/src/notifications/upcoming-notifier.logic.ts b/apps/calendar-web/src/notifications/upcoming-notifier.logic.ts index 87c90d062..bc71b0a87 100644 --- a/apps/calendar-web/src/notifications/upcoming-notifier.logic.ts +++ b/apps/calendar-web/src/notifications/upcoming-notifier.logic.ts @@ -12,8 +12,6 @@ import { type NotificationPort } from "@web/notifications/notification.port"; import { recordNotificationShowFailure, resetNotificationShowFailureCount, - trackNotificationScheduled, - trackNotificationSkipped, } from "@web/notifications/notification-diagnostics"; import { inEffectiveTimeZone } from "@web/timezone/in-time-zone"; import { dayEventQueryRange } from "@web/views/Day/util/day-window.util"; @@ -145,7 +143,7 @@ export function announceUpcomingEvents( const announced = pruneFiredKeys(firedKeys, now); for (const { event, reminderMinutes } of due) { const key = notificationKey(event, reminderMinutes); - trackNotificationScheduled(reminderMinutes); + track("notifications_scheduled", { reminder_minutes: reminderMinutes }); const shown = showUpcomingEventNotification( port, event, @@ -161,7 +159,7 @@ export function announceUpcomingEvents( } // A silent browser drop must not burn the key: the next tick retries. recordNotificationShowFailure(); - trackNotificationSkipped("show_failed"); + track("notifications_skipped", { reason: "show_failed" }); track("notifications_show_failed"); } return announced; diff --git a/apps/calendar-web/src/notifications/useUpcomingEventNotifier.ts b/apps/calendar-web/src/notifications/useUpcomingEventNotifier.ts index 2e81dec4f..e8e197424 100644 --- a/apps/calendar-web/src/notifications/useUpcomingEventNotifier.ts +++ b/apps/calendar-web/src/notifications/useUpcomingEventNotifier.ts @@ -4,9 +4,9 @@ import { useTodayTimedEvents } from "@web/components/Sidebar/UpNextCard/useUpNex import { getNotificationPort } from "@web/notifications/notification.port"; import { isNotificationsPrefEnabled, + trackNotificationPermissionState, useNotificationsEffectivelyOn, } from "@web/notifications/notification.state"; -import { trackNotificationPermissionState } from "@web/notifications/notification-diagnostics"; import { announceUpcomingEvents, toNotifiableEvents, diff --git a/packages/sync/src/domain/event-instance-assembly.ts b/packages/sync/src/domain/event-instance-assembly.ts index cbecdb660..580f5c7a9 100644 --- a/packages/sync/src/domain/event-instance-assembly.ts +++ b/packages/sync/src/domain/event-instance-assembly.ts @@ -160,19 +160,10 @@ const toPopupReminderMinutes = ( event: EventRecord, seriesMaster?: EventRecord, ): { popupReminderMinutes?: readonly number[] } => { - const own = reminderMinutesFromProviderMetadata(event.providerMetadata); - if (own !== undefined) { - return { popupReminderMinutes: own }; - } - if (seriesMaster) { - const inherited = reminderMinutesFromProviderMetadata( - seriesMaster.providerMetadata, - ); - if (inherited !== undefined) { - return { popupReminderMinutes: inherited }; - } - } - return {}; + const minutes = + reminderMinutesFromProviderMetadata(event.providerMetadata) ?? + reminderMinutesFromProviderMetadata(seriesMaster?.providerMetadata); + return minutes !== undefined ? { popupReminderMinutes: minutes } : {}; }; const toInstanceContent = (event: EventRecord) => {