refactor: share notification banner subscription and drop unused skip reasons

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(

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(<NotificationPermissionBannerGate />);
+
+    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(<NotificationPermissionBannerGate />);
+
+    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(<NotificationPermissionBannerGate />);
+    await user.click(screen.getByRole("button", { name: "Dismiss" }));
+
+    expect(banner()).not.toBeInTheDocument();
+    unmount();
+
+    render(<NotificationPermissionBannerGate />);
+    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(<NotificationPermissionBannerGate />);
+
+    expect(banner()).toHaveTextContent("is not showing them");
+    expect(
+      screen.queryByRole("button", { name: "Allow notifications" }),
+    ).not.toBeInTheDocument();
+  });
+});

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 <NotificationPermissionBanner kind={kind} onDismiss={handleDismiss} />;
+  return (
+    <NotificationPermissionBanner
+      kind={kind}
+      onDismiss={() => {
+        dismissNotificationBlockedBanner();
+        setDismissed(true);
+      }}
+    />
+  );
 }

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();
   });
 

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<string, number | string | boolean>,
-): void {
-  track("notifications_skipped", { reason, ...extra });
-}
-
-export function trackNotificationScheduled(reminderMinutes: number): void {
-  track("notifications_scheduled", { reminder_minutes: reminderMinutes });
+  sessionBrowserStore.remove(SHOW_FAILURE_SESSION_KEY);
 }

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,
   );

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;

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,

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) => {