1
\$\begingroup\$

I have this view in SwiftUI; it works as expected, and for the most part, works great. My only concern with it is that it feels extremely clunky. Reading through the view itself is very difficult. Does anyone have recommendations for cleaning this thing up?

Effectively, the sign in and register pages both have the same options, and there are only minor text differences. however you'll notice I've got some buttons that seem to repeat, but I don't see the need in extracting to its own view because this is the only place it'll be used.

struct RegisterOptionsView: View {
    @EnvironmentObject var baseNavVM: BaseLaunchViewModel
    @ObservedObject var registerOptionsViewModel = RegisterOptionsViewModel()
    @State var isLoggingIn = false

    var body: some View {
        ZStack {
            VStack(spacing: 20) {
                NavigationLink(destination: getDestination(), isActive: $registerOptionsViewModel.shouldNavigateToRegister) { EmptyView() }
                .navigationTitle(isLoggingIn ? "Login" : "Register")

                Spacer()

                VStack(spacing: 30) {
                    VStack {
                        Text("Stello")
                            .font(Fonts.title)
                        Text("[STEL'-LO]")
                            .font(Fonts.body)
                        Text("VERB, GREEK")
                            .font(Fonts.body)
                    }

                    StelloDivider()

                    Text("SENT WITH A PURPOSE")
                        .font(Fonts.subheading)
                        .multilineTextAlignment(.center)
                }

                Spacer()

                // MARK: - APPLE SIGN IN
                VStack {
                    Button(action: {
                        registerOptionsViewModel.selectedLoginOption = .apple
                        withAnimation {
                            isLoggingIn ? baseNavVM.loggedIn = true :
                            registerOptionsViewModel.shouldNavigateToRegister.toggle()
                        }
                    }, label: {
                        HStack(spacing: 16) {
                            Image(systemName: "applelogo")
                                .font(.system(size: 20))
                            Text(isLoggingIn ? "Sign in with Apple" : "Register with Apple")
                                .font(Fonts.button)
                        }
                    }).buttonStyle(StelloFillButtonStyle())
                        .offset(y: 32 )
                        .frame(height: registerOptionsViewModel.showOptions ? 0 : 60 )
                        .hidden(registerOptionsViewModel.showOptions)

                    // MARK: - Forgot Password
                    Group {
                        NavigationLink(destination: Text("Forgot Password"), label: {
                            Text("Forgot Password?")
                                .font(Fonts.body)
                                .underline()
                                .foregroundColor(.blue)
                        })
                            .frame(height: !isLoggingIn ? 0 : 60 )
                            .hidden(!isLoggingIn)
                            .navigationTitle("Password Recovery")

                        // MARK: - Email
                        Button(action: {
                            registerOptionsViewModel.selectedLoginOption = .email
                            withAnimation {
                                isLoggingIn ? baseNavVM.loggedIn = true :
                                registerOptionsViewModel.shouldNavigateToRegister.toggle()
                            }
                        }, label: {
                            Text(isLoggingIn ? "Email" : "Register with Email")
                                .font(Fonts.button)
                        }).buttonStyle(StelloFillButtonStyle())

                        //MARK: - Facebook
                        Button(action: {
                            registerOptionsViewModel.selectedLoginOption = .facebook
                            withAnimation {
                                isLoggingIn ? baseNavVM.loggedIn = true :
                                registerOptionsViewModel.shouldNavigateToRegister.toggle()
                            }
                        }, label: {
                            Text(isLoggingIn ? "Facebook" : "Register with Facebook")
                                .font(Fonts.button)
                        }).buttonStyle(StelloFillButtonStyle())

                        //MARK: - Google
                        Button(action: {
                            registerOptionsViewModel.selectedLoginOption = .google
                            withAnimation {
                                isLoggingIn ? baseNavVM.loggedIn = true :
                                registerOptionsViewModel.shouldNavigateToRegister.toggle()
                            }
                        }, label: {
                            Text(isLoggingIn ? "Google" : "Register with Google")
                                .font(Fonts.button)
                        }).buttonStyle(StelloFillButtonStyle())

                    }
                    .frame(height: !registerOptionsViewModel.showOptions ? 0 : 60 )
                    .hidden(!registerOptionsViewModel.showOptions)

                    // MARK: - Other Options
                    Button(action: {
                        withAnimation(.easeInOut) {
                            registerOptionsViewModel.showOptions.toggle()
                        }
                    }, label: {
                        if !registerOptionsViewModel.showOptions {
                            Text("Other Options")
                                .font(Fonts.button)
                        } else {
                            Text("Cancel")
                                .font(Fonts.button)
                        }
                    }).buttonStyle(StelloFillButtonStyle())
                }

            }
            .padding(.horizontal)
        }
    }

    // MARK: - View Functions
    func getDestination() -> AnyView {
        switch registerOptionsViewModel.selectedLoginOption {
        case .apple:
            return AnyView(Text("Apple"))
        case .email:
            return AnyView(Text("Email"))
        case .facebook:
            return AnyView(Text("Facebook"))
        case .google:
            return AnyView(Text("Google"))
        default:
            return AnyView(EmptyView())
        }
    }
}
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

As your code is not complete, it's hard to see exactly how your code works and therefore hard to make suggestions.

But here are my thoughts:

Yes, you should extract the buttons to a subview. It's not important that you don't use them somewhere else or not. It matters that you are needlessly duplicating code.

In order to test, I had to modify your code (for instance, create the ViewModels you reference) in order to let it compile and while doing so, I had to make the same changes to each Button. If the Button had been abstracted out, then I would have been able to make fewer changes. The same is true for you. If you decide to make any changes to your Button code, you'll have to do it in several places, instead of just one.

Cleaning up code is not only for other programmers or the wider community, but also for yourself. If you return to your code a year later, or even just a month later, any tidying up you've done before will help you better understand the code when you return to it.

I'm also not sure if some of your variables need to be in a ViewModel or should be local @State variables. If they only pertain to this View and don't propagate, it might be better to make them local.

Also not sure why you have a ZStack with only one View (a VStack) inside. If that's the full code, and not an extract for posting here, then it's unnecessary.

Most of your withAnimation blocks are identical. You could do the following:

  1. Define a property: var customAction: () for the struct RegisterOptionsView

  2. Add an init(): init() { self.customAction = <code from current withAnimation> }

  3. Simplify your withAnimation calls to just: withAnimation { customAction }

As you, I like to use VeryDescriptiveNames for my structs and properties, etc, but one thing I do just to save a little space, is I'll abbreviate ViewModel to VM, like you did with BaseNavVM. I'd do the same with RegisterOptionsViewModel, and change it to RegisterOptionsVM.

Another thing is enums. You can define computed properties, such as:

enum LoginOptions {
    case apple
    case email
    case facebook
    case google

    var registerText: String {
        switch self {
            case .apple: return "Register with Apple"
            case .email: return "Register with Email"
            case .facebook: return "Register with Facebook"
            case .google: return "Register with Google"
        }
    }

    var loginText: String {
        switch self {
            case .apple: return "Sign in with Apple"
            case .email: return "Email"
            case .facebook: return "Facebook"
            case .google: return "Google"
        }
    }
}

Doing this would allow you to simplify your Text(isLoggingIn ? .. : ..) views. Added benefit: autocompletion when referencing properties as opposed to hard-coded strings.

Hope you find this useful.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.