Skip to content

feat: post hero#6142

Merged
capJavert merged 3 commits into
mainfrom
post-hero
Jun 5, 2026
Merged

feat: post hero#6142
capJavert merged 3 commits into
mainfrom
post-hero

Conversation

@capJavert
Copy link
Copy Markdown
Contributor

@capJavert capJavert commented Jun 3, 2026

Changes

API dailydotdev/daily-api#3925

Events

Did you introduce any new tracking events?

Experiment

Did you introduce any new experiments?

Manual Testing

Caution

Please make sure existing components are not breaking/affected by this PR

Preview domain

https://post-hero.preview.app.daily.dev

@capJavert capJavert self-assigned this Jun 3, 2026
@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 3, 2026

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

Project Deployment Actions Updated (UTC)
daily-webapp Ready Ready Preview Jun 5, 2026 11:05am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
storybook Ignored Ignored Jun 5, 2026 11:05am

Request Review

@capJavert
Copy link
Copy Markdown
Contributor Author

@claude review

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Jun 5, 2026

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


Code Review: feat: post hero

Read through the full diff (git diff origin/main...HEAD) and traced all postHighlight/hero usages across the codebase. This is a clean rename of the feed-level post.postHighlight field to post.hero, plus two new significance values (breakout, evergreen). Tests, fragment, col-span logic and impression logging are all updated consistently. 👍

Findings

1. Dead code left behind in posts.ts (should remove)

The Post.postHighlight field and the PostHighlight interface in packages/shared/src/graphql/posts.ts are now unreferenced — every consumer (ArticleFeaturedWideGridCard, feedHighlightColSpan, useLogImpression) reads post.hero, and FEED_POST_FRAGMENT no longer requests postHighlight. So:

  • posts.ts:322postHighlight?: PostHighlight | null; has no readers
  • posts.ts:250-256interface PostHighlight is now unused

Per CLAUDE.md ("Delete dead code — don't leave code for later"), these should be removed. Keep PostHighlightSignificance (posts.ts:239) — it's still the base for PostHeroSignificance.

Fix this →

2. Naming collision (pre-existing, but worth a note)

PostHighlight and PostHighlightSignificance are defined in both graphql/posts.ts and graphql/highlights.ts (the unrelated highlights-feed page). CLAUDE.md flags duplicate names in different locations as confusing. Removing the dead posts.ts PostHighlight (finding #1) also eliminates half of this collision for free.

Minor / optional

  • New significance values untested: feedHighlightColSpan.spec.ts and ArticleFeaturedWideGridCard.spec.tsx still only cover breaking/major/notable/routine. Since the whole point of the PR is breakout (→ colSpan 4, "Breaking out") and evergreen (→ colSpan 3, "Evergreen"), consider adding those to the it.each tables to lock in the new mappings.
  • Significance nullability: PostHero.significance is non-nullable (posts.ts:329), whereas the old PostHighlight.significance was … | null. Consumers use post.hero?.significance ?? null so this is safe — just confirm the API (daily-api#3925) never returns hero with a null significance.

Nothing blocking — the core change is correct and consistent. The dead postHighlight/PostHighlight cleanup is the only thing I'd ask for before merge.
· branch post-hero

@capJavert capJavert enabled auto-merge (squash) June 5, 2026 10:59
@capJavert capJavert merged commit 3d2e6de into main Jun 5, 2026
12 checks passed
@capJavert capJavert deleted the post-hero branch June 5, 2026 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant