Skip to content

feat(onboarding): redesign the post-signup funnel steps - #6391

Open
tsahimatsliah wants to merge 22 commits into
mainfrom
claude/onboarding-button-alignment-6e8571
Open

feat(onboarding): redesign the post-signup funnel steps#6391
tsahimatsliah wants to merge 22 commits into
mainfrom
claude/onboarding-button-alignment-6e8571

Conversation

@tsahimatsliah

@tsahimatsliah tsahimatsliah commented Jul 29, 2026

Copy link
Copy Markdown
Member

Redesigns the post-signup onboarding funnel (/onboarding) into one visual language, from the signup wall through to the Plus step.

The paid /helloworld funnel is unchanged. Every shared surface here gates its onboarding treatment on isOnboarding, threaded from the page through FunnelStepper into FunnelProgressContext (useIsOnboardingFunnel).

Status: review findings are addressed. One item needs a human decision before merge — the edge-aura dependency. Preview: https://claude-onboarding-button-alignme.preview.app.daily.dev


What ships to everyone

The funnel has no background treatment of its own — every step renders on the flat page surface. The redesign is the chrome around the content:

  • Shared top bar. FunnelStepTopBar — logo top-left (icon only below laptop, 24px on desktop), Skip top-right, an even 24px on all three sides. The row is sized off the logo, so the space above the wordmark matches the gutter beside it and the y position is identical on every step.
  • One type scale. OnboardingHeadline / OnboardingSubheadline: one size, weight and 440px measure. Six steps previously used three different sizes.
  • Docked glass CTA. FunnelGlassBar — a blurred, bordered bar the CTA sits inside, on a 440px rail (funnelStepRail), with a scrim so content dissolves under it. Used by the steps, by account details and by verify-email, so the primary action looks the same on all of them.

Per screen

  • Signup wall, account details, verify email, sign-back. These render from the isAuthenticating branch before FunnelStepper mounts, so they take the funnel's chrome through an explicit isOnboardingFunnel prop rather than context — there is no context to read yet. It carries the headline scale and the glass CTA. Deliberately not folded into simplified, which eleven other surfaces set, including /helloworld and the recruiter modals.
  • Pick tags. CTA is disabled rather than hidden below the minimum; floating search field; tighter tag grid (scoped, so feed settings and the post panels keep theirs); tag-styled preview toggle. The feed preview is capped at 3 cards per row with a matching width cap — it previously took its column count from the viewport and divided its own narrower box by six, so cards landed far under their designed 340px.
  • Content types. Icons and an always-visible selection ring; retired types (Community picks, Standups) dropped — and the same filter feeds getContentTypeNotEmpty, so the "pick at least one" gate can't count a card the step never drew.
  • Reading reminder. Full-width option rows with hints, in its own OnboardingReadingReminder; state extracted to useReadingReminder so Submit and skip sit in the docked bar. Implements the APG radio pattern (one tab stop, arrows move selection).
  • Plus / Upload CV. Production's layouts kept; only the title pair follows the shared scale.

The experiment

onboarding_chrome, default control:

Arm Renders
control flat surface, no progress dots
aura animated edge-aura frame + dots under the CTA

Read through useOnboardingChrome so the two pieces can't disagree. Everything under "What ships to everyone" is baseline and is not gated by this flag.

Where the Terms/Privacy notice appears

It lives on RegistrationForm, under the button that creates the account — not on the funnel shell, which also serves screens where the account already exists.

Screen Notice
Signup wall yes — the hero's own, tablet+ (unchanged from main)
Account details yes — at every width
Welcome back / Log in / Verify email no

Account details previously had none below tablet, so a phone could reach "Sign up" without ever being shown the terms.

Keeping it out of the paid funnel

Review found the redesign reaching /helloworld in thirteen places. All fixed:

  • isGlass was hardcoded on six steps, so the paid funnel got the glass bar plus a second logo and Skip over its own stepper header. The gate now lives inside FunnelStepCtaWrapper, which reads useIsOnboardingFunnel() itself — no caller can get it wrong.
  • useReadingReminder ran above the !isOnboarding early return, so /helloworld mounted it alongside main's ReadingReminder — which runs the same effects internally — and logged the step's impression twice per view. The onboarding branch is its own component now. FunnelReadingReminder.spec.tsx pins both directions.
  • ReadingReminder is restored byte-identical to main; the new screen is a separate component, because the two differ in where the actions live.
  • ContentTypes, OnboardingPWA, UploadCv, EditTag, RegistrationFieldsForm and FunnelProfileForm keep main's type scale, width caps, padding, field icons and containers off the onboarding path.
  • ProfileForm moved out of stepsFullWidth into an onboarding-only list.
  • CTA copy restored: Get it for {browser} (not Add to {browser}).

The new dependency

⚠️ The one open question on this PR.

edge-aura draws the experiment arm's animated frame. MIT, zero runtime deps, no network calls or eval, and it honours prefers-reduced-motion. It is also two weeks old with a single maintainer, so:

  • Pinned exactly (0.6.0, no caret) — a pre-1.0 package should not pull in an unreviewed 0.6.x.
  • Loaded through next/dynamic, gated on hasAura. The engine is ~64 kB of canvas code and FunnelStepBackground renders on every funnel step, so a static import shipped it to the whole control cohort and into the extension bundle for a background they never see. Only the type import stays static, and that is erased at compile time.

It needs an explicit yes from whoever owns the bundle: permanent build coupling (paths in three tsconfigs, a jest moduleNameMapper in three packages) for a decorative frame, and extension releases go through store review. If the answer is no, the arm detaches cleanly — useOnboardingChrome is the only reader of the flag, and control ships either way.

Also worth a look

  • The top-bar logo was a live link: linkDisabled only adds pointer-events-none, keeping the real href and the tab stop, so Tab + Enter walked the user out of the funnel mid-flow. The mark is drawn directly as decoration now, and FunnelStepTopBar.spec.tsx pins that.
  • The content-type cards are a multi-select set, so they carry role="checkbox" + aria-checked rather than aria-pressed. The hidden <input> behind them was unreachable and nothing read its name, so it is gone. The unselected ring moved to border-subtlest-primary — it is the only affordance marking a card as selectable.
  • ⚠️ One change is intentionally global. PlusListItem's tooltip dropped its width override on every Plus surface. The old value fought itself: !tablet:max-w-72 was dead (Tailwind's important prefix goes after the variant), leaving !max-w-full to un-cap the tooltip to the full viewport. That was wrong everywhere, so the fix is global. Nothing else about those tooltips changes; the centering is untouched.
  • Fixed a pre-existing strict-mode violation in ContentTypes/helpers.ts, surfaced once the file entered the changed-file guard.

Tests

Five specs, each pinning something that broke or could silently break:

Spec Pins
FunnelReadingReminder.spec.tsx the hook is never called outside the funnel, exactly once inside — the doubled impression event
FunnelStepTopBar.spec.tsx the strip's own width, and that the logo is decoration with no link to tab into
RegistrationForm.spec.tsx the consent line shows in the funnel and nowhere else
AuthHeader.spec.tsx both directions of the funnel headline scale
OnboardingSignupHero.spec.tsx the shell renders no footer chrome and no disclaimer of its own

Reviewing

  • Storybook: Components/Onboarding/Signup funnel steps — per-step stories with the experiment arm as an arg, plus mobile and desktop overviews for both arms.
  • Worth a click through /helloworld on the preview. The paid-funnel branches were restored by reading main's markup; only ReadingReminder is provably byte-identical, and the reading-reminder double-log above is the kind of leak that survives a read-through.
  • To review the aura arm on the preview, the flag default needs a temporary flip — GrowthBook dev tools are off in preview builds.

🤖 Generated with Claude Code

Preview domain

https://claude-onboarding-button-alignme.preview.app.daily.dev

@vercel

vercel Bot commented Jul 29, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
daily-webapp Ready Ready Preview Jul 31, 2026 12:23pm
storybook Building Building Preview Jul 31, 2026 12:23pm

Request Review

One design language across the /onboarding funnel: shared headline scale,
440px rail, top chrome strip (logo + skip), docked glass CTA, and the
giveback brand-gradient canvas. The onboarding_chrome experiment adds an
edge-aura frame + progress dots as the variant arm. The paid /helloworld
funnel is untouched — shared surfaces gate on isOnboarding.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The tag step's feed preview took its column count from the viewport, so on a
wide screen it asked for six columns and divided its own narrower box by six —
cards landed far under the 340px they are designed at. FeedLayoutProvider now
accepts a maxNumCards cap, and the preview pairs a 3-column cap with the width
those columns need. A Storybook story renders the real preview at 3 and 4 so
the count can be chosen visually.

The top bar's logo and skip also step up a size from laptop, and the row is now
sized off the logo rather than the taller button, so the space above the
wordmark matches the 24px gutter beside it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
h-logo-big (33px) overpowered the corner strip. The desktop size is now a
single named constant so it is one line to retune.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The glass CTA carries the funnel's own logo and skip, so six shared steps
passing a bare isGlass gave the paid funnel a second logo and a second Skip
over its stepper header. The gate now lives in FunnelStepCtaWrapper, which
reads useIsOnboardingFunnel() directly.

Also drops FunnelVerifyEmail: nothing ever emits FunnelStepType.VerifyEmail,
so the step never rendered — the live screen is AuthOptionsInner's
EmailVerification tab, which already shows a large mail icon. Two bugs died
with it (the auto-submit dropped the last digit, and Resend reset the cooldown
without asking the backend for a new code).

The signup and verify-email screens now take the funnel's canvas, top bar and
headline scale via an onboarding-only prop, so the two screens in front of the
funnel stop looking like a different product.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Follow-up gating from the review pass. The paid funnel now keeps main's
behaviour on every step the redesign touched:

- The content-types, install-PWA and edit-tags steps keep main's container,
  padding and "Next" label; only the onboarding funnel gets the rail and
  "Continue".
- The browser-extension step keeps "Get it for {browser}"; the shorter
  "Add to {browser}" is onboarding-only, and a Freyja-provided cta still wins.
- Restored bg-gradient-funnel-onboarding-tall and the edit-tags branch that
  selects it; only the onboarding funnel swaps in OnboardingBackground.
- ProfileForm moved out of stepsFullWidth into an onboarding-only list, so
  /helloworld keeps its tablet:max-w-md column.

Also: the funnel's brand mark is now drawn directly instead of through Logo.
linkDisabled only adds pointer-events-none -- it keeps the real href and stays
in the tab order -- so one Tab + Enter walked the user out of the funnel.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…board

Closes out the review pass. Every shared surface the redesign touched now keeps
main's behaviour for the paid funnel:

- ReadingReminder is restored byte-identical to main and the new screen lives in
  OnboardingReadingReminder. A separate component, not a flag: the two differ in
  where the actions live, so the paid screen renders its own Submit and skip
  while the funnel's sit in the docked CTA rail.
- The new screen implements the APG radio pattern -- one tab stop with roving
  tabindex, arrows move selection. Replacing native radios with role="radio"
  buttons had dropped arrow-key navigation entirely.
- ContentTypes takes isOnboarding: retired types, the two-column grid and the
  shared headline are onboarding-only. The retired-title filter moved into
  helpers and is now applied to getContentTypeNotEmpty too, so the "pick at
  least one" gate can no longer count a card the step never rendered.
- OnboardingPWA, UploadCv, EditTag and RegistrationFieldsForm keep main's type
  scale, width caps, padding and field icons off the onboarding path.
- FunnelProfileForm only docks its submit in the CTA rail when onboarding.

Also fixes a pre-existing strict-mode violation in ContentTypes/helpers.ts,
surfaced because the file is now in the changed-file guard.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ton-alignment-6e8571-eng

# Conflicts:
#	packages/shared/src/components/auth/OnboardingRegistrationForm.tsx
The account-details screen on /onboarding is not the page's isAuthenticating
branch -- the funnel is configured to reach it through the organicRegistration
step, so the URL is /onboarding?stepId=... and FunnelOrganicSignup owns the
AuthOptions. It builds its own staticAuthProps and renders its own FooterLinks,
so nothing set on the page reached it: the headline stayed at typo-title2 and
both the footer strip and the terms strip stayed on screen.

It now passes hideSignupDisclaimer and onboardingHeadline, and drops the footer
strip -- all gated on useIsOnboardingFunnel(), since the paid funnel shares this
step and keeps its own chrome.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The live /onboarding signup flow runs through the heroLanding step, not the
organicRegistration one -- FunnelHeroLanding renders AuthOptions inside
OnboardingSignupHero, with its own staticAuthProps. So the account-details
screen kept AuthHeader's typo-title2 and its terms strip.

It now passes hideSignupDisclaimer and onboardingHeadline, gated on
useIsOnboardingFunnel() since the paid funnel shares the step.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The live funnel has no profileForm step -- its steps are heroLanding, editTags,
contentTypes, uploadCv, plusCards, readingReminder, installPwa,
browserExtension -- so the account-details screen is the heroLanding step's
expanded auth form inside OnboardingSignupHero, not FunnelProfileForm. That is
why styling the funnel step never showed up in the flow.

OnboardingSignupHero's isFormExpanded branch now renders the funnel's canvas
(OnboardingBackground) and FunnelStepTopBar in place of the marketing shell's
own logo, and drops the footer links and the disclaimer row. Gated on
useIsOnboardingFunnel(), and scoped to the expanded form so the signup wall and
the paid funnel's landing are untouched.

FunnelProfileForm is left in place: it predates this work and Freyja can still
emit a profileForm step for campaign cohorts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@tsahimatsliah tsahimatsliah left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

A genuinely large redesign of the post-signup funnel, and the craft in it is high — the shared type scale, the funnelStepRail, moving the isGlass gate inside FunnelStepCtaWrapper so no caller can leak it, the FeedContext maxNumCards cap (a real bug: the embedded preview was dividing its own narrow box by a viewport-derived column count), the getContentTypeNotEmpty fix so the gate can't count a card the step never drew, the strict-mode fix in filters/helpers.ts, and removing the focusable-but-pointer-events-none logo link that let Tab + Enter walk out of the funnel. AuthHeader.spec.tsx pinning both directions of onboardingHeadline is the right instinct given simplified has eleven consumers.

Three blocking items, all inline, plus notes. None of them are about the design.

Blocking

  1. New third-party dependency edge-aura@^0.6.0 — first published two and a half weeks ago, one maintainer, ~478 downloads/month, statically imported so it ships to the 100% control cohort and into the extension bundle. Needs an owner's sign-off, an exact pin, and a dynamic import gated on hasAura.
  2. Terms/Privacy notice disappears from the phone-width signup flowhideSignupDisclaimer plus the FooterLinks removal leaves no consent line on any screen below tablet. Compliance call, not a visual one.
  3. /helloworld now double-logs the reading-reminder impressionuseReadingReminder is called above the !isOnboarding early return, alongside main's ReadingReminder which logs the same event. The paid funnel is the one thing this PR promises not to touch.

Non-blocking

Inline: the two different CTA fallback labels (Continue vs Next), the unmentioned global tooltip alignment change in PlusListItem, retiring content types by display-title string, and aria-pressed vs role="checkbox" on CardCheckbox.

Two description issues, since the description is what a reviewer navigates by:

  • The "Removed FunnelVerifyEmail" bullet describes intra-branch churn, not a change to main. That file never existed on main — it was added in this branch's first commit and deleted in a later one, so the net diff contains nothing. The two bugs it claims to have fixed (stale-closure auto-submit, Resend cooldown) were bugs in code that never shipped. There's nothing for a reviewer to verify and it shouldn't reach a changelog.
  • The PlusListItem note covers the width override but not the alignment change that ships with it.

Reviewability

55 files, +3,222/−406, and it bundles a redesign, a new experiment, a new runtime dependency, cross-cutting edits to shared primitives used well outside onboarding (SearchField, CardCheckbox, FeedContext, FeedPreviewControls, PlusListItem) and ~1,190 lines of stories. The description itself says an earlier revision leaked into /helloworld in twelve places and that only ReadingReminder is provably byte-identical to main — and finding 3 above is a thirteenth leak that survived that pass, found by reading one early return. That pattern is the argument for splitting: the dependency plus the aura/dots experiment arm is a self-contained slice that would take the riskiest part out of this diff, and whatever ships should get a human click-through of /helloworld on the preview rather than a read-through of restored markup.

Verification

  • Read the review guide, code-style and privacy rules
  • Verified the edge-aura registry metadata and unpacked the published tarball (reduced-motion handling, no network/eval, static engine import)
  • Traced the isOnboarding threading: page → FunnelStepperFunnelProgressContextuseIsOnboardingFunnel, and every consumer of the new props
  • Traced the mobile signup path screen by screen for the disclaimer regression
  • Checked the mount effects in useReadingReminder against main's ReadingReminder
  • Checked shared-primitive blast radius and FunnelVerifyEmail against main's history
  • Not verified by me: /helloworld end to end, the aura arm rendering, light theme on the new cards, and any visual comparison against design
  • Tests not run locally (no node toolchain in my environment); CI was green/pending at review time

Verdict

Changes requested (posted as a comment — GitHub blocks a formal request-changes on the caller's own PR) on the three blocking items.

Reviewed by AI.

import type { CSSProperties, ReactElement, ComponentProps } from 'react';
import React, { useEffect, useMemo, useState } from 'react';
import classNames from 'classnames';
import { EdgeAura } from 'edge-aura/react';

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking (dependency risk): this static import puts a brand-new third-party package into the shared bundle. From the npm registry today: edge-aura was first published 2026-07-12 (about two and a half weeks ago), latest 0.6.0, one maintainer, ~478 downloads in the last month, 122 kB unpacked / ~64 kB of dist JS. I unpacked the tarball — the code itself looks clean (no network calls, no eval, MIT, zero runtime deps, and it does honour prefers-reduced-motion), so this is about exposure rather than anything malicious.

Three concrete problems:

  1. It ships to everyone, for a feature almost nobody sees. edge-aura/react statically imports the engine chunk, and this module is imported at the top of FunnelStepBackground, which renders on every funnel step including /helloworld. The experiment defaults to gradient, so 100% of users download the engine and the aura arm is the only cohort that uses it. next/dynamic (or React.lazy) gated on hasAura keeps it out of the control path entirely.
  2. It lands in the extension bundle, which is why this PR needs edge-aura mappings in the extension's tsconfig and jest config. Extension releases go through store review, so a bad 0.6.x from a single-maintainer package is expensive to undo.
  3. ^0.6.0 on a pre-1.0 package means any 0.6.x publish is pulled in unreviewed. Repo convention is to pin tightly enough that an unreviewed release can't disrupt builds — for a decorative dependency this new, I'd pin the exact version.

Independent of the above: adding a dependency that needs paths overrides in three tsconfigs plus a jest moduleNameMapper in three packages is a lot of permanent build coupling for a background animation. Worth an explicit "yes we want this dependency" from someone who owns the bundle before it merges — has that happened?

Reviewed by AI.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two of the three are fixed in 1b264b0:

  • Pinned to an exact 0.6.0, no caret.
  • next/dynamic gated on hasAura, so the control cohort and the extension bundle no longer download the engine. The import type { EdgeAuraOptions } is erased at compile time, so it costs nothing.

The third one is yours, not mine to answer: nobody who owns the bundle has said yes to the dependency. I have written the trade-off into the description under "The new dependency" so it is a decision on the PR rather than a line in package.json. If the answer is no, the aura arm is separable — useOnboardingChrome is the only thing reading the flag, and the control path is the gradient canvas that ships regardless.

Comment thread packages/webapp/pages/onboarding.tsx
Comment thread packages/shared/src/features/onboarding/steps/FunnelReadingReminder.tsx Outdated
Comment thread packages/shared/src/features/onboarding/shared/FunnelStepCtaWrapper.tsx Outdated
Comment thread packages/shared/src/components/plus/PlusListItem.tsx Outdated
Comment thread packages/shared/src/components/onboarding/ContentTypes/helpers.ts Outdated
Comment thread packages/shared/src/components/fields/CardCheckbox.tsx Outdated
tsahimatsliah and others added 2 commits July 30, 2026 17:33
Blocking:

- /helloworld logged the reading-reminder impression twice per view.
  useReadingReminder ran above the !isOnboarding early return, alongside
  main's ReadingReminder which runs the same mount effects. The onboarding
  branch is now its own component so the hook only runs where it renders.
- The Terms/Privacy notice had disappeared from the phone-width signup
  flow: the wall shows its own only from tablet up, and the account-details
  screen no longer rendered one. It is docked once at the bottom of that
  screen, at every width, on both entry paths.
- edge-aura is pinned exactly (0.6.0) and loaded through next/dynamic gated
  on hasAura, so the control cohort and the extension bundle no longer carry
  the canvas engine. Whether we want the dependency at all is still an open
  question for whoever owns the bundle.

Non-blocking:

- One CTA fallback per branch, named, instead of two bare literals; the
  three steps that restated the same ternary now pass their label through.
- CardCheckbox takes role="checkbox" + aria-checked, drops the unreachable
  hidden input, and darkens the unselected ring, which is the only thing
  marking a card as selectable.
- PlusListItem keeps main's centered tooltip; only the width un-capping,
  which was a real bug on every Plus surface, ships globally.
- Documented why the retired content types join on display copy: every
  other join into that data (TOGGLEABLE_TYPES, getAdvancedContentTypes,
  contentTypeIcon) already keys on title, and the id is a row id.

Mockup-to-eng-pass: 1

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Brings in #6398 (feed card glass action bar contrast), merged into the PR
branch on GitHub while these fixes were in progress. No conflicts.

Mockup-to-eng-pass: 1
… the auth screens

- The brand sweep drops from a 42rem band to 21rem, so the colour occupies
  roughly the top fifth of the screen instead of half of it.
- The top bar's sticky wrapper had no width. On the sign-back screen, whose
  shell centres its children, it shrink-wrapped to 80px and the logo drifted
  to the middle of the screen, landing on the "Welcome back!" headline.
- "Welcome back!" rendered at typo-title2 because AuthSignBack never received
  the funnel headline flag. It does now, and the shell gained the same pt-3
  clearance the steps use under the top bar.
- Account details and verify-email ended on a bare full-width button while
  every funnel step docks its CTA in the bordered glass bar. The bar is now
  its own FunnelGlassBar component, used by FunnelStepCtaWrapper,
  RegistrationForm and EmailCodeVerification.

`onboardingHeadline` is renamed `isOnboardingFunnel` on the props that thread
through AuthOptions, since it now carries the CTA treatment as well. AuthHeader
keeps the old name — it only ever renders a headline.

Mockup-to-eng-pass: 1

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The disclaimer was docked on the auth shell, which also serves sign-back,
login and verify-email — screens where the account already exists and consent
has already been given. It now lives on RegistrationForm, under the button
that creates the account, so it follows the signup screen on both entry paths
and at every width without the shell having to guess which tab is active.

RegistrationForm.spec.tsx pins both directions; the hero spec now asserts the
shell renders no footer chrome and no disclaimer of its own.

Mockup-to-eng-pass: 1

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tsahimatsliah

Copy link
Copy Markdown
Member Author

Status — ready for internal review

Everything from the review round is fixed and those threads are resolved. One thread is deliberately left open: the edge-aura dependency, which needs a yes/no from whoever owns the bundle. It is the only thing blocking.

Since the review, three things changed beyond the review items:

  • The brand gradient is half as tall (42rem21rem). It was reaching 69% down a 900px screen; it now fades out by 34%.
  • The sign-back screen was broken, not just misaligned. FunnelStepTopBars sticky wrapper had no width, so under that shells items-center it shrink-wrapped to 80px and centred itself — the logo sat on top of the "Welcome back!" headline. Fixed at the component, so it holds under any parent. The headline also now takes the funnel scale.
  • Account details and verify-email ended on a bare button while every step docks its CTA in the bordered bar. The bar is its own FunnelGlassBar now, shared by all three.

Two things to know while reviewing:

  • onboardingHeadline was renamed isOnboardingFunnel on the props threading through AuthOptions, because it now carries the CTA treatment as well as the headline scale. AuthHeader keeps the old name — it only renders a headline.
  • The Terms/Privacy line moved onto RegistrationForm, so it shows on the screen that creates the account and nowhere else. Account details previously had none below tablet.

Still worth a human pass: a click through /helloworld on the preview. Thirteen leaks into the paid funnel have been found and fixed across two review rounds, and the last one was invisible to tests — only ReadingReminder is provably byte-identical to main.

The onboarding funnel has no background treatment of its own: the steps, the
account-details screen and the auth shell all render on the flat page surface.
The paid funnel's per-step gradients are untouched, and the aura experiment arm
still draws its own frame.

`OnboardingChromeVariant.Gradient` becomes `Control` to match — the flag is new
in this PR and only ever compared against `Aura`, so the renamed value is safe
even if the flag was already created in GrowthBook.

Adds FunnelStepTopBar.spec.tsx, which pins the strip's own width. Without it the
strip shrink-wrapped under a centring parent and the logo landed on the
"Welcome back!" headline — a regression nothing else would have caught.

Mockup-to-eng-pass: 1

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@tsahimatsliah tsahimatsliah left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Thorough, well-documented redesign, and the shared-code gating is mostly careful (useIsOnboardingFunnel inside FunnelStepCtaWrapper is the right call). CI is green and the new specs pin the important regressions.

However, the PR's core invariant — "the paid /helloworld funnel is unchanged" — does not hold for three steps. Those are the blockers below; everything else is non-blocking.

Blocking

1. FunnelProfileForm renders a dead duplicate CTA on /helloworld.
The step is now always wrapped in FunnelStepCtaWrapper with cta={{ label: 'Sign up' }}, but form/type="submit" and any onClick are spread only when isOnboarding. On the paid funnel hasGlass is false, so the wrapper still renders its sticky primary button — with no handler, no form, and the label "Sign up" — while RegistrationFieldsForm also renders its own submit button (formId is undefined there). Result on /helloworld: two "Sign up" buttons, the docked one inert. Suggested direction: render the wrapper only when isOnboarding, mirroring what FunnelReadingReminder does with its early return.

2. FunnelBrowserExtension changes the paid funnel's step.
The rewrite is unconditional: the CTA moves from inline in the content column into the wrapper's sticky footer, the headline moves from LargeTitle to OnboardingHeadline (typo-title1), the skip line drops to Callout/Tertiary, and the video cap goes 48rem → 40rem. On top of that, skip is now passed unconditionally, and BrowserExtension is in stepsWithOnlySkipHeader — so on /helloworld the stepper Header skip is still rendered and the wrapper's own Skip button appears below the CTA. That is a second skip control plus an extra FunnelTargetId.StepCta-tagged element on a step that previously had one. Please gate the new layout and the skip prop on isOnboarding.

3. FunnelEditTags changes the paid funnel's layout and CTA behavior.
funnelStepRail and py-6 pt-3 are applied unconditionally, replacing tablet:max-w-md ... p-6 pt-10 for /helloworld (28rem → 32rem, different vertical rhythm, laptop:justify-center dropped from containerClassName). Separately, the CTA changes from hidden (opacity-0 + pointer-events-none + aria-hidden) to visibly disabled below the tag minimum — a deliberate and good change for onboarding, but it also ships to the paid funnel. Both should be behind isOnboarding, or called out explicitly as intended paid-funnel changes.

Non-blocking

4. hideSignupDisclaimer looks like a no-op on the onboarding path. packages/webapp/pages/onboarding.tsx:245 sets it, and FunnelHeroLanding/FunnelOrganicSignup pass it too — but OnboardingRegistrationForm only renders disclaimer in the !isOnboardingTrigger branch, and these call sites all use AuthTriggers.Onboarding. Worth confirming there is a real path where it does something; otherwise it is a new prop threaded through three components for nothing.

5. RETIRED_CONTENT_TITLES keys on API display copy. Documented in the file, and consistent with getAdvancedContentTypes/contentTypeIcon — but this one silently un-retires cards on a backend rename rather than just losing an icon, and the same rename would also change what the "pick at least one" gate counts. Given the API returns a stable id, is a title-keyed retire list the right long-term shape, or should this be driven by a backend flag? At minimum a test asserting the two titles still exist in the fixture would catch drift.

6. edge-aura tsconfig paths. Three tsconfigs now hard-code ../shared/node_modules/edge-aura/dist/react.d.ts. That path depends on pnpm's layout and on the package's internal dist filename, and nothing fails loudly if either moves — a local ambient .d.ts for the edge-aura/react subpath would be more stable. Agreeing with the PR's own flag: a 2-week-old, single-maintainer, pre-1.0 dependency entering the extension build (delayed store-review rollback) for a decorative frame needs an explicit owner yes. Since the aura is /onboarding-only, consider keeping it out of the extension bundle entirely rather than relying on next/dynamic never being reached there.

7. Comment volume. Many of these blocks are multi-paragraph design rationale (FunnelStepBackground's aura options, FunnelGlassBar, common.ts, FunnelStepTopBar). This repo's convention is to keep that reasoning in the PR description/commit message rather than in the source, and the PR description already covers most of it. Trimming would also shrink an already-large diff.

8. FunnelProgressContext and useIsOnboardingFunnel live in FunnelStepDots.tsx. Fourteen or so modules now import funnel-wide context from a file named after a decorative component. A FunnelProgressContext.tsx (or shared/context.ts) would read better.

Verification

  • Read apps-pr-review-guide.md, code-style-rules.md, repo conventions
  • Full diff traced across shared consumers (CardCheckbox, FeedPreviewControls, FeedLayoutProvider, PlusListItem, auth components)
  • Both funnels traced per changed step (isOnboarding gating audited step by step)
  • Experiment default (onboarding_chromecontrol) and conditional evaluation checked
  • CI inspected — all checks green
  • Not manually verified on preview; /helloworld regressions above are read from the diff and should be confirmed on the preview build

Verdict

Comment — three paid-funnel regressions should be fixed or explicitly accepted before merge.

Reviewed by AI.

Comment thread packages/shared/src/features/onboarding/steps/FunnelProfileForm.tsx Outdated
Comment thread packages/shared/src/features/onboarding/steps/FunnelEditTags.tsx Outdated
simplified: true,
// The account-details and verify-email screens stay clean: no footer nav
// strip, no terms strip. The cookie box is the only thing left docked.
hideSignupDisclaimer: true,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question (non-blocking): does this do anything? OnboardingRegistrationForm renders disclaimer only in the !isOnboardingTrigger branch, and every call site that sets hideSignupDisclaimer (here, FunnelHeroLanding, FunnelOrganicSignup) uses AuthTriggers.Onboarding. If there is no reachable path where the strip would otherwise render, this prop is threaded through three components for no effect and could be dropped.

Reviewed by AI.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, and it is closer than it looks. Two of the three call sites hard-code AuthTriggers.Onboarding, so there the prop is indeed inert. This one is loginState?.trigger || AuthTriggers.OnboardingloginState is set by whoever called showLogin, so a non-Onboarding trigger is reachable in principle and the inline strip would render.

I have kept it and written that reasoning into the comment rather than dropping it, since the failure mode of being wrong is a terms strip appearing mid-form on the wall. If you would rather trade that for three fewer props, say so and I will remove it.

Comment thread packages/shared/src/components/onboarding/ContentTypes/helpers.ts Outdated
// `moduleResolution: node` predates package `exports` maps, so TS cannot
// follow edge-aura's "./react" subpath on its own. Bundlers do read the
// map, so the import specifier stays `edge-aura/react` and only the
// type lookup is pointed at the file behind it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: this hard-codes both pnpm's directory layout and edge-aura's internal dist/react.d.ts filename, in three tsconfigs. A patch release that renames the bundled types file, or a hoisting change, breaks typecheck with no other signal. A local ambient declaration for the edge-aura/react subpath in shared would be self-contained and would not need duplicating into extension/storybook.

Separately, on the dependency question the PR raises: since the aura is /onboarding-only, is there a reason to keep it resolvable from the extension build at all rather than excluding it there outright? Extension rollback goes through store review, which is the expensive direction.

Reviewed by AI.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both fair. On the ambient declaration: it would remove the pnpm-layout coupling, but the honest version still has to point at edge-aura/dist/react to re-export the real types, so it trades one hard-coded path for another unless we hand-write the prop types and lose them drifting from the package. The clean fix is moduleResolution: bundler/node16 so TS follows the exports map, which is a repo-wide change well outside this PR.

On excluding it from the extension build: agreed that is the expensive direction to roll back. It is only resolvable there because the extension compiles shared, which contains FunnelStepBackground — the next/dynamic gate keeps it out of the runtime bundle, but typecheck and jest still need to resolve the specifier.

I have deliberately not churned three tsconfigs while the dependency itself is undecided — if the answer on edge-aura is no, all of this wiring goes with it. If it is yes, I will do the resolution work as a follow-up rather than bury it here.

- RETIRED_CONTENT_TITLES was exported but only ever read inside its own module.
- The reading-reminder rows claimed to match the content-type cards' selection
  ring but used a fainter token; both are border-subtlest-primary now, which is
  the value the card ring was raised to for light-theme contrast.
- previewWidthClass kept a `4` branch that nothing could select — a leftover
  from the deleted 3-vs-4 comparison story. Replaced with one constant.
- OnboardingReadingReminder's component docblock sat above the options array,
  so it documented the wrong symbol.
- FunnelGlassBar carries `w-full` itself instead of one of its three callers
  passing it, since the caller that needed it docks the bar in an items-end
  form where a shrink-wrapped bar hugs the right edge.

Mockup-to-eng-pass: 1

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All three rewrote `/helloworld` unconditionally:

- FunnelProfileForm wrapped the step in FunnelStepCtaWrapper regardless, so the
  paid funnel got a docked "Sign up" with no form, no type=submit and no
  onClick — inert — while RegistrationFieldsForm still rendered its own submit.
  Two Sign up buttons, one dead. Main's markup is restored behind an early
  return.
- FunnelBrowserExtension moved the CTA into the docked bar and passed `skip`
  unconditionally. The step is in `stepsWithOnlySkipHeader`, so the stepper
  header already renders one: the paid funnel got two Skips. The headline
  scale, the skip copy tone and the video cap (48rem to 40rem) were leaking
  too. Same early-return treatment.
- FunnelEditTags replaced main's aria-hidden + opacity-0 CTA with a visible
  disabled button, swapped `tablet:max-w-md p-6 pt-10` for the 32rem rail, and
  dropped `laptop:justify-center`. All three now behind isOnboarding.

paidFunnelParity.spec.tsx pins both arms of the two that render differently.

Also from the same review round:
- helpers.spec.ts pins the content-type retirement and doubles as a tripwire:
  the fixture carries the two titles verbatim, so a rename that reaches it
  turns drift into a red build.
- Documented why `hideSignupDisclaimer` is set on the onboarding page — the
  trigger falls back to Onboarding but takes `loginState` first, so the branch
  that renders the inline strip is reachable.

Mockup-to-eng-pass: 1

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
20.3% of this PR's added lines were comments; this brings it to 9.8%. What went
was design narration and tuning history — why 21rem beat 42rem, which edge-aura
stock values were rejected, what a previous attempt looked like. That belongs in
a commit message, and half of it already described a gradient deleted three
iterations ago.

What stays: library gotchas (the slash modifier can't alpha a CSS-variable
colour), repo-wide traps (Tailwind's important prefix goes after the variant),
load-bearing classes (`w-full` under a centring parent), and why a branch exists
at all (paid-funnel parity).

Mockup-to-eng-pass: 1

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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