Homepage: eager first trending thumbnail, Turnstile only on intent - #1604
Conversation
|
Warning Review limit reached
Next review available in: 38 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
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 |
PR Summary by QodoHomepage: prioritize first trending thumbnail and mount Turnstile on intent
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1. Captcha slot not reserved
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Greptile SummaryThe PR reduces anonymous homepage startup cost by deferring Turnstile until form interaction and prioritizes the first rendered trending thumbnail.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/web/src/app/_components/landing-page/landing-subscribe-form.tsx | Defers Turnstile mounting until user intent while preserving the existing captcha token and retry flow. |
| apps/web/src/app/_components/landing-page/landing-trending.tsx | Precomputes card thumbnails and prioritizes only the first card that renders an image. |
| apps/web/src/specs/features/landing-page.spec.tsx | Adds focused coverage for image priority, deferred challenge mounting, submit intent, and signed-in 403 recovery. |
Reviews (3): Last reviewed commit: "Homepage: eager first trending thumbnail..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
aaff611 to
c263035
Compare
Code Review by Qodo
1. Turnstile slot can overflow
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| <div className="min-w-[300px] min-h-[65px]"> | ||
| {engaged && ( |
There was a problem hiding this comment.
2. Turnstile slot can overflow 🐞 Bug ≡ Correctness
The new reserved Turnstile placeholder uses min-w-[300px], which can exceed the footer column’s available width on narrow viewports (e.g., 320px wide screens with px-4), causing horizontal page overflow/scrollbars even before the widget mounts.
Agent Prompt
### Issue description
The Turnstile placeholder reserves space with `min-w-[300px]`, which can be wider than the footer’s content box on small screens (notably 320px-wide devices after `px-4` padding). Because it’s a flex item, this can force horizontal overflow and introduce a page-level horizontal scrollbar.
### Issue Context
The landing footer wraps `LandingSubscribeForm` in a flex layout and applies horizontal padding. The Turnstile widget is ~300px wide, but the placeholder currently *enforces* that width via `min-w`, even when the widget isn’t mounted.
### Fix Focus Areas
- apps/web/src/app/_components/landing-page/landing-subscribe-form.tsx[152-166]
### Suggested change
- Replace `min-w-[300px]` with a responsive-safe sizing rule that won’t exceed container width, e.g.
- `className="w-[300px] max-w-full min-h-[65px]"` (preferred over `min-w`), and/or
- add `overflow-x-auto` or `overflow-x-hidden` to prevent page-level overflow if the embedded widget can’t shrink.
This keeps the “reserve space” behavior while preventing the placeholder itself from forcing horizontal overflow.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Taken: the slot is now w-[300px] max-w-full min-h-[65px], so it never exceeds the column on a 320px screen while still reserving the widget's height.
On a phone the first "Trending now" thumbnail is the largest element in the viewport. It streamed in through Suspense with loading=lazy and no priority hint, so the preload scanner never saw it and it waited for layout: PageSpeed reported about 1.3 s of load delay on the LCP element. The first card that has a thumbnail is now eager with fetchpriority=high (React also hoists a preload for it); the rest stay lazy. The newsletter form mounted the Turnstile widget as soon as the page hydrated for anyone without an active user, about 560 KB of third-party script and challenge payload for a form at the bottom of the page. The widget now mounts on the first interaction with the form (focus, pointer, touch, typing, or a submit attempt, so the 403 retry path for a signed-in caller still reveals it). Its 300x65 slot is reserved from the first paint so the late mount does not shift the button, and submit stays gated on the token. Closes #1594
c263035 to
9c610ce
Compare
|
On the testability note about |
1 participant
Two homepage costs measured with PageSpeed on mobile.
The first "Trending now" thumbnail is the mobile LCP element. It streamed in through Suspense with
loading="lazy"and no priority hint, so it was not discoverable from the initial document and waited for layout (about 1.3 s of resource load delay in the breakdown). The first card that actually has a thumbnail is nowloading="eager"+fetchpriority="high"(React hoists a matching preload for it); the rest stay lazy. A text-only post at the top no longer takes the hint away from the card the reader sees.LandingSubscribeFormmounted the Turnstile widget as soon as the page hydrated for anyone without an active user: about 560 KB of third-party script and challenge payload, plus main-thread time, for a form at the bottom of the page that most visitors never reach. The widget now mounts on the first interaction with the form (focus, pointer, touch, typing, or a submit attempt, so the 403 retry for a signed-in caller still reveals it). Its 300x65 slot is reserved from the first paint so the late mount does not push the button to a new line, and submit stays gated on the token as before.Test plan
landing-page.spec.tsx: first-with-thumbnail gets eager + high and the others stay lazy; Turnstile is absent until focus, typing or a submit attempt; a signed-in caller sees the widget only after a 403. 23 tests pass.next lintandtsc --noEmitclean.Closes #1594