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] OnboardingCoordinator - 플로우 연결 #189

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

FpRaArNkK
Copy link
Contributor

@FpRaArNkK FpRaArNkK commented Oct 8, 2024

Related Issue

연관된 이슈번호를 작성해주세요
#188

Description

이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)

온보딩 플로우에 포함된 화면과 Coordinator의 연결을 작성했습니다.
App 시동 시 AppCoordinator 부터 시작하며, 첫 Splash Coordinator에서 유저 로그인 상태를 체크합니다.
이후 유저 상태에 따라 AppCoordinator에서 로그인,온보딩(OnboardingCoordinator) / 메인(MainTabCoordinator) 화면 분기처리를 진행합니다.

-> 수정되었습니다!
App 시동 시 AppCoordinator 부터 시작하며, 첫 Onboarding Coordinator에서 유저 로그인 상태를 체크합니다.
이후 유저 상태에 따라 온보딩(OnboardingCoordinator에서 라우팅) / 메인(AppCoordinator -> MainTabCoordinator) 화면 분기처리를 진행합니다.

  1. 약관동의, 정보입력 작성 유무 파악을 위해 AuthManager에 추가 기능을 작성했습니다.
  • 해당 부분에서 AuthManager의 코드 작성 구조에 변동 사항이 있습니다.
  1. 온보딩/메인 화면 분기 처리에 필요한 데이터 로직을 AuthManager에 작성했습니다.
  2. AppCoordinator의 구조를 유지하며 Splash, OnboardingCoordinator 를 작성했습니다.
  • Onboarding Coordinator 로 로직을 통합했습니다!

Screenshot (Optional)

Simulator Screen Recording - iPhone 16 Pro - 2024-10-08 at 20 17 30

Requirements for Review

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

  1. Coordinator의 네비게이션 UI는 별도 작업이 필요해보입니다!
  2. Onboarding 관련 여러 추가 로직이 필요합니다. 추가 작업해서 올리겠습니다.

코디네이터 로직 관련해서 좋은 레퍼런스가 될 지는 모르겠습니다..! 이상한 부분 보이면 꼭 말씀 부탁드립니다!
+
수정 필요부분 반영했습니다!

@FpRaArNkK FpRaArNkK added the 💡Feat 새로운 기능 구현 이슈 label Oct 8, 2024
@FpRaArNkK FpRaArNkK self-assigned this Oct 8, 2024
Copy link
Contributor

@iHyunWoo iHyunWoo left a comment

Choose a reason for hiding this comment

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

전반적으로 Coordinator에서 자식의 State, Action을 scope로 들고 있어야하나가 의문입니다.
그거 안해도 .routeAction에서 다 자식 action을 처리할 수 있을 것 같아서요!

ex)

// AppCoordinator.swift
// 최상단 앱 코디네이터에서 splash에서 goToMainScreen 액션을 보내면
// 현재 라우트를 mainTab으로 덮어씌우고 시키는 함수
case .router(.routeAction(_, action: .splash(.goToMainScreen))):
  state.routes = [.root(.mainTab(.init))]

추가로 print 제거 신경 써주시면 감사하겠습니다!


@ObservableState
struct State: Equatable {
var loginState: OnboardingLoginStore.State
Copy link
Contributor

Choose a reason for hiding this comment

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

state를 코디네이터에 저장할 필요가 있나요?
아니라면 각 state와 scope빼고 route로만 관리해도 될 것 같네요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

없어보입니다..! 알려주신대로 state scope aciton 내용 제거해서 반영해보겠습니다

// 약관 동의 화면으로 이동
case .login(.goToNextScreen):
print("되는건가")
state.routes.append(.push(.agreement(state.agreementState)))
Copy link
Contributor

Choose a reason for hiding this comment

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

routes.append 없이 바로 .push 가능합니다.
state.routes.push(.agreement(...

Copy link
Contributor Author

@FpRaArNkK FpRaArNkK Oct 8, 2024

Choose a reason for hiding this comment

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

와 꿀팁 감사합니다... 가독성이 훨씬 좋습니다 반영하겠습니다!


// 약관 동의 화면으로 이동
case .login(.goToNextScreen):
print("되는건가")
Copy link
Contributor

Choose a reason for hiding this comment

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

print 제거 부탁드립니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

앗 삭제를 빼먹었습니다 죄송합니다 ㅠㅠ

splash: .init(),
onBoarding: .init(onBoarding: .init()))
splash: .initialState,
onboarding: .initialState)
var routes: [Route<AppScreen.State>]
var mainTab: MainTabCoordinator.State
Copy link
Contributor

Choose a reason for hiding this comment

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

여기서도 굳이 자식 state를 모두 들고 있을 필요는 없어 보입니다.


// Splash - 로그인 체크 결과 화면 라우팅
case .router(.routeAction(_, action: .splash(let action))):
switch action {
Copy link
Contributor

Choose a reason for hiding this comment

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

이렇게 하면 화면 별 화면 이동을 보기 좋은 것 같습니다.👍👍

.root(.onboarding(.manualState([
.root(.login(.init()), embedInNavigationView: true),
.push(.agreement(.init()))
])))
Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분이 이해가 안되네요.
AppCoordinator -> OnboardingCoordinator를 띄우면서 agreement를 push하는건가요?
그렇다면 그냥
AppCoordinator -> OnboardingCoordinator를 push하면서 그 root로 agreement를 하면 될 것 같습니다.

그러면 manualState 함수도 필요 없어보이는데요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

앱 첫 진입 시 로그인 로직을 체크하여 AppCoordi -> OnboardingCoordi 를 표시하고,
분기처리하여 [root, n번 화면]의 구성을 가져가려 이렇게 구성했습니다..!
AppCoordinator의 라우팅으로 OnboardingCoordinator의 라우팅까지 건드려야해서
일단은 이렇게 구성했습니다

@FpRaArNkK
Copy link
Contributor Author

FpRaArNkK commented Oct 8, 2024

연현님과의 1:1 미팅 후

  1. Coordinator Route 외 내용 정리
  2. SplashCoordi - Onboarding Coordi 통합

으로 개선 작업 방향이 잡혔습니다.
운동만 갔다오고 호딱 반영해서 올리겠습니다!

@FpRaArNkK FpRaArNkK changed the title [Feat] Splash/OnboardingCoordinator - 플로우 연결 [Feat] OnboardingCoordinator - 플로우 연결 Oct 8, 2024
@FpRaArNkK
Copy link
Contributor Author

@iHyunWoo @Seokki-Kwon
수정 필요사항 반영 완료했습니다 확인 부탁드립니다!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💡Feat 새로운 기능 구현 이슈
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants