refactor: UI tweaks#1255
Merged
Merged
Conversation
russellwheatley
commented
May 14, 2025
| Divider() | ||
| EmailLinkView() | ||
| } else if authService.authView == .phoneAuth { | ||
| // TODO: - how are we rendering the phone auth View?? |
Member
Author
There was a problem hiding this comment.
This is the nub of the problem and it is the same problem we have rendering the buttons. Core auth package doesn't know about PhoneAuthView so how can we render it?
We could:
- Use AnyView again like with rendering buttons.
- Move Phone Auth package in to Core Auth package - A couple hundred lines of code extra, not a tremendous amount.
- An alternative solution???
Contributor
There was a problem hiding this comment.
To match behavior from old FirebaseUI, this should be a button that launches the phone auth view flow in a modal view. I think if we can we should keep it a separate package despite its small size since it's less popular than email auth.
Member
Author
There was a problem hiding this comment.
I've updated to present a modal, I've had to follow a similar pattern to renderingButtons() logic for the same reason discussed previously (i.e. using AnyView): dea04f6 (#1255)
morganchen12
approved these changes
May 27, 2025
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.
2c4dd76(#1255)af59d88(#1255)bf20f0c(#1255)To give you context on Phone Auth, the UI was rendering directly in the AuthPickerView:

The aim was to make it Phone Auth a button so it would make AuthPickerView less cluttered and closer to previous implementation:

The problem we have now is - how do we shift the View to Phone Auth? It has to happen from within Core Auth package (AuthPickerView). See comment for further details: https://github.com/firebase/FirebaseUI-iOS/pull/1255/files#r2088761197
I guess we should bear in mind this is for alpha release at the moment.