fix(ui): hide unavailable alternative verification methods - #9355
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 0be5443 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 |
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughSign-in Device Trust and second-factor flows now pass the “Use another method” handler only when multiple verification factors are supported. The backup-code card accepts an optional handler. Test fixtures now support shared second-factor and client-trust setup, including email-code verification. Unit and integration tests cover hidden and visible alternative-method states. A patch changeset documents the behavior. Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ui/src/components/SignIn/__tests__/SignInFactorTwo.test.tsx (1)
410-419: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a visible-state test for
SignInFactorTwo.This test covers only one supported factor. Add a second case with
supportPhoneCode: trueandsupportEmailCode: true, then assert thatUse another methodis present. The positive test inSignInClientTrust.test.tsxdoes not verify the regular second-factor wiring inSignInFactorTwo.tsx.As per coding guidelines, unit tests are required for all new functionality and must verify behavior and edge cases.
🤖 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/components/SignIn/__tests__/SignInFactorTwo.test.tsx` around lines 410 - 419, Add a second test alongside the existing single-factor case in the SignInFactorTwo test suite, configuring startSignInFactorTwo with both supportPhoneCode and supportEmailCode enabled. After preparing the second factor and rendering SignInFactorTwo, assert that “Use another method” is visible, covering the regular two-factor wiring.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/test/fixture-helpers.ts`:
- Line 224: Add a stable emailAddressId field to the email-code fixture object
created in fixture-helpers.ts, so SignInFactorTwoEmailCodeCard can pass it to
prepareSecondFactor when selecting the supportEmailCode factor. Preserve the
existing strategy and safe_identifier values.
---
Nitpick comments:
In `@packages/ui/src/components/SignIn/__tests__/SignInFactorTwo.test.tsx`:
- Around line 410-419: Add a second test alongside the existing single-factor
case in the SignInFactorTwo test suite, configuring startSignInFactorTwo with
both supportPhoneCode and supportEmailCode enabled. After preparing the second
factor and rendering SignInFactorTwo, assert that “Use another method” is
visible, covering the regular two-factor wiring.
🪄 Autofix
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), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 6039a61b-f86a-4f23-90ee-292367704787
📒 Files selected for processing (8)
.changeset/tidy-device-trust-methods.mdintegration/tests/client-trust.test.tspackages/ui/src/components/SignIn/SignInClientTrust.tsxpackages/ui/src/components/SignIn/SignInFactorTwo.tsxpackages/ui/src/components/SignIn/SignInFactorTwoBackupCodeCard.tsxpackages/ui/src/components/SignIn/__tests__/SignInClientTrust.test.tsxpackages/ui/src/components/SignIn/__tests__/SignInFactorTwo.test.tsxpackages/ui/src/test/fixture-helpers.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)clerk/cli(auto-detected)
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@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: |
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
Summary
Root cause
The Device Trust and second-factor containers always passed an alternative-method callback to their verification cards. The cards use the callback’s presence to decide whether to render the action, so the link appeared even when there was no alternative factor.
Impact
Users no longer see a dead-end “Use another method” action when only one verification factor is available. The action remains visible when multiple factors exist.
Validation
pnpm --filter @clerk/ui exec vitest run src/components/SignIn/__tests__/SignInClientTrust.test.tsx src/components/SignIn/__tests__/SignInFactorTwo.test.tsxE2E_APP_ID=next.appRouter.withNeedsClientTrust pnpm playwright test --config integration/playwright.config.ts integration/tests/client-trust.test.tspnpm --filter @clerk/ui format:checkpnpm turbo build --filter=@clerk/ui