Skip to content

feat(auth): add content slots to phone number and verification code screens - #1412

Open
demolaf wants to merge 2 commits into
auth/method-picker-layoutfrom
auth/phone-content-slots
Open

demolaf wants to merge 2 commits into
auth/method-picker-layoutfrom
auth/phone-content-slots

Conversation

@demolaf

@demolaf demolaf commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

Adds public EnterPhoneNumberScreen and EnterVerificationCodeScreen, so apps can draw the phone sign-in screens themselves while the library keeps sending codes, signing in, navigating and reporting errors. Stacked on #1411.

Changes

  • Each screen owns its input state and passes its content a PhoneNumberContentState or VerificationCodeContentState: Bindings for input, closures for actions, plus isValid, isLoading and a localized errorMessage.
  • DefaultEnterPhoneNumberContent and DefaultEnterVerificationCodeContent hold the stock markup, and the existing internal views now render them through the screens.
  • The verification screen adds Resend Code with a 60 second countdown, matching the Android phone screen.
  • Verifying now goes through signIn(credentials:), so phone sign-in with MFA reaches the resolution screen instead of dropping the challenge.
  • isLoading now covers sending the code, which previously showed no progress and allowed repeated taps.

API Usage

AuthPickerView { authenticatedApp }
  .pickerDestination { screen in
    switch screen {
    case .enterPhoneNumber:
      EnterPhoneNumberScreen { state in
        TextField("Phone number", text: state.phoneNumber)
        if let errorMessage = state.errorMessage {
          Text(errorMessage)
        }
        Button("Send code", action: state.sendCode)
          .disabled(!state.isValid || state.isLoading)
      }
    case let .enterVerificationCode(verificationID, fullPhoneNumber):
      EnterVerificationCodeScreen(
        verificationID: verificationID,
        fullPhoneNumber: fullPhoneNumber
      ) { state in
        DefaultEnterVerificationCodeContent(state: state)
          .background(.yellow.opacity(0.1))
      }
    default:
      DefaultAuthPickerDestination(screen: screen)
    }
  }

Preview

Default, with resend countdown Custom content showing errorMessage
Default verification code screen Custom verification code screen

Maintainer note: Fixes internal CPRN-518

…creens

Add EnterPhoneNumberScreen and EnterVerificationCodeScreen, generic over content that receives a PhoneNumberContentState or VerificationCodeContentState. The screens own the input state and the send, verify, resend and change-number actions; the content only draws. DefaultEnterPhoneNumberContent and DefaultEnterVerificationCodeContent hold the stock markup, and the existing views render them through the new screens.

The verification code screen adds a resend button with a 60 second countdown. Verifying now goes through signIn(credentials:), so an MFA challenge reaches the resolution screen instead of being dropped. isLoading also covers sending the code, which previously showed no progress and allowed repeated taps.
@demolaf
demolaf added this pull request to stack #1404 October 20, 2026 15:58

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the phone authentication views by separating state management and business logic from the UI layout. It introduces EnterPhoneNumberScreen and EnterVerificationCodeScreen to handle state and actions, delegating rendering to customizable content views like DefaultEnterPhoneNumberContent and DefaultEnterVerificationCodeContent. A critical bug was identified in EnterVerificationCodeScreen where the navigation stack is cleared when multi-factor authentication is required but no MFA handler is configured.

Comment on lines +129 to +133
if case let .mfaRequired(mfaInfo) = outcome, let onMFA = mfaHandler {
onMFA(mfaInfo)
return
}
authService.navigator.clear()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If outcome is .mfaRequired but mfaHandler is nil, the code currently falls through and calls authService.navigator.clear(). This is a bug because the user is not actually signed in yet (MFA is still required), but the navigation stack is cleared as if sign-in succeeded. We should handle this case by reporting an error and keeping the user on the verification screen.

Suggested change
if case let .mfaRequired(mfaInfo) = outcome, let onMFA = mfaHandler {
onMFA(mfaInfo)
return
}
authService.navigator.clear()
switch outcome {
case let .mfaRequired(mfaInfo):
if let onMFA = mfaHandler {
onMFA(mfaInfo)
} else {
let error = AuthServiceError.multiFactorAuth("Multi-factor authentication is required, but no MFA handler is configured.")
errorMessage = error.localizedDescription
reportError?(error)
}
case .signedIn:
authService.navigator.clear()
}

Set the in-flight flag before starting each action's Task, so a quick second tap can't send two codes or overlap verify and resend. Only clear the navigator on .signedIn, so an MFA challenge with no handler doesn't drop the user back to the picker.

Type the state closures as @mainactor and give both state structs public initializers, matching AuthMethodPickerState. Clear errorMessage when the input changes, document every field, disable Change number while loading, and show the resend countdown as its own text instead of a concatenated string.
@demolaf
demolaf marked this pull request as ready for review October 21, 2026 10:11
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