-
Notifications
You must be signed in to change notification settings - Fork 0
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
앱 설정 UI 작업 #38
base: dev
Are you sure you want to change the base?
앱 설정 UI 작업 #38
Conversation
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.
고생하셨습니다. 빌드해서 확인했습니다!
- delegate 설정 함수 나눔
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.
고생하셨습니다. :)
tableView | ||
]) | ||
|
||
setConstraints() |
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.
제약 조건을 나누는 함수를 따로 빼는 게 나을 지 고민해봐야겠네요
make.horizontalEdges.bottom.equalToSuperview() | ||
} | ||
|
||
setTableView() |
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.
delegate를 위임하는 함수와 data binding함수를 나누는 게 좋아보입니다.
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.
넵 수정하겠습니다~!
import RxSwift | ||
|
||
class BaseSettingViewController: NagazaViewController { | ||
private var tableViewData = PublishRelay<[String]>() |
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.
BaseSettingViewController를 상속할 때 동일한 타입의 데이터면 괜찮을 수 있는데
각 화면마다 데이터 타입이 달라질 수 있어서 데이터는 각 vc의 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.
넵 수정하겠습니다~!
@@ -13,7 +13,7 @@ public final class AppSettingBuilder: AppSettingBuildable { | |||
public func build(rootViewControllable: ViewControllable) -> AppSettingCoordinating { | |||
|
|||
let coordinator = AppSettingCoordinator(viewControllable: rootViewControllable) | |||
|
|||
coordinator.start() |
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.
Coordinator는 전체적으로 잘 적용하신 것 같습니다!
Motivation
Key Changes 🔑