Skip to content
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

[Feat] #392 - 솝마디 결과 뷰, 부적 화면 전환 및 데이터 바인딩 #394

Merged
merged 13 commits into from
Sep 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
[Feat] #392 - DailySoptuneCardCoordinator 생성
  • Loading branch information
dlwogus0128 committed Sep 28, 2024
commit b76e5e7751e8deee18a8d065f1d29cdc5b59f9a3
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,12 @@
import BaseFeatureDependency
import Core

public protocol DailySoptuneCardPresentable: ViewControllable { }
public protocol DailySoptuneCardViewControllable: ViewControllable { }

public protocol DailySoptuneCardCoordinatable {
var onGoToHomeButtonTapped: (() -> Void)? { get set }
var onBackButtonTapped: (() -> Void)? { get set }
}

public typealias DailySoptuneCardViewModelType = ViewModelType & DailySoptuneCardCoordinatable
public typealias DailySoptuneCardPresentable = (vc: DailySoptuneCardViewControllable, vm: any DailySoptuneCardViewModelType)
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import Domain
public protocol DailySoptuneResultViewControllable: ViewControllable { }

public protocol DailySoptuneResultCoordinatable {
var onNaviBackButtonTap: (() -> Void)? { get set }
var onNaviBackButtonTapped: (() -> Void)? { get set }
var onKokButtonTapped: ((PokeUserModel) -> Driver<(PokeUserModel, PokeMessageModel, isAnonymous: Bool)>)? { get set }
var onReceiveTodaysFortuneCardButtonTapped: ((DailySoptuneCardModel) -> Void)? { get set }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import Core
import Domain
@_exported import DailySoptuneFeatureInterface
import PokeFeatureInterface

public final class DailySoptuneBuilder {
@Injected public var dailySoptuneRepository: DailySoptuneRepositoryInterface
Expand All @@ -35,8 +34,10 @@ extension DailySoptuneBuilder: DailySoptuneFeatureBuildable {
}

public func makeDailySoptuneCardVC(cardModel: DailySoptuneCardModel) -> DailySoptuneCardPresentable {
let dailySoptuneCardVC = DailySoptuneCardVC(cardModel: cardModel)
return dailySoptuneCardVC
let useCase = DefaultDailySoptuneUseCase(repository: dailySoptuneRepository)
let viewModel = DailySoptuneCardViewModel(useCase: useCase)
let dailySoptuneCardVC = DailySoptuneCardVC(cardModel: cardModel, viewModel: viewModel)
return (dailySoptuneCardVC, viewModel)
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
//
// DailySoptuneCardCoordinator.swift
// DailySoptuneFeature
//
// Created by Jae Hyun Lee on 9/28/24.
// Copyright © 2024 SOPT-iOS. All rights reserved.
//

import UIKit
import Combine

import Core
import BaseFeatureDependency
import DailySoptuneFeatureInterface
import Domain

public final class DailySoptuneCardCoordinator: DefaultCoordinator {

public var finishFlow: (() -> Void)?

private let factory: DailySoptuneFeatureBuildable
private let router: Router

private let cardModel: DailySoptuneCardModel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

질문!
현재 요구사항과 뷰 구조를 제가 잘 몰라서 생긴 질문인데요~!
cardModel을 Coordinator가 속성으로 들고 있지 않고 start()의 파라미터로 받게 하는건 어려울까요!?
지금도 크게 문제될 것은 없지만 코디네이터가 굳이 이런 도메인 상태를 가져야 하는가 하는 짧은 생각이 들어서요~ ㅎㅎ

    public override func start(cardModel: DailySoptuneCardModel) {
        showDailySoptuneCard(cardModel: DailySoptuneCardModel)
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

헉 ... 이 부분 논의해보고 수정하겠습니다 ...ㅇ.ㅇ


private weak var rootController: UINavigationController?

public init(router: Router, factory: DailySoptuneFeatureBuildable, cardModel: DailySoptuneCardModel) {
self.router = router
self.factory = factory
self.cardModel = cardModel
}

public override func start() {
showDailySoptuneCard()
}

private func showDailySoptuneCard() {
var dailySoptuneCard = factory.makeDailySoptuneCardVC(cardModel: cardModel)

dailySoptuneCard.vm.onBackButtonTapped = { [weak self] in
self?.router.dismissModule(animated: true)
self?.finishFlow?()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guard let으로 self 한 번 확인해주세요 !

}

self.rootController = dailySoptuneCard.vc.asNavigationController
self.router.present(self.rootController, animated: true, modalPresentationSytle: .overFullScreen)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ public final class DailySoptuneResultCoordinator: DefaultCoordinator {
private let factory: DailySoptuneFeatureBuildable
private let pokeFactory: PokeFeatureBuildable
private let router: Router

private weak var rootController: UINavigationController?

public init(router: Router, factory: DailySoptuneFeatureBuildable, pokeFactory: PokeFeatureBuildable) {
self.router = router
self.factory = factory
Expand All @@ -35,7 +36,7 @@ public final class DailySoptuneResultCoordinator: DefaultCoordinator {
private func showDailySoptuneResult() {
var dailySoptuneResult = factory.makeDailySoptuneResultVC()

dailySoptuneResult.vm.onNaviBackButtonTap = { [weak self] in
dailySoptuneResult.vm.onNaviBackButtonTapped = { [weak self] in
self?.router.dismissModule(animated: true)
self?.finishFlow?()
}
Expand All @@ -47,14 +48,30 @@ public final class DailySoptuneResultCoordinator: DefaultCoordinator {

dailySoptuneResult.vm.onReceiveTodaysFortuneCardButtonTapped = { [weak self] cardModel in
guard let self else { return }
let dailySoptuneCardVC = self.factory.makeDailySoptuneCardVC(cardModel: cardModel).viewController
dailySoptuneCardVC.modalPresentationStyle = .overFullScreen
dailySoptuneResult.vc.viewController.present(dailySoptuneCardVC, animated: false)
self.runDailySoptuneCardFlow(cardModel: cardModel)
}

router.push(dailySoptuneResult.vc)
rootController = dailySoptuneResult.vc.asNavigationController
router.present(rootController, animated: false, modalPresentationSytle: .overFullScreen)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분 저는 viewcontrollable까지만 전달했는데 asNavigationController로 지정해주신 이유가 있을까요?
개인적인 생각으로는 main에서는 새 내비게이션이 시작하니까 showDailySoptuneMain에서는 asNavigationController가 맞는 것 같고 result랑 card에서는 viewControllable만 넘겨주는 것이 맞는 것 같습니다 !

근데 vc를 넘겨주는 것과 vc.viewControllable의 차이를 잘 모르겠어요. . . @lsj8706 혹시 설명해주실 수 있나요 ? 선배님 ..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dailySoptuneResult.vc를 present를 띄우고 있는데, 만약 이 vc위에 다른 vc를 push 해야 한다는 니즈가 있다면, 지금처럼 asNavigationController 사용해서 네비게이션 컨트롤러로 만들어서 올려야해요.
이런 니즈가 없다면 그냥 viewControllable 상태로 올려도 무방해요!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아하!!! 감사합니다

}

internal func runDailySoptuneCardFlow(cardModel: DailySoptuneCardModel) {
let dailySoptuneCardCoordinator = DailySoptuneCardCoordinator(
router: Router(
rootController: rootController ?? self.router.asNavigationController
), factory: factory
, cardModel: cardModel
)

dailySoptuneCardCoordinator.finishFlow = { [weak self, weak dailySoptuneCardCoordinator] in
dailySoptuneCardCoordinator?.childCoordinators = []
self?.removeDependency(dailySoptuneCardCoordinator)
}

addDependency(dailySoptuneCardCoordinator)
dailySoptuneCardCoordinator.start()
}

private func showMessageBottomSheet(userModel: PokeUserModel, on view: UIViewController?) -> AnyPublisher<(PokeUserModel, PokeMessageModel, isAnonymous: Bool), Never> {
Copy link
Member

@lsj8706 lsj8706 Sep 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이건 그냥 제가 예전에 쌓아둔 개인적인 아쉬움인데 공유차 남겨요.

현재는 바텀 시트를 보여주고 이 바텀시트에서 사용자가 선택한 데이터들을 Publisher 형태로 제공하고 있는데 이걸 바텀 시트를 띄우는 메서드인 showMessageBottomSheet()의 리턴값으로 바로 반환하니까 이 함수를 처음 보는 사람 입장에서는 showMessageBottomSheet()는 왜 (PokeUserModel, PokeMessageModel, isAnonymous: Bool) 이런 값들을 반환하는거지? 이 값들은 뭘까 하는 의문이 생길 수 있다고 봐요. 함수명은 그저 바텀 시트를 show 한다고만 알려주는데 뭔가를 리턴하고 이 리턴 값의 정체가 불분명 하니까요.

그래서 이런 바텀 시트류의 UI에서는 콜백 인터페이스가 Publisher나 클로저가 아닌 Delegate 패턴이어야 더 명확하다고 생각해요. (예전에는 이런 개념이 없었어서 그냥 혼용했었음 😅)
예를 들어 이렇게 바뀌는 거에요.

private func showMessageBottomSheet(userModel: PokeUserModel, on view: UIViewController?) {
  guard let bottomSheet = self.pokeFactory
            .makePokeMessageTemplateBottomSheet(messageType: messageType)
  // 생략
  bottomSheet.delegate = self
  // 생략
}

extension DailySoptuneResultCoordinator: MessageBottomSheetDelegate {
   func messageBottomSheet(pickedMessage: PokeMessageModel) {} // 필요한 정보들 전달(축약 버전)
}

즉, 이벤트 전달을 위한 바인딩을 할 때 언제 클로저(또는 Publisher)를 쓸지 또는 Delegate를 쓰면 좋을지 상황에 맞게 잘 고민해보면 좋을 것 같아요. 각각 장단점이 있어요.

사실 여기서는 이미 publisher로 인터페이스가 맞춰져 있어서 그냥 이대로 가는게 최선이에요 ㅋㅋㅋ 그냥 공부할 때 도움되라고 남긴 코멘트입니당

let messageType: PokeMessageType = userModel.isFirstMeet ? .pokeSomeone : .pokeFriend

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,21 @@
//

import UIKit
import Combine

import Core
import DSKit
import Domain

import BaseFeatureDependency

public final class DailySoptuneCardVC: UIViewController, DailySoptuneCardPresentable {
public final class DailySoptuneCardVC: UIViewController, DailySoptuneCardViewControllable {

// MARK: - Properties

public var viewModel: DailySoptuneCardViewModel
private let cardModel: DailySoptuneCardModel
private var cancelBag = CancelBag()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요거 private let으로 바꿔도 될 것 같긴하네요


// MARK: - UI Components

Expand All @@ -44,8 +47,9 @@ public final class DailySoptuneCardVC: UIViewController, DailySoptuneCardPresent

// MARK: - initialization

init(cardModel: DailySoptuneCardModel) {
init(cardModel: DailySoptuneCardModel, viewModel: DailySoptuneCardViewModel) {
self.cardModel = cardModel
self.viewModel = viewModel
super.init(nibName: nil, bundle: nil)
}

Expand All @@ -60,6 +64,7 @@ public final class DailySoptuneCardVC: UIViewController, DailySoptuneCardPresent
setUI()
setLayout()
setData()
bindViewModels()
}
}

Expand Down Expand Up @@ -114,4 +119,13 @@ private extension DailySoptuneCardVC {
self.cardLabel.partColorChange(targetString: "\(cardModel.name)", textColor: UIColor(hex: "\(cardModel.imageColorCode)"))
self.cardImage.setImage(with: cardModel.imageURL)
}

private func bindViewModels() {
let input = DailySoptuneCardViewModel.Input(
goToHomeButtonTap: self.goToHomeButton.publisher(for: .touchUpInside).mapVoid().asDriver(),
backButtonTap: self.backButton.publisher(for: .touchUpInside).mapVoid().asDriver()
)

let output = self.viewModel.transform(from: input, cancelBag: self.cancelBag)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
//
// DailySoptuneCardViewModel.swift
// DailySoptuneFeature
//
// Created by Jae Hyun Lee on 9/28/24.
// Copyright © 2024 SOPT-iOS. All rights reserved.
//

import Foundation
import Combine

import Core
import Domain

import DailySoptuneFeatureInterface

public final class DailySoptuneCardViewModel: DailySoptuneCardViewModelType {

public var onGoToHomeButtonTapped: (() -> Void)?
public var onBackButtonTapped: (() -> Void)?

// MARK: - Properties

private let useCase: DailySoptuneUseCase
private var cancelBag = CancelBag()

// MARK: - Inputs

public struct Input {
let goToHomeButtonTap: Driver<Void>
let backButtonTap: Driver<Void>
}

// MARK: - Outpust

public struct Output {

}

// MARK: - initialization

public init(useCase: DailySoptuneUseCase) {
self.useCase = useCase
}
}

extension DailySoptuneCardViewModel {

public func transform(from input: Input, cancelBag: CancelBag) -> Output {
let output = Output()

input.goToHomeButtonTap
.withUnretained(self)
.sink { _ in
self.onGoToHomeButtonTapped?()
}.store(in: cancelBag)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 쓰면 withUnretained는 의미가 없을거에요.
withUnretained의 구현부를 보시면 self를 weak 참조 한다음에 리턴해주고 있어요. 이 리턴된 값을 사용해야 약한 참조가 유지돼요.
(그래서 개인 취향이지만 전 귀찮아서 [weak self] 쓰고 그냥 guard let이나 self?로 처리해요)

Suggested change
input.goToHomeButtonTap
.withUnretained(self)
.sink { _ in
self.onGoToHomeButtonTapped?()
}.store(in: cancelBag)
input.goToHomeButtonTap
.withUnretained(self)
.sink { owner in
owner.onGoToHomeButtonTapped?() // 여기서 owner가 self를 약한 참조한 변수
}.store(in: cancelBag)


input.backButtonTap
.withUnretained(self)
.sink { _ in
self.onBackButtonTapped?()
}.store(in: cancelBag)

return output
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ import Domain

import DailySoptuneFeatureInterface

public class DailySoptuneResultViewModel: DailySoptuneResultViewModelType {
public final class DailySoptuneResultViewModel: DailySoptuneResultViewModelType {

public var onNaviBackButtonTap: (() -> Void)?
public var onReceiveTodaysFortuneCardTap: ((DailySoptuneCardModel) -> Void)?
public var onNaviBackButtonTapped: (() -> Void)?
public var onReceiveTodaysFortuneCardTap: ((DailySoptuneCardModel) -> Void)?
public var onKokButtonTapped: ((Domain.PokeUserModel) -> Core.Driver<(Domain.PokeUserModel, Domain.PokeMessageModel, isAnonymous: Bool)>)?
public var onReceiveTodaysFortuneCardButtonTapped: ((Domain.DailySoptuneCardModel) -> Void)?

Expand Down Expand Up @@ -65,7 +65,7 @@ extension DailySoptuneResultViewModel {
input.naviBackButtonTap
.withUnretained(self)
.sink { _ in
self.onNaviBackButtonTap?()
self.onNaviBackButtonTapped?()
}.store(in: cancelBag)

input.receiveTodaysFortuneCardTap
Expand Down