feat(ui): spotlight interactive captcha during sign-in/sign-up#8907
feat(ui): spotlight interactive captcha during sign-in/sign-up#8907alexcarpenter wants to merge 9 commits into
Conversation
Add an optional onInteractiveChange prop to CaptchaElement that fires when Cloudflare Turnstile escalates to / resolves an interactive challenge (driven off the existing #clerk-captcha maxHeight MutationObserver). Behavior-preserving: no caller passes the prop yet. Also seeds reusable test-only captcha helpers and Cloudflare test-sitekey constants, plus a turnstile.ts characterization test pinning that invisible challenges never touch #clerk-captcha.
Consolidate the scattered <CaptchaElement/> render sites in the SignIn/SignUp Start flows into one slot rendered as a sibling of the descriptors.main column (direct child of Card.Content), so the widget can stay mounted while sitting outside the soon-to-be-inert subtree. Add an opt-in `gapless` prop that drops the collapsed element out of flow (position:absolute) so it adds no gap gutter to the flex parent; only the start-flow slot opts in, leaving the other render sites unchanged. Behavior-preserving (no spotlight wired yet).
When an interactive bot-protection challenge appears, collapse and inert the descriptors.main column (social buttons, divider, form) so only the header and the captcha widget remain — bringing the 'Verify you are human' check to the foreground. The card restores once the challenge resolves. The passkey action is moved out of the main column so it stays reachable during the spotlight. No focus is moved; the captcha is a non-inert sibling reachable by keyboard. Invisible challenges (~99%) are unaffected.
…line spotlight Sandbox-only: when the query param is present, override the loaded environment to use Cloudflare's test sitekey (3x…FF), render inline, and disable the OAuth bypass so the interactive captcha spotlight can be exercised without changing instance settings or editing src/. No effect without the param.
…xHeight 0 as 0px The interactive check compared maxHeight against the literal '0', but browsers serialize a reset maxHeight of '0' to '0px' when read back off style, so the collapse read as still-interactive and the spotlight never lifted (blank card after the challenge resolved). Compare the parsed length instead. The test helper now collapses with '0px' to mirror the real browser value (jsdom keeps a literal '0', which hid this bug).
…ex gap The collapsed start-flow column used visibility:hidden;height:0, which keeps it a flex child of Card.Content and so still contributes a gap gutter, leaving a large empty space above the spotlighted captcha. display:none removes it from flow entirely; the subtree stays mounted so form state is preserved.
🦋 Changeset detectedLatest commit: 39763d0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited) Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a "captcha spotlight" behavior to sign-in and sign-up flows: when Turnstile escalates to an interactive challenge, the main card column is made inert and hidden ( ChangesCaptcha Spotlight Feature
Sequence Diagram(s)sequenceDiagram
participant Turnstile as Cloudflare Turnstile
participant CaptchaElement
participant SignStart as SignInStart / SignUpStart
participant MainColumn as Main Card Column
Note over Turnstile,MainColumn: Invisible challenge – no spotlight
Turnstile->>CaptchaElement: style.maxHeight stays '0'
CaptchaElement->>SignStart: onInteractiveChange not called
MainColumn-->>MainColumn: remains visible and interactive
Note over Turnstile,MainColumn: Interactive challenge – spotlight activated
Turnstile->>CaptchaElement: style.maxHeight = 'unset'
CaptchaElement->>CaptchaElement: MutationObserver → isInteractive=true
CaptchaElement->>SignStart: onInteractiveChange(true)
SignStart->>MainColumn: inert + display:none
Note over Turnstile,MainColumn: Challenge resolved – spotlight lifted
Turnstile->>CaptchaElement: style.maxHeight = '0px'
CaptchaElement->>CaptchaElement: MutationObserver → isInteractive=false
CaptchaElement->>SignStart: onInteractiveChange(false)
SignStart->>MainColumn: remove inert + display
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ui/src/elements/CaptchaElement.tsx (1)
16-27: ⚡ Quick winAdd an explicit return type to exported
CaptchaElement.Line 16 exports a public component without an explicit return type. Please annotate it explicitly for API clarity and consistency.
Proposed change
export const CaptchaElement = ({ onInteractiveChange, gapless, }: { onInteractiveChange?: (interactive: boolean) => void; /** * When true, the element is removed from flow (`position:absolute`) while collapsed so it adds no * gap gutter to a flex parent, switching to `position:static` while interactive. Opt-in so the * other (non-spotlight) render sites keep their current positioning. */ gapless?: boolean; -}) => { +}): JSX.Element => {As per coding guidelines:
**/*.{ts,tsx}requires explicit return types for functions, especially public APIs.🤖 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 `@packages/ui/src/elements/CaptchaElement.tsx` around lines 16 - 27, The exported CaptchaElement component function lacks an explicit return type annotation, which violates the coding guidelines requiring explicit return types for public APIs. Add an explicit return type annotation after the closing parenthesis of the parameter destructuring in the CaptchaElement function signature, before the arrow function body. Use an appropriate React component return type (such as React.ReactElement, JSX.Element, or React.FC with the props type) to clearly specify what the component returns.Source: Coding guidelines
🤖 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 `@packages/ui/src/elements/__tests__/CaptchaElement.test.tsx`:
- Line 21: The maxHeight assertions on lines 21 and 61 are comparing string
values that may be serialized differently (e.g., '0' vs '0px'), causing flaky
tests. Normalize both the maxHeight value and the expected value to extract only
the numeric portion before comparison. You can do this by parsing the
style.maxHeight to remove the 'px' suffix or converting both to numeric values,
ensuring consistent assertions regardless of CSS serialization format.
---
Nitpick comments:
In `@packages/ui/src/elements/CaptchaElement.tsx`:
- Around line 16-27: The exported CaptchaElement component function lacks an
explicit return type annotation, which violates the coding guidelines requiring
explicit return types for public APIs. Add an explicit return type annotation
after the closing parenthesis of the parameter destructuring in the
CaptchaElement function signature, before the arrow function body. Use an
appropriate React component return type (such as React.ReactElement,
JSX.Element, or React.FC with the props type) to clearly specify what the
component returns.
🪄 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: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 5ebb7487-8af5-47ac-803c-a308ef76bd17
📒 Files selected for processing (11)
.changeset/captcha-inline-spotlight.mdpackages/clerk-js/sandbox/app.tspackages/clerk-js/src/utils/captcha/__tests__/turnstile.test.tspackages/ui/src/components/SignIn/SignInStart.tsxpackages/ui/src/components/SignIn/__tests__/SignInStart.test.tsxpackages/ui/src/components/SignUp/SignUpForm.tsxpackages/ui/src/components/SignUp/SignUpStart.tsxpackages/ui/src/components/SignUp/__tests__/SignUpStart.test.tsxpackages/ui/src/elements/CaptchaElement.tsxpackages/ui/src/elements/__tests__/CaptchaElement.test.tsxpackages/ui/src/test/captcha.ts
💤 Files with no reviewable changes (1)
- packages/ui/src/components/SignUp/SignUpForm.tsx
|
|
||
| const el = getCaptcha(); | ||
| expect(el).not.toBeNull(); | ||
| expect(el.style.maxHeight || '0').toBe('0'); |
There was a problem hiding this comment.
Normalize maxHeight assertions to avoid '0' vs '0px' false results.
Line 21 and Line 61 rely on a literal '0' comparison. Collapsed values can serialize as '0px', and Line 61 can pass even if the captcha never expanded.
Suggested fix
- expect(el.style.maxHeight || '0').toBe('0');
+ expect(parseFloat(el.style.maxHeight || '0')).toBe(0);
...
- await waitFor(() => expect((getCaptcha().style.maxHeight || '0') !== '0').toBe(true));
+ await waitFor(() => expect(getCaptcha().style.maxHeight).toBe('unset'));Also applies to: 61-61
🤖 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 `@packages/ui/src/elements/__tests__/CaptchaElement.test.tsx` at line 21, The
maxHeight assertions on lines 21 and 61 are comparing string values that may be
serialized differently (e.g., '0' vs '0px'), causing flaky tests. Normalize both
the maxHeight value and the expected value to extract only the numeric portion
before comparison. You can do this by parsing the style.maxHeight to remove the
'px' suffix or converting both to numeric values, ensuring consistent assertions
regardless of CSS serialization format.
… MutationObserver callbacks
anagstef
left a comment
There was a problem hiding this comment.
Looks good in general. But I have a main concern:
The implementation on the CaptchaElement is hooked on a CSS implementation detail that the before-interactive-callback is triggering (max-height value). This is fragile and may silently break in the future.
I would suggest we go with another signal, like toggling data-cl-interactive="true" on #clerk-captcha when before-interactive-callback is called and then reset its value on the finally block.
What do you think?
| <CaptchaElement | ||
| gapless | ||
| onInteractiveChange={setCaptchaIsInteractive} | ||
| /> |
There was a problem hiding this comment.
This may be breaking customers that have applied layout CSS changes on the Card.Content component, because it adds one more child to it.
The impact may be really small here, but let's make sure first.
Same applies for SignInStart.tsx
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
What
Spotlight the interactive bot-protection challenge during sign-in/sign-up.
When Turnstile escalates to an interactive "Verify you are human" challenge, the start card now brings it to the foreground — collapses +
inerts the rest (social/form), keeps header/footer/passkey reachable — until solved. Invisible challenges unaffected.How
CaptchaElement: newonInteractiveChange(MutationObserver on#clerk-captchamaxHeightsurfaces interactive ↔ resolved) +gapless(drop out of flow while collapsed, no flex gap gutter).maincolumn (display:none) +inertwhile interactive; passkey action relocated outside the collapsed subtree.@clerk/uipatch changeset.Test
pnpm vitest runinpackages/ui(+ turnstile unit test inclerk-js).cd packages/clerk-js && pnpm dev:sandbox, openhttp://localhost:4000/sign-up?captcha=interactive(or/sign-in).?captcha=interactiveforces the CF test sitekey + inline render + disables OAuth bypass. Click the checkbox → form snaps back on resolve, no gap during challenge. Drop the param for normal invisible captcha.Summary by CodeRabbit
New Features
Tests