fed-00006: Add bindings email - #27
Conversation
| @Bindable var viewModel = viewModel | ||
| ScrollView { | ||
| VStack(alignment: .leading, spacing: 16) { | ||
| Text("auth_bindings_title") |
There was a problem hiding this comment.
можно встроенный наивный кодген использовать, чтобы так стринги хрупко не писать
There was a problem hiding this comment.
Generate String Catalog Symbols» = Yes и все будет само кодгениться
| Text("auth_bindings_subtitle") | ||
| .font(.subheadline) | ||
| .foregroundStyle(.secondary) | ||
| .frame(maxWidth: .infinity, alignment: .center) | ||
| .multilineTextAlignment(.center) |
There was a problem hiding this comment.
было бы хорошо такие вещи выносить в отдельные вью переменные чтобы в боди было легко читать состав, а сейчас фокус теряется из-за того, что весь экран заполнен модификаторами, плюс сам боди становится очень длинным
| Text("profile_edit_bind_email_title") | ||
| .font(.title3) | ||
| .fontWeight(.semibold) | ||
| .frame(maxWidth: .infinity, alignment: .center) | ||
| .multilineTextAlignment(.center) |
There was a problem hiding this comment.
типа вот такое можно в переменную вынести внутри структуры EmailBindingView и было бы легко читать типа
VStack {
profile_edit_bind_email_title()
profile_edit_bind_email_subtitle()
email_binding_details()
}
типа такого
| Text("profile_edit_bind_email_subtitle") | ||
| .font(.subheadline) | ||
| .foregroundStyle(.secondary) | ||
| .frame(maxWidth: .infinity, alignment: .center) | ||
| .multilineTextAlignment(.center) | ||
| Text("email_binding_details") | ||
| .font(.footnote) | ||
| .foregroundStyle(.secondary) | ||
| .frame(maxWidth: .infinity, alignment: .center) |
There was a problem hiding this comment.
а вообще тут все дублируется почти, так что еще лучше было бы сделать фабрику или кастомные модификатор, которые бы принимали параметры с различиями
| TextField( | ||
| String(localized: "profile_edit_bind_email_email_example"), | ||
| text: $viewModel.email | ||
| ) | ||
| .keyboardType(.emailAddress) | ||
| .textInputAutocapitalization(.never) | ||
| .autocorrectionDisabled() | ||
| .disabled(viewModel.isEmailLocked) | ||
| .padding(.horizontal, 14) | ||
| .padding(.vertical, 14) | ||
| .background(Color(uiColor: .secondarySystemGroupedBackground)) | ||
| .clipShape(RoundedRectangle(cornerRadius: 12)) |
There was a problem hiding this comment.
тоже легко выносится в переменную с понятным названием
| @State private var viewModel = EmailBindingViewModel() | ||
| @FocusState private var isCodeInputFocused: Bool | ||
|
|
||
| var body: some View { |
There was a problem hiding this comment.
тут бы хорошо разбить весь боди на переменные, уж очень он длинный получился, больше 30 строк лучше не писать, читать код становится трудно
| } | ||
| } | ||
|
|
||
| func confirmCode(onSuccess: @escaping (String) -> Void) { |
There was a problem hiding this comment.
почему замыкание, а не async await который может выдать ошибку?
| var formattedTimer: String { | ||
| let minutes = remainingSeconds / 60 | ||
| let seconds = remainingSeconds % 60 | ||
| return String(format: "%02d:%02d", minutes, seconds) | ||
| } |
There was a problem hiding this comment.
| var formattedTimer: String { | |
| let minutes = remainingSeconds / 60 | |
| let seconds = remainingSeconds % 60 | |
| return String(format: "%02d:%02d", minutes, seconds) | |
| } | |
| var formattedTimer: String { | |
| Duration.seconds(remainingSeconds) | |
| .formatted(.time(pattern: .minuteSecond)) | |
| } |
лучше так
| private var cooldownTask: Task<Void, Never>? | ||
| private var stateChanged: ((State) -> Void)? | ||
| private let emailRegex = | ||
| try! Regex("[A-Z0-9a-z._%+-]+@[A-Za-z0-9.-]+\\.[A-Za-z]{2,64}") |
There was a problem hiding this comment.
сможем без форс анврапов?
| struct State { | ||
| let email: String | ||
| let code: String | ||
| let isSendingCode: Bool | ||
| let isConfirmingCode: Bool | ||
| let remainingSeconds: Int | ||
| let isEmailLocked: Bool | ||
| let canRequestCode: Bool | ||
| let isEmailValid: Bool | ||
| let canConfirm: Bool | ||
| let formattedTimer: String | ||
| } |
There was a problem hiding this comment.
если это вью модель (по названию не очевидно), то я бы поднял это на самый верх
| @State private var viewModel = EmailLoginViewModel() | ||
| @FocusState private var isCodeInputFocused: Bool | ||
|
|
||
| var body: some View { |
There was a problem hiding this comment.
тут я бы тоже вьюги вынес по переменным
| .padding(.vertical, 14) | ||
| .background(Color(uiColor: .secondarySystemGroupedBackground)) | ||
| .clipShape(RoundedRectangle(cornerRadius: 12)) | ||
|
|
There was a problem hiding this comment.
вообще всегда пустая строчка в коде - это верный признак того, что код можно разбить по разным функциям, или переменным, в нашем случае
alex-npmn
left a comment
alex-npmn
left a comment
There was a problem hiding this comment.
разнеси пожалуйста в бодях вещи по отдельным приватным компутед пропертям внутри той же структуры, где этот боди находится
тогда у тебя будет суперпонятный код типа
body {
VStack {
text
subtext
button
}
}
так же гораздо проще понимать, что к чему
# Conflicts: # Friendly/Sources/ProfileEdit/ProfileEditView.swift # Friendly/Sources/ProfileEdit/ProfileEditViewModel.swift
7b5331a to
b639d79
Compare
| .focused($isFocused) | ||
| .opacity(0.01) | ||
| .frame(width: 1, height: 1) | ||
| } |
There was a problem hiding this comment.
разобьешь тут тоже по переменным?
| isPresented: .constant(viewModel.error != nil), | ||
| ) { | ||
| Button("sign_up_error_ok") { | ||
| Button(String(localized: .signUpErrorOk)) { |
There was a problem hiding this comment.
по моему не нужен инит через String()
There was a problem hiding this comment.
в основном везде есть инит по локализации
2 participants
No description provided.