-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Auth] Convert LoginView to SwiftUI #13628
base: main
Are you sure you want to change the base?
Conversation
accentColor: .white, | ||
backgroundColor: .orange | ||
) { | ||
Task { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd refactor the code in this closure into a function on the view. Or maybe even into a view model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored out into a function on the view. I'll leave this thread open to consider doing a view model. For now, I'll focus on refactoring the UI bits..
} | ||
} | ||
.alert("Enter one time passcode.", isPresented: $showingAlert) { | ||
TextField("Verification Code", text: $onetimePasscode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To keep the view's body
readable, this should probably be a standalone view or a view builder function on the surrounding view
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, sorry could you explain this a little bit more? I'm not fully following.
createAccountButton.centerXAnchor.constraint(equalTo: centerXAnchor), | ||
createAccountButton.topAnchor.constraint(equalTo: loginButton.bottomAnchor, constant: 5), | ||
]) | ||
// TODO(ncooke3): Use view modifiers? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or implement a custom button style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, a button style sounds good. When I tried to create a very simple button style, the custom style lost the button's default ability to change text color on tap. I feel like I'm not doing something right if I have to reimplement basic button effects.
private struct MyButtonStyle : ButtonStyle {
func makeBody(configuration: Configuration) -> some View {
configuration.label
}
}
#Preview {
Button("Hello") {}
.buttonStyle(MyButtonStyle())
}
Should I leave this as is? What's the most idiomatic pattern?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#no-changelog