Internal Ad Tiles Frontend#4869
Open
ncarazon wants to merge 10 commits into
Open
Conversation
Contributor
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds combined feed tiles, promo tile UI, and dismissal wiring across the posts feed and question sidebar, plus a seeded random helper for deterministic tile selection and ad exposure filtering. ChangesCombined feed tiles rollout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ncarazon
force-pushed
the
feat/internal-ad-tiles
branch
from
af7f06b to
6b7ddef
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
front_end/src/app/(main)/questions/[id]/components/sidebar/similar_questions/use_sidebar_tile.ts (1)
45-48: ⚡ Quick winConsider logging errors from the dismissal API call.
The
onDismisscallback performs optimistic dismissal (setsdismissed = trueimmediately) but silently ignores errors fromClientMiscApi.dismissFeedTile(). While this provides good UX, consider logging failures to aid debugging and monitoring.🔍 Proposed addition to log errors
const onDismiss = useCallback((id: string) => { setDismissed(true); - void ClientMiscApi.dismissFeedTile(id); + void ClientMiscApi.dismissFeedTile(id).catch((error) => { + console.error("Failed to dismiss feed tile:", error); + }); }, []);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@front_end/src/app/`(main)/questions/[id]/components/sidebar/similar_questions/use_sidebar_tile.ts around lines 45 - 48, The onDismiss callback currently calls ClientMiscApi.dismissFeedTile(id) and ignores any errors; update onDismiss (and the async call) to catch failures and log them (e.g., via console.error or the app's logger) while preserving optimistic UI behavior setDismissed(true). Specifically, wrap the promise returned by ClientMiscApi.dismissFeedTile(id) with a .catch or try/catch and log the error with contextual info (include the id and a short message) so failures are recorded for debugging.front_end/src/components/promo_tiles/ad_tile.tsx (1)
38-44: 💤 Low valueConsider optimizing images when possible.
Line 74 sets
unoptimized={true}on theImagecomponent, which bypasses Next.js image optimization. While this is necessary for external/dynamic images, it may impact performance if internal images are used.If
ad.imageorproject.header_imagecan be internal assets, consider conditionally enabling optimization based on the image source.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@front_end/src/components/promo_tiles/ad_tile.tsx` around lines 38 - 44, The Image component in ad_tile.tsx currently forces unoptimized which disables Next.js image optimization; update the rendering of Image (the Image component using the image variable / ad.image or project.header_image) to set unoptimized conditionally: detect whether the src is an internal asset (e.g., src string startsWith('/') or matches your allowed domains) and only set unoptimized={true} for external/dynamic URLs, otherwise allow Next.js optimization (omit or set unoptimized={false}); implement this check in the ad tile component and use that boolean when rendering the Image to preserve optimization for internal images.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@front_end/src/app/`(main)/questions/[id]/components/sidebar/similar_questions/use_sidebar_tile.ts:
- Around line 32-43: The memoized selection in useSidebarTile uses getDaySeed()
inside the useMemo callback but does not include the day seed in the dependency
array, so compute the day seed outside the callback (e.g. const daySeed =
getDaySeed() or memoize it) and include that daySeed in the useMemo deps for
tile; update the dependency array for the useMemo that returns tile (which uses
seededRandom(post.id + daySeed)) to [tiles, dismissed, post.id, daySeed] so the
tile rotation recomputes when the day changes.
- Line 39: The optimistic ad-tile dismissal lacks error handling: update the
onDismiss handler in use_sidebar_tile.ts (the function that sets dismissed =
true and calls ClientMiscApi.dismissFeedTile(id)) to catch failures from
ClientMiscApi.dismissFeedTile(id), log the error (use existing logger or
console.error) and roll back the optimistic state (set dismissed back to false)
or schedule a retry; ensure you reference the dismissed state variable and the
ClientMiscApi.dismissFeedTile call so the rollback/retry and logging are applied
when the promise rejects.
In `@front_end/src/components/posts_feed/build_feed_items.ts`:
- Around line 44-48: Add documentation to the front-end type for project/ad
tiles indicating exposure_rate is a percent in the range 1–100 (matching backend
PositiveSmallIntegerField with MinValueValidator(1) and MaxValueValidator(100));
update the appropriate type/interface in front_end/src/types/projects.ts (e.g.,
the AdTile or Tile interface that exposes ad.exposure_rate) with a comment/JSDoc
stating "exposure_rate: percent (1-100)" so maintainers won't assume a 0-1
fraction, and ensure any related type name referenced in build_feed_items.ts
(tiles, tile.type, tile.ad.exposure_rate) points to that documented type.
In `@front_end/src/components/posts_feed/paginated_feed.tsx`:
- Around line 146-150: The current handleDismiss uses setDismissedIds and
fire-and-forget ClientMiscApi.dismissFeedTile, which only hides locally and
leaves postsFeedKeys.tiles() cache stale; replace this with a proper mutation
(e.g., useMutation) that performs an optimistic update by removing the id from
the postsFeedKeys.tiles() query data and/or invalidates that query on success,
and on error rolls back the optimistic change and logs the failure; update the
handler (handleDismiss) to call the mutation instead of directly calling
ClientMiscApi.dismissFeedTile and remove/adjust direct setDismissedIds usage so
the shared cache (postsFeedKeys.tiles()) is the single source of truth.
In `@front_end/src/utils/posts_feed.ts`:
- Around line 7-13: The seed fallback currently uses `seed | 0 || 1` which
converts 0 to 1 and breaks determinism for seed=0; in the seededRandom function
change the initialization of s to preserve zero seeds (e.g. replace `let s =
seed | 0 || 1;` with a nullish/null check such as `let s = seed == null ? 1 :
(seed | 0);`) so that seededRandom(0) remains distinct while still defaulting to
1 only when seed is null/undefined; update the initialization of variable `s` in
function `seededRandom` accordingly.
---
Nitpick comments:
In
`@front_end/src/app/`(main)/questions/[id]/components/sidebar/similar_questions/use_sidebar_tile.ts:
- Around line 45-48: The onDismiss callback currently calls
ClientMiscApi.dismissFeedTile(id) and ignores any errors; update onDismiss (and
the async call) to catch failures and log them (e.g., via console.error or the
app's logger) while preserving optimistic UI behavior setDismissed(true).
Specifically, wrap the promise returned by ClientMiscApi.dismissFeedTile(id)
with a .catch or try/catch and log the error with contextual info (include the
id and a short message) so failures are recorded for debugging.
In `@front_end/src/components/promo_tiles/ad_tile.tsx`:
- Around line 38-44: The Image component in ad_tile.tsx currently forces
unoptimized which disables Next.js image optimization; update the rendering of
Image (the Image component using the image variable / ad.image or
project.header_image) to set unoptimized conditionally: detect whether the src
is an internal asset (e.g., src string startsWith('/') or matches your allowed
domains) and only set unoptimized={true} for external/dynamic URLs, otherwise
allow Next.js optimization (omit or set unoptimized={false}); implement this
check in the ad tile component and use that boolean when rendering the Image to
preserve optimization for internal images.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f2e4ef05-3f07-48c7-8d54-7a0028b07ac1
📒 Files selected for processing (16)
front_end/src/app/(main)/questions/[id]/components/question_page_shell/tabs.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/tabs/similar_questions.tsxfront_end/src/app/(main)/questions/[id]/components/sidebar/similar_questions/index.tsxfront_end/src/app/(main)/questions/[id]/components/sidebar/similar_questions/use_sidebar_tile.tsfront_end/src/components/posts_feed/build_feed_items.tsfront_end/src/components/posts_feed/feed_tournament_tile.tsxfront_end/src/components/posts_feed/hooks/use_posts_feed_query.tsfront_end/src/components/posts_feed/index.tsxfront_end/src/components/posts_feed/paginated_feed.tsxfront_end/src/components/promo_tiles/ad_tile.tsxfront_end/src/components/promo_tiles/index.tsxfront_end/src/components/promo_tiles/tile_status_row.tsxfront_end/src/components/promo_tiles/tournament_tile.tsxfront_end/src/services/api/misc/misc.shared.tsfront_end/src/types/projects.tsfront_end/src/utils/posts_feed.ts
💤 Files with no reviewable changes (2)
- front_end/src/components/posts_feed/feed_tournament_tile.tsx
- front_end/src/app/(main)/questions/[id]/components/question_page_shell/tabs/similar_questions.tsx
ncarazon
force-pushed
the
feat/internal-ad-tiles
branch
from
642aafd to
784fdf2
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@front_end/src/types/projects.ts`:
- Around line 196-204: Update the inline comment for the
AdTileData.exposure_rate field to correctly reflect the valid 0–100 range (so 0
means never show and 100 means always show); locate the AdTileData type
declaration and change the comment "percent 1–100" to something like "percent
0–100: chance this ad is shown in a given feed slot" to match the downstream
rand() * 100 < exposure_rate logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 11e5a8b4-816d-45eb-9021-3314b375e1c1
📒 Files selected for processing (17)
front_end/src/app/(main)/actions.tsfront_end/src/app/(main)/questions/[id]/components/question_page_shell/tabs.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/tabs/similar_questions.tsxfront_end/src/app/(main)/questions/[id]/components/sidebar/similar_questions/index.tsxfront_end/src/app/(main)/questions/[id]/components/sidebar/similar_questions/use_sidebar_tile.tsfront_end/src/components/posts_feed/build_feed_items.tsfront_end/src/components/posts_feed/feed_tournament_tile.tsxfront_end/src/components/posts_feed/hooks/use_posts_feed_query.tsfront_end/src/components/posts_feed/index.tsxfront_end/src/components/posts_feed/paginated_feed.tsxfront_end/src/components/promo_tiles/ad_tile.tsxfront_end/src/components/promo_tiles/index.tsxfront_end/src/components/promo_tiles/tile_status_row.tsxfront_end/src/components/promo_tiles/tournament_tile.tsxfront_end/src/services/api/misc/misc.shared.tsfront_end/src/types/projects.tsfront_end/src/utils/posts_feed.ts
💤 Files with no reviewable changes (2)
- front_end/src/app/(main)/questions/[id]/components/question_page_shell/tabs/similar_questions.tsx
- front_end/src/components/posts_feed/feed_tournament_tile.tsx
✅ Files skipped from review due to trivial changes (1)
- front_end/src/app/(main)/questions/[id]/components/question_page_shell/tabs.tsx
🚧 Files skipped from review as they are similar to previous changes (10)
- front_end/src/utils/posts_feed.ts
- front_end/src/components/promo_tiles/ad_tile.tsx
- front_end/src/services/api/misc/misc.shared.ts
- front_end/src/components/promo_tiles/tile_status_row.tsx
- front_end/src/app/(main)/questions/[id]/components/sidebar/similar_questions/use_sidebar_tile.ts
- front_end/src/components/posts_feed/hooks/use_posts_feed_query.ts
- front_end/src/app/(main)/questions/[id]/components/sidebar/similar_questions/index.tsx
- front_end/src/components/promo_tiles/tournament_tile.tsx
- front_end/src/components/posts_feed/build_feed_items.ts
- front_end/src/components/posts_feed/paginated_feed.tsx
cemreinanc
approved these changes
Jun 12, 2026
…ypes, and API methods
…t, dismiss logic, and ad click analytics
…faXmark icon, and adjust ad tile background, border, and spacing
ncarazon
force-pushed
the
feat/internal-ad-tiles
branch
from
fbe261b to
28710a4
Compare
ncarazon
force-pushed
the
feat/internal-ad-tiles
branch
from
d75b570 to
578b16f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #4866
Summary
Adds internal ad tiles – a new content surface that lets the programs team promote tournaments and custom campaigns directly in the question feed and similar questions sidebar
Implemented changes:
promo_tiles/: consolidatedAdTile,TournamentTile, and a sharedTileStatusRowinto a single folder; added aPromoTiledispatcher that routes by tile type, replacing duplicated routing logic in the feed and sidebar/ad-tiles/endpoint; ad tiles filtered byexposure_rateusing a seeded random roll (stable per day, rotates daily); tiles inserted at randomized positions (~1 per 10 posts); dismiss removes the slot without shifting remaining tilesinternalAdClickedanalytics event on clickDemo videos
internal-ad-tile-wide.mp4
internal-ad-tile-narrow.mp4
internal-ad-tile-similar-sidebar.mp4
Summary by CodeRabbit