feat(onboarding): horizon signup wall — flag-gated full-bleed hero variant - #6491
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
tsahimatsliah
left a comment
There was a problem hiding this comment.
Summary
A new flag-gated horizon signup-wall variant reusing the panel's split-column machinery, plus a three-way split of the old splitSignupStyle boolean so a wall can take the geometry without the copy or the CTA hierarchy. CI is green, the diff is scoped to the variant, control walls (cards, panel, desk) look byte-identical, and the sanitizer/HeroBackgroundLayer/precedence issues the description lists as self-review fixes are genuinely fixed in the branch.
Two things I'd want resolved before this runs as an experiment, plus a handful of non-blocking notes. Details are inline.
Blocking
FunnelHeroLanding.tsx:105— enrollment fires for onboarding-funnel visits that never render a wall (already-authenticated users and completed onboarding), diluting the treatment/control split on a signup-completion metric.
Question (potentially blocking)
FunnelHeroLanding.tsx:124— the bounded flag hold blanks the funnel's entry screen for up to 1s for both arms whenever boot arrives without experiment features.swipe_onboarding, cited as the precedent, does not hold render.
Non-blocking
- Email CTA tap target and the
!importantstack (OnboardingRegistrationForm.tsx:208) - Three interdependent style booleans on a shared auth component (
common.tsx:139) - Duplicated inline artwork markup vs. the existing
LandingHeroCovercomponent (OnboardingSignupHero.tsx:229) - Unsized hero asset at
fetchpriority="high"on the funnel entry screen (image.ts:161) - No test coverage for the flag override / hold logic itself
Verification
- Root + package
AGENTS.mdread; scope matches the stated task - Flag/enrollment path traced through
useConditionalFeature→FeaturesReadyContext→trackingCallback→ allocation POST - Control walls and
heroStylescompact-phone rules checked for inherited-compensation loss (horizon correctly reusesonb-hero-main/onb-split-cta/onb-split-login) - CI inspected: all checks passing; branch is behind
mainand needs an update before merge - Not verified by me: preview at 390×844 / 375×667, and the artwork's real LCP on a cold cache
Verdict
Comment — no code-correctness blocker, but the enrollment scope should be fixed before this is turned on as an experiment.
Reviewed by AI.
Review follow-ups — all six addressedOne commit: 55cee74. All six comments are resolved in code rather than deferred, including the four marked non-blocking — the API-shape one in particular was cheap now and would only get more expensive with a third wall.
The blocking one
const isSkippingWall =
(isLoggedIn && user?.infoConfirmed) ||
isOnboardingComplete ||
(isLoggedIn && !isOnboardingActionsReady);
const shouldEvaluateWallFlag = isAuthReady && isOnboarding && !isSkippingWall;The third clause is the same argument applied one step further: a signed-in user's On the holdYou were right to push on it. I kept a hold rather than following New coverage
Suite: 211 passing across onboarding + auth. Verified, not assumed
One thing I did not touch
Ready for another look. |
A flag-gated fourth signup wall, to run against the served one (`cards`) after the panel lost its test. The marketing homepage's hero artwork is the right half full-bleed — no frame, no overlays, nothing to read on the image — dissolving into the page at the seam, with the tagline and the auth options on the rail. Stacked, it takes the panel's shape: an artwork band on top, the form bottom-anchored underneath for thumb reach. The band keeps more height than the panel's on short viewports, because this column has no divider and no bordered email button. The CTA stack is the other half of the bet. One solid primary (Google), GitHub filled-secondary rather than an outline, and email demoted to a text link — a wall that recommends a default instead of offering three identical doors. `splitSignupStyle` had geometry, copy and hierarchy welded into one boolean; it becomes a `signupStyle` discriminant so the horizon can take the geometry while keeping "Continue with…", which logs returning users straight in rather than building a wrong door. Enrollment mirrors the step's render predicate, not just "is this the onboarding funnel": evaluating the flag is what fires GrowthBook's trackingCallback, so a visit that never paints a wall — already authenticated and confirmed, or onboarding already complete — must not allocate, or it sits in the denominator of both arms unable to convert. The wall holds render until the flag resolves so treatment users never paint a frame of the control, bounded at 200ms because `ready` never flips when boot returns no experiment features. Flag defaults to `false`. Cards, desk and panel are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
55cee74 to
a793452
Compare
Rebased onto
|
tsahimatsliah
left a comment
There was a problem hiding this comment.
Re-review (branch rewritten after the 20 Aug pass)
Every item from the last round is genuinely addressed, and a couple went further than I asked:
- Enrollment —
shouldEvaluatenow mirrors the render predicate viaisSkippingWall, with six specs pinning each path. The third clause (signed-in user whose actions haven't landed) is the one I didn't ask for and it is correct. ✅ - Flag hold — 1000ms → 200ms, armed only when the visit is being enrolled, with the boot-ordering reasoning recorded in the constant. The paid-funnel timer nit fell out with it. ✅
- Email CTA — plain
<button>, nine!importants gone,min-h-12for a 48px target under a 20px label. ✅ signupStylediscriminant — done rather than deferred;git grep splitSignupStyleon the branch is clean across all four files, and two specs assert the panel/cards mapping. ✅HorizonArt— one<img>, two named crops. Agreed on keeping it separate fromLandingHeroCoverrather than adding a variant that opts out of the component's reason for existing. ✅- Image — resize 404 and the cache-partition claim both verified rather than asserted, and the description now treats signed derivatives as the ramp prerequisite. ✅
Three new things, one of which undercuts the fetchpriority mitigation added in response to the last round. None are large.
Findings
- Both
HorizonArtinstances are always mounted (CSS-hidden only), so thehigh/lowsplit doesn't do what the comment says. - A signed-in, unconfirmed user can still paint the control wall and then flip to horizon — the exact flash the hold exists to prevent, in the one cohort the hold deliberately doesn't cover.
- Small polish: a reintroduced
!importanttrio, a nested ternary behind an eslint-disable, and anaria-labelthat disagrees with its visible label.
Notes, not findings
funnel step viewis logged byuseFunnelTrackingoffstep?.id, so it fires during the hold and for visits that never paint a wall. Pre-existing and arguably correct, but it means step-view is no longer a proxy for wall-view in this experiment's analysis.- The treatment moves four things at once against
cards: artwork/layout, single-primary hierarchy, email demoted to a link, and Google-first ordering (forced bybackground === 'horizon', overriding the developer-first default). Deliberate as a wall-level test, but a win won't attribute to any one of them — worth naming which you'd keep if it wins. showOrbs/imageMode/imageMobileare silently inert onhorizon, so a Freyja step serving them plus the flag on gets none of them.
Verification
- CI green;
git grep splitSignupStyleclean on the branch - Re-read the enrollment gate, the hold, and both
useConditionalFeaturepaths - Confirmed step-view logging is funnel-level, so the hold doesn't drop the event
- Not verified by me: the measured 48px email row and the 375/390 layouts (Storybook is the evidence)
Reviewed by AI.
Review follow-ups, plus a comment pass. Both artwork elements are in the DOM at every width — the wrappers use Tailwind visibility, and `display: none` does not stop an <img> loading — so the band's `low` and the column's `high` were two hints for one deduped request, resolved by DOM order rather than by viewport. The split claimed a mobile protection that was never in effect. One hint now, and the comment says what actually ships. `isWallPending` required `shouldEvaluateWallFlag`, which is false while a signed-in user's onboarding actions are still loading — so that visit painted the served wall, then swapped to horizon in the same commit once the actions landed. The control-arm flash the hold exists to prevent, reached through the actions fetch instead of the flag fetch. Enrollment resolves through both, so both are now held, with a spec covering the cohort. The stepped-down provider takes ButtonVariant.Float, which is tertiary plus surface-float plus a hairline — what two of the three overrides were recreating by hand. One override left, for the label. The nested ternary becomes getProviderVariant, matching the other getters in the file and dropping its eslint-disable. The email link loses an aria-label that read "Signup using email" over a visible "Continue with email", so its accessible name no longer contradicts what is on screen. Comments cut back to what the code cannot say itself: the flag default rule, why evaluating enrolls, why the hold is bounded, that `w_768` 404s, and the two crops' geometry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5ff51c1 to
697374d
Compare
All five addressed, plus a comment pass — and one bug I found reviewing my own fixTwo commits on the branch now; the second is the review round so the delta stays readable.
The one that mattered mostYou were right that the priority split was fiction. Both wrappers are Tailwind visibility, I took your second option rather than the viewport hook:
|
Ports the signupStyle refactor from #6491 verbatim — the splitSignupStyle boolean becomes a discriminant ('splitCreateAccount' keeps the panel's treatment, 'singlePrimary' is the horizon's) — and opts the strip banner into singlePrimary: Google is the one solid primary, GitHub steps down to a fill, email becomes a text link and the divider goes. preferGithub is off so the solid button leads the stack. Byte-identical to the #6491 versions of the three auth files, so whichever PR lands second merges clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ports the signupStyle refactor from #6491 verbatim — the splitSignupStyle boolean becomes a discriminant ('splitCreateAccount' keeps the panel's treatment, 'singlePrimary' is the horizon's) — and opts the strip banner into singlePrimary: Google is the one solid primary, GitHub steps down to a fill, email becomes a text link and the divider goes. preferGithub is off so the solid button leads the stack. Byte-identical to the #6491 versions of the three auth files, so whichever PR lands second merges clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| // A signed-in user's completion is only known once their actions land; | ||
| // until then we can't tell a wall viewer from someone about to be | ||
| // transitioned past this step. Anonymous visitors have nothing to fetch. | ||
| const isResolvingEnrollment = isLoggedIn && !isOnboardingActionsReady; | ||
| // Enrollment must not fire for a visit that never paints a wall, or it | ||
| // sits in the denominator of both arms unable to convert. | ||
| const isSkippingWall = | ||
| (isLoggedIn && user?.infoConfirmed) || | ||
| isOnboardingComplete || | ||
| isResolvingEnrollment; | ||
| // Evaluating is what enrolls: `getFeatureValue` fires GrowthBook's | ||
| // trackingCallback, which POSTs the allocation. | ||
| const shouldEvaluateWallFlag = | ||
| isAuthReady && isOnboarding && !isSkippingWall; | ||
| const { value: isHorizonWallEnabled, isLoading: isHorizonFlagLoading } = | ||
| useConditionalFeature({ | ||
| feature: featureSignupWallHorizon, | ||
| shouldEvaluate: shouldEvaluateWallFlag, | ||
| }); | ||
| const isHoldingForEnrollment = | ||
| isResolvingEnrollment || (shouldEvaluateWallFlag && isHorizonFlagLoading); | ||
| const holdTimeoutMs = isLoggedIn | ||
| ? ACTIONS_RESOLVE_TIMEOUT_MS | ||
| : FLAG_RESOLVE_TIMEOUT_MS; | ||
| const [hasWaitedForEnrollment, setHasWaitedForEnrollment] = useState(false); | ||
| useEffect(() => { | ||
| if (!isHoldingForEnrollment) { | ||
| return undefined; | ||
| } | ||
| const timeout = setTimeout( | ||
| () => setHasWaitedForEnrollment(true), | ||
| holdTimeoutMs, | ||
| ); | ||
| return () => clearTimeout(timeout); | ||
| }, [isHoldingForEnrollment, holdTimeoutMs]); | ||
| // Painting the served wall and swapping later would show the control arm | ||
| // to treatment users and waste a hero download. Enrollment resolves | ||
| // through the actions fetch as well as the flag, so both are held. | ||
| const isWallPending = isHoldingForEnrollment && !hasWaitedForEnrollment; | ||
| const background = isHorizonWallEnabled ? 'horizon' : backgroundParam; | ||
| const isHorizonWall = background === 'horizon'; | ||
| const oauthOrder = | ||
| oauthOrderParam ?? (isHorizonWall ? 'googleFirst' : undefined); |
There was a problem hiding this comment.
what's this? looks like a huge slop. also why do you touch the order of the oauth?
There was a problem hiding this comment.
Fair on both counts.
The slop: agreed — that block accumulated derived state across three review rounds and ended up over-engineered (and hiding two bugs). Just pushed 969d4f8 which deletes most of it: enrollment is now anonymous onboarding visitors only — one predicate, one 200ms safety-net timeout, net −48 lines. Logged-in users keep the served wall and stay out of the experiment entirely, which is also better experiment hygiene: they get routed to the social-registration display anyway, not the CTA stack this wall redesigns, and the metric is anonymous→signup.
The OAuth order: deliberate, not incidental. The horizon design is one solid primary CTA instead of two identical white buttons, and Google was picked as that primary (broadest one-click path). Order and primacy are the same decision here — the first provider gets the solid treatment. Freyja-served oauthOrder still wins; the code only supplies the default on the flag path so the flag renders the approved design.
Honest caveat: it does bundle two changes into one test (layout + provider order), so a win/loss won't attribute between them. If you'd rather isolate the layout, I can drop the default and keep GitHub-first as the solid primary — say the word and it's a two-line change.
There was a problem hiding this comment.
anonymous onboarding visitors only — one predicate, one 200ms safety-net timeout, net −48 lines. Logged-in users keep the served wall and stay out of the experiment entirely, which is also better experiment hygiene: they get routed to the social-registration display anyway, not the CTA stack this wall redesigns, and the metric is anonymous→signup.
what is this BS? this is just a weird hack. why should we keep them out of the experiment?
There was a problem hiding this comment.
You're right — carve-out removed in fa2f570. Enrollment is now just isAuthReady && isOnboardingFunnel, same as swipe_onboarding: everyone who reaches the step evaluates, logged-in included, and the flag resolves off the boot payload for all of them. No cohort logic left — one predicate and a bounded 200ms hold so treatment users never flash the control.
Ports the signupStyle refactor from #6491 verbatim — the splitSignupStyle boolean becomes a discriminant ('splitCreateAccount' keeps the panel's treatment, 'singlePrimary' is the horizon's) — and opts the strip banner into singlePrimary: Google is the one solid primary, GitHub steps down to a fill, email becomes a text link and the divider goes. preferGithub is off so the solid button leads the stack. Byte-identical to the #6491 versions of the three auth files, so whichever PR lands second merges clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The enrollment/hold block had grown to seven pieces of derived state and two timeouts across three review rounds, and a fresh review found two bugs inside it: the actions hold was not gated on the onboarding funnel, so the paid funnel's entry screen blanked for logged-in users, and the hold deadline keyed off who the user was rather than which fetch was being waited on. The machinery existed to handle logged-in users' enrollment edge cases — but logged-in users don't belong in this experiment. The metric is anonymous-visitor signup conversion, and a signed-in unconfirmed user is routed to the social-registration display rather than the CTA stack the wall redesigns. Enrollment is now anonymous onboarding visitors only, who always render a wall, so exposure still fires exactly for wall viewers; logged-in visits keep the served wall, unheld and unenrolled. Both bugs die with the code they lived in: one predicate, one 200ms safety-net timeout. Also guards the email link's hover while disabled, so it stops brightening as if clickable during social auth. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
969d4f8 to
74af3c4
Compare
Ports the signupStyle refactor from #6491 verbatim — the splitSignupStyle boolean becomes a discriminant ('splitCreateAccount' keeps the panel's treatment, 'singlePrimary' is the horizon's) — and opts the strip banner into singlePrimary: Google is the one solid primary, GitHub steps down to a fill, email becomes a text link and the divider goes. preferGithub is off so the solid button leads the stack. Byte-identical to the #6491 versions of the three auth files, so whichever PR lands second merges clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…outs Review pushback: excluding logged-in users from the experiment was a carve-out nobody asked for. Enrollment is now simply "onboarding funnel, auth ready" — the same shape as swipe_onboarding and the panel's own test — so everyone who reaches the step evaluates, logged-in included, and the flag resolves off the boot payload at the same moment for all of them. One predicate, one bounded 200ms hold, nothing special-cased. The cost is the small symmetric dilution from users who evaluate and are then auto-transitioned past the step; it lands in both arms alike, so it costs a sliver of power, not validity. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ports the signupStyle refactor from #6491 verbatim — the splitSignupStyle boolean becomes a discriminant ('splitCreateAccount' keeps the panel's treatment, 'singlePrimary' is the horizon's) — and opts the strip banner into singlePrimary: Google is the one solid primary, GitHub steps down to a fill, email becomes a text link and the divider goes. preferGithub is off so the solid button leads the stack. Byte-identical to the #6491 versions of the three auth files, so whichever PR lands second merges clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ports the signupStyle refactor from #6491 verbatim — the splitSignupStyle boolean becomes a discriminant ('splitCreateAccount' keeps the panel's treatment, 'singlePrimary' is the horizon's) — and opts the strip banner into singlePrimary: Google is the one solid primary, GitHub steps down to a fill, email becomes a text link and the divider goes. preferGithub is off so the solid button leads the stack. Byte-identical to the #6491 versions of the three auth files, so whichever PR lands second merges clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ports the signupStyle refactor from #6491 verbatim — the splitSignupStyle boolean becomes a discriminant ('splitCreateAccount' keeps the panel's treatment, 'singlePrimary' is the horizon's) — and opts the strip banner into singlePrimary: Google is the one solid primary, GitHub steps down to a fill, email becomes a text link and the divider goes. preferGithub is off so the solid button leads the stack. Byte-identical to the #6491 versions of the three auth files, so whichever PR lands second merges clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ports the signupStyle refactor from #6491 verbatim — the splitSignupStyle boolean becomes a discriminant ('splitCreateAccount' keeps the panel's treatment, 'singlePrimary' is the horizon's) — and opts the strip banner into singlePrimary: Google is the one solid primary, GitHub steps down to a fill, email becomes a text link and the divider goes. preferGithub is off so the solid button leads the stack. Byte-identical to the #6491 versions of the three auth files, so whichever PR lands second merges clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Slimmed after the horizon signup wall (#6491) merged: the SignupStyle union, the single-primary CTA hierarchy and the AuthOptionsInner threading this branch carried all landed with it, so what remains is the banner-only work — the strip drops the media above the headline (geo flag emoji, social network icon, referrer avatar) across all personalized variants, evens its vertical padding, adopts the merged single-primary style, and geoToEmoji leaves with its last consumer. Co-Authored-By: Tsahi Matsliah <tsmatliah@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What
A fourth signup wall,
horizon, behind thesignup_wall_horizonflag — to run against the served one (cards) after the panel lost its test.The marketing homepage's hero artwork is the right half, full-bleed: no frame, no overlays, nothing to read on the image, dissolving into the page at the seam. The rail carries the tagline and the auth options. Stacked, it takes the panel's shape — artwork band on top, form bottom-anchored underneath for thumb reach.
Why it should beat the panel
splitSignupStylesilently rewrote "Continue with…" to "Sign up with…", which is the wrong door for every returning visitor — those buttons log existing users straight in. The horizon keeps "Continue with…".Auth CTA hierarchy
splitSignupStylehad geometry, copy and hierarchy welded into one boolean. It becomes asignupStylediscriminant, so a wall names its treatment and the illegal combinations are unrepresentable:signupStylesplitCreateAccountordividersinglePrimaryBoth imply the split-column geometry.
cardsanddeskpass nothing and are untouched.Provider marks are sized to the label (20px), not the button. The rail is 360px while the copy above keeps a 440px measure, so the CTA stack reads as one glanceable target. The email link is a plain
<button>rather thanButton— the variant's box, shadow and hover--button-backgroundwould each need overriding to look like a link — with amin-h-12row, so the tap target clears 44px while the label stays a 20px link.Experiment readiness
Follows the flag rules in
AGENTS.md: declared infeatureManagement.ts, defaults to the control (false), evaluated throughuseConditionalFeaturewithshouldEvaluate.isAuthReady && isOnboardingFunnel) — the same shape asswipe_onboardingand the panel's own test, with no cohort carve-outs. Evaluating is what allocates:getFeatureValuefires GrowthBook'strackingCallback, which POSTsexperiment_id/variation_id. Users who evaluate and are then auto-transitioned past the step dilute both arms symmetrically — a sliver of power, not a bias. The paid funnel never enrolls. Pinned byFunnelHeroLanding.spec.tsx.isAuthReadyand GrowthBook'sreadycome out of the same boot payload one commit apart, so the deadline is a safety net, not a budget. Kept short becausereadynever flips when boot returns no experiment features, and a longer wait would blank the funnel's entry screen for both arms.background: 'horizon'from Freyja exactly as the panel's test was run. The Google-first order keys off the resolved background, not the flag, so the Freyja path is complete on its own.funnel step view,transition funnel,complete funnel,leave funnel) are step-level and background-agnostic — unchanged. Every provider and the email link keepdata-funnel-track="signup provider", soclick funnel elementfires with identical target ids across arms.Scope
Trimmed to what the experiment needs:
.onb-art-half, overriding only the short-viewport height (this column has no divider and no bordered email button, so the panel's 32dvh would leave a dead gap).SignupWallComparison.stories.tsxalready exists onmain; this adds one frame entry to it rather than new comparison stories.HorizonArtcomponent with two named crops.Known follow-up
Responsive image variants — worth doing before this ramps far. The hero is a single signed Cloudinary URL; the signature covers the transformation, so
w_768404s rather than resizing (verified against the live URL) and phones pull the full asset. Sized derivatives must be exported and signed upstream before asrcSetcan exist. Until then the stacked band requests atfetchpriority="low"so it doesn't race the auth options on a cold mobile connection; the desktop column, which is half the screen, keepshigh.Test plan
Components/Onboarding/Steps/FunnelHeroLanding → Horizon, and the existingSignup wall comparisonpage, which now includes itbackground-color: rgba(0,0,0,0)andbox-shadow: nonewhile moving the label to full strengthsignupStylerefactorlint_shared,test_shared(209 passing across onboarding + auth, including 10 enrollment/hold specs) and prettier run locally before pushingbackground: 'horizon'🤖 Generated with Claude Code
Preview domain
https://feat-signup-wall-horizon.preview.app.daily.dev