feat: major headlines push notifications

[nimrodkra]

Summary

• Adds featureMajorHeadlinesPush flag and three opt-in surfaces for Major Headlines alerts: notification settings ("Happening Now" section), /highlights banner, and a 3-dots menu on in-feed Happening Now cards.
• Introduces shared useMajorHeadlinesSubscription hook that composes push permission + notification settings as the single source of truth across surfaces.
• Persistent banner dismissal via new ActionType.DismissedMajorHeadlinesAlertsBanner. Adds LogEvents with origin attribution (settings, highlights_page, feed_card).

Test plan

• With flag on and logged out: no banner, no card menu, no settings row
• With flag on and logged in (not subscribed): banner shows on /highlights, card menu shows "Get real-time alerts", settings row shows muted state
• Click "Turn on alerts": browser permission prompt → toast "You'll be the first to know when news breaks." → banner hides, card menu flips to "Turn off real-time alerts", settings flips to subscribed
• Dismiss banner: stays hidden across reloads (action persisted)
• Toggle off from card menu: settings reflects muted state and toast shows "Real-time alerts turned off."

API counterpart: dailydotdev/daily-api#3818

Preview domain

https://feat-major-headlines-push-notifi.preview.app.daily.dev

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
daily-webapp	Ready	Preview	Apr 27, 2026 7:20am

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
storybook	Ignored		Apr 27, 2026 7:20am

[nimrodkra]

@claude please review this pr

[claude]

Claude finished @nimrodkra's task in 2m 39s —— View job

---

PR Review: feat/major-headlines-push-notifications

• Gather context and understand the request
• Read changed files
• Review hook: useMajorHeadlinesSubscription.ts
• Review components: HighlightCardOptions, EnableHighlightsAlerts
• Review notification tab changes
• Review supporting changes (feature flags, log events, actions, utils)
• Post final review

---

Overall this is a clean, well-structured feature. The shared hook pattern is good design — single source of truth across three surfaces. A few issues worth addressing before merge, ranging from a logic concern to minor nits.

Bug: subscribe fires analytics and mutates settings regardless of push permission result

packages/shared/src/hooks/notifications/useMajorHeadlinesSubscription.ts:50-77

onEnablePush returns a boolean (true = push granted, false = denied or popup-based). The current subscribe implementation ignores that return value:

await onEnablePush(ORIGIN_TO_PROMPT_SOURCE[origin]);

// These run unconditionally, even if push was denied
setNotificationStatusBulk([...]);
logEvent({ event_name: LogEvent.EnableMajorHeadlinesAlerts, ... });

Two concrete problems:

1. If the user hits "Block" on the browser permission prompt, the settings still flip to subscribed and the "You'll be the first to know" toast fires. The UI shows "Turn off real-time alerts" even though no alerts will actually arrive via push.
2. The EnableMajorHeadlinesAlerts analytics event fires on every click, including denials, which inflates the metric.

If the intent is "subscribing to email + inApp notifications regardless of push" — that's a valid product choice, but the button copy "Get real-time alerts" implies push delivery. Either:

• Gate setNotificationStatusBulk on onEnablePush returning true, or
• Keep the current behaviour but update copy to reflect that email/inApp subscriptions are always enabled on click.

Fix this →

---

Medium: isLoading returned but never consumed — double-click not guarded

useMajorHeadlinesSubscription exposes isLoading but neither HighlightCardOptions nor EnableHighlightsAlerts uses it to disable the button during an in-flight mutation:

// HighlightCardOptions.tsx
const { isSubscribed, subscribe, unsubscribe } = useMajorHeadlinesSubscription();
// isLoading not destructured

// EnableHighlightsAlerts.tsx
const { isSubscribed, subscribe } = useMajorHeadlinesSubscription();
// isLoading not destructured

A fast double-click on "Turn on alerts" will fire two concurrent setNotificationStatusBulk mutations. Either consume isLoading on the buttons, or use a local isPending state after the first call.

Fix this →

---

Medium: Channel-subscription inconsistency between surfaces

Banner / card menu (subscribe/unsubscribe in the hook): subscribes/mutes both inApp and email channels atomically via setNotificationStatusBulk.

Settings tab (EmailNotificationsTab, InAppNotificationsTab): toggles each channel independently via toggleSetting. So a user can have email on but inApp off.

After the user sets fine-grained preferences in settings, clicking "Get real-time alerts" from the card menu will override and re-subscribe both channels, potentially undoing a deliberate choice.

Worth a product decision: should the one-click CTA surfaces respect existing channel preferences (skip already-subscribed channels), or is an "all-in" re-subscription on click intentional?

---

Low: Redundant optional chain on useAuthContext result

packages/shared/src/components/cards/highlight/HighlightCardOptions.tsx:78-79

const auth = useAuthContext();
const user = auth?.user;  // ?.  is unnecessary — useAuthContext() never returns null

useAuthContext() will throw if used outside a provider; it never returns null/undefined. Every other callsite in the codebase uses const { user } = useAuthContext() directly. Fix this →

---

Low: Missing tests for notification settings tab changes

EmailNotificationsTab.tsx and InAppNotificationsTab.tsx each gained a new "Happening Now" section gated by featureMajorHeadlinesPush. No tests exist for this section (no new .spec.tsx added for either file). The existing tests for those files should cover the flag-off case, but there's no coverage for the flag-on rendering or toggle interaction.

---

Low: useMemo on hook return is unnecessary

useMajorHeadlinesSubscription.ts:106-121

Wrapping a hook's return object in useMemo is unusual — hooks return values each render anyway, and the internal useCallback memos already stabilise the functions. The isSubscribed/isPushEnabled/isLoading primitives don't benefit from memoisation either. This isn't harmful, just adds noise. Consider removing.

---

Looks good

• Feature flag + shouldEvaluate pattern is correctly wired (shouldEvaluate: !!user).
• Banner dismissal via ActionType + useActions is the right approach for persistence.
• useLogEventOnce for the impression is correct.
• isActionsFetched guard prevents a flash of the banner before actions load.
• type="button" is set correctly on all buttons (no accidental form submits).
• Split between outer guard component (HighlightCardOptions) and inner content component (HighlightCardOptionsContent) is clean.
• Tests cover the main happy/sad paths well.

[nimrodkra]

Addressed @claude review feedback:

• Channel inconsistency: hook now mutates only inApp. Email preference is left to the settings page, so one-click CTAs no longer override deliberate user choices. (Pairs with the API change that defaults email to Muted for major_headline_added since no email template is wired up.)
• Double-click guard: banner and card menu now consume isLoading plus a local isPending flag.
• Redundant optional chain: useAuthContext() is now destructured directly.
• Unnecessary useMemo: removed; useCallback already stabilizes the function refs.

Skipped:

• onEnablePush return gating: kept current behavior — push permission denial still leaves the in-app subscription enabled, which matches "real-time alerts" (in-app + push when granted). Analytics fire on opt-in regardless of push grant.
• New settings tab tests: existing specs cover flag-off; the gated section is straightforward and the toggle reuses existing primitives.

[claude]

Claude finished @nimrodkra's task in 1m 30s —— View job

---

Follow-up Review: Feedback Addressed ✓

• Gather context and understand the request
• Read updated files to verify fixes
• Assess skipped items
• Post final review

---

All four addressed items are correctly implemented. Here's the verification:

Channel inconsistency ✅ — useMajorHeadlinesSubscription.ts now passes only { channel: 'inApp', ... } in both subscribe (line 55–61) and unsubscribe (line 77–83). isSubscribed is derived solely from settings?.inApp (line 44–45), which is consistent with how InAppNotificationsTab drives its toggle.

Double-click guard ✅ — Both components are correct:

• HighlightCardOptions: destructures isLoading (line 34), guards early in handleToggle with if (isPending || isLoading) return (line 38).
• EnableHighlightsAlerts: same isPending || isLoading guard in handleEnable (line 65), plus disabled={isPending || isLoading} on the button (line 115) — good that the button is visually disabled here too.

Redundant optional chain ✅ — const { user } = useAuthContext() (line 87 of HighlightCardOptions.tsx) — matches the project-wide pattern.

Unnecessary useMemo ✅ — Hook now returns a plain object (lines 93–99). Clean.

---

Skipped items — accepted

onEnablePush return gating: The product framing is sound. The CTA subscribes to in-app delivery; push is additive when granted. Firing analytics on opt-in intent (rather than push grant result) is the right call — the user expressed intent regardless of OS permission outcome. No objection.

Settings tab tests: The gated section (isMajorHeadlinesEnabled && <NotificationSection>...) is a single conditional wrapping a NotificationSwitch that reuses existing primitives. The test surface is thin and covered by the hook-level mock approach used in the existing specs. Acceptable.

---

One minor nit (non-blocking)

In HighlightCardOptions, the three-dots trigger button remains clickable while a mutation is in-flight — the guard is silent (early return in handleToggle), so the dropdown can still be opened and the same item will appear, but clicking it will do nothing. EnableHighlightsAlerts handles this more cleanly with an explicit disabled prop on the button. Not a blocker given the short mutation window, but worth noting for consistency.

---

LGTM overall. The channel-scoping change is the most meaningful fix and it's correct. Ready to merge once CI is green.

[capJavert]

Went quickly through code I don't see anything out of the ordinary, if it works ok on preview go for it 🚀