feat: post hero ad placement#6166
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@claude review |
|
Claude finished @capJavert's task in 4m 29s —— View job Review: post hero ad placementSolid refactor — extracting a single stateful 🔴 High — likely TS regression: deleted module augmentation
declare module 'graphql-request/dist/types' {
interface GraphQLError {
extensions?: Record<string, unknown>;
message?: string;
}
}This PR replaces the entire file with the
Please run 🔴 High — an ad can be dropped when a wide card straddles the cadence point
🟡 Medium — layout inputs now derived independently in two placesThe two builders must receive identical
🟡 Medium — settings toggle now always rendered
🟢 Nits
|
4d42a36 to
a6e55d3
Compare
|
@claude review likely TS regression: deleted module augmentation - its intended |
|
Claude finished @capJavert's task in 1m 46s —— View job Re-review: post hero ad placementThanks for the clarifications — noting both as intended and dropping them:
The branch has also changed since my first pass, and the headline issue I flagged is now fixed. Walking the current code: ✅ Resolved — wide card straddling an ad slot (was my 🔴 #2)The "an ad can be dropped when a wide card steps over the cadence point" bug is handled now. Both placement sites clamp the wide card so its right edge can't cross the next scheduled ad slot:
So a span‑3 hero at ✅ Resolved — layout-input drift between the two builders (was my 🟡 #1)This was my other main concern and it's largely gone:
Both 🟢 Remaining nit (non-blocking)
Net: the two items you called out are acknowledged as intentional, the high-severity ad-drop case is fixed with symmetric clamps on both paths, and the cadence/render drift is resolved by centralizing placements in
|
|
@claude review |
|
Claude finished @capJavert's task in 2m 38s —— View job Re-review: post hero ad placementWalked the current branch end-to-end. The two big items from my earlier passes are still in good shape, and the items you flagged as intentional are noted below. One new finding worth a look. 🟡 Medium —
|
|
@claude review HighlightChip useConditionalFeature call is fine, it has no cost |
|
Claude finished @capJavert's task in 1m 30s —— View job Re-review: post hero ad placementWalked the current branch. The one open finding from my last pass is now fixed, and the nit you called out is acknowledged. ✅ Resolved —
|
Changes
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-ads-placement.preview.app.daily.dev