Repository navigation
Conversation
demolaf
added this pull request to stack #1404
September 29, 2026 13:00
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the authentication UI flow in FirebaseAuthSwiftUI by extracting the sheet presentation, navigation stack, and error/MFA handling into a new AuthPickerContent view. This allows AuthPickerView to support customizable first screens and destination views via pickerContent and pickerDestination modifiers, while falling back to the newly introduced DefaultAuthPickerContent and DefaultAuthPickerDestination. Key feedback points out that DefaultAuthPickerContent's generic parameter is overly restricted by its initializer, a UIKit dependency is introduced via UIColor.label, and a redundant Task dispatch is used in reportError on a MainActor-annotated struct.
demolaf
marked this pull request as ready for review
September 30, 2026 08:08
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
.pickerContentand.pickerDestinationtoAuthPickerView, so apps can replace the auth sheet's first screen or any pushed screen while the library keeps driving navigation, MFA and account conflicts. Stacked on #1402.Changes
AuthPickerViewkeeps its signature. The new modifiers returnAuthPickerContent, which now owns the sheet,NavigationStack, error alert, and MFA and account-conflict handling.DefaultAuthPickerContent,DefaultAuthMethodPickerandDefaultAuthPickerDestination, holding the markup that was private insideAuthPickerView..pickerContentreplaces only the signed-out screen.SignedInViewand the "Authenticating..." overlay stay inAuthPickerContent, so custom content can't hide them.DefaultAuthPickerContentis generic over its provider list from the start, so the custom-layout initializer in the next PR is additive.Naming, pending team review
AuthPickerContent(the wrapper) sits besideDefaultAuthPickerContent(the default for itsPickerContentslot). Kept for now; the alternative is renaming the wrapper, e.g.ComposedAuthPickerView, before release.API Usage
Preview
Maintainer note: Fixes internal CPRN-516