feat: add advertise-here CTA on ad surfaces

[nimrodkra]

Summary

• Add feature-flagged "Advertise here" CTA across all ad placements (feed grid/list cards, post sidebar widget, comment ads) behind ad_referral_cta GrowthBook flag
• Rename "Remove" button to "Go ad-free" for clearer Plus upgrade messaging
• Make PostSidebarAdWidget fully clickable (entire card links to ad target), matching feed card behavior
• Add impression + click analytics with target_type: "advertise here cta" and placement-specific target_id for CTR tracking
• Fix strict TypeScript errors in AdGrid, AdList, SignalAdList forwardRef generics

Test plan

• Enable ad_referral_cta flag in GrowthBook and verify "Advertise here" appears on feed cards (grid + list), sidebar ad widget, and comment ads
• Disable the flag and verify the CTA is hidden
• Verify "Go ad-free" button links to Plus page
• Verify clicking anywhere on sidebar ad widget navigates to advertiser link
• Verify "Promoted by xxx" link in sidebar widget is still independently clickable
• Verify analytics: impression fires on render, click fires on CTA click, both include correct target_type and target_id

Made with Cursor

Preview domain

https://feat-ad-referral-cta.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 13, 2026 0:43am

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
storybook	Ignored		Apr 13, 2026 0:43am

[nimrodkra]

@claude please review this pr

[claude]

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

---

PR Review: feat: add advertise-here CTA on ad surfaces

• Gather context and understand the PR
• Read changed files
• Review implementation
• Post feedback

---

Overall this is well-structured work. The AdvertiseLink component is clean, feature-flagged correctly, and the analytics integration follows existing patterns. A few issues worth addressing before merge.

---

Issues

1. Regression: AdRefresh button silently removed from AdGrid and AdList

The PR removes AdRefresh (and the onRefreshClick handler) from both feed ad card variants — this wasn't called out in the PR description and isn't mentioned in the test plan. The onRefresh prop is still defined in AdCardProps and still passed from FeedItemComponent.tsx:519-523, making it dead code:

// FeedItemComponent.tsx:519-523 — still passes onRefresh but it's now ignored
onRefresh={
  disableAdRefresh ? undefined : (ad: Ad) => onAdAction(AdActions.Refresh, ad)
}

And in both AdGrid and AdList, refetch/isRefetching from useAutoRotatingAds are destructured but unused. If this removal is intentional, AdCardProps.onRefresh, the FeedItemComponent prop logic, and possibly disableAdRefresh/AdRefresh should be cleaned up too. If it's accidental, the buttons need to be restored.

Fix this →

---

2. SignalAdList missing AdvertiseLink

The PR description says "all ad placements (feed grid/list cards…)" but SignalAdList received only the TypeScript forwardRef fix — no AdvertiseLink CTA was added. Is this intentional? If Signal feed ads should also show the CTA, it needs to be wired up the same way as AdGrid/AdList.

---

3. Indentation inconsistency in AdGrid.tsx

packages/shared/src/components/cards/ad/AdGrid.tsx:81 — the className prop has incorrect indentation relative to its siblings:

              size={ButtonSize.Small}
            className="z-1 typo-footnote"   // ← misaligned
              {...combinedClicks(() => onLinkClick?.(ad))}

Fix this →

---

4. No analytics distinction between grid and list placements

Both AdGrid and AdList use targetId={TargetId.AdCard}. This makes it impossible to compare CTR by feed layout. Consider adding TargetId.AdCardGrid and TargetId.AdCardList if per-placement analysis matters for this experiment.

---

Minor / Observations

• AdvertiseLink impression dedup: The useEffect fires on [isEnabled, logEvent, targetId]. As long as logEvent is stable (it is — wrapped in useCallback in LogContext), this is fine.
• rel inconsistency: AdvertiseLink uses anchorDefaultRel (noopener noreferrer) while other ad links use only "noopener". This is actually better practice for external links (prevents referrer leakage), but worth standardising across ad surfaces eventually.
• PostSidebarAdWidget overlay: The absolute inset-0 z-0 overlay + relative z-1 on interactive elements is the correct pattern and matches how feed cards handle this.
• AdAsComment return type fix (ReactElement → ReactElement | null) and the onAdAction guard for !ad are good correctness fixes.
• promotedText null→undefined is a nice cleanup.

---

Summary

The core CTA feature is solid. The main blocker is item 1 (the unannounced AdRefresh removal) — either confirm it's intentional and clean up the dead code, or restore the buttons. Items 2 and 3 are worth fixing before merge.

[nimrodkra]

Addressed the review feedback in 99f29ef:

• removed the dead feed-card onRefresh wiring left behind after the requested refresh-button removal
• added AdvertiseLink to SignalAdList so the CTA now appears across all feed ad variants in scope
• kept the existing TargetId.AdCard analytics shape for now; we can split grid/list target IDs in a follow-up if we decide we need that experiment granularity

Also re-ran:

• pnpm --filter shared lint
• node ./scripts/typecheck-strict-changed.js
• NODE_ENV=test pnpm --filter shared exec jest packages/shared/src/components/cards/ad/AdCard.spec.tsx --runInBand