Skip to content

Conversation

@s9hn
Copy link
Contributor

@s9hn s9hn commented Jun 8, 2025

Issue

Overview (Required)

  • Router 모듈을 다음 스크린에 적용합니다.
    • SessionScreen
    • SessionDetailScreen
    • SettingScreen
    • BookmarkScreen

Problem

  • 기존 코드는 홈 스크린 위로 쌓이는 모든 스택을 제거합니다. 당장은 saveState 분기에 맞추어 popUpTo를 호출하도록 구현했습니다.
  • 기존 코드는 탭바 네비게이션에서 사용할 launchSingleTop옵션을 사용할 수 없어 이를 추가했습니다.
  • 현재 2025/app - Domain 모듈의 유즈케이스 인터페이스를 힐트가 Binds 할 수 없어, 스크린 테스트를 진행할 수 없습니다.
    [App] ScreenTest Hilt 빌드 에러 해결 #521 해당 이슈에서 작업하겠습니다.

Question

  • 바텀 탭 루트 객체도 각각 feature-api 모듈로 분리 해야할까요?
  • 현재 Router 모듈의 Navigate 데이터 클래스의 생성자에 launchSingleTop을 추가했습니다. 확장성을 고려해 navOption을 받는게 좋을까요?
  • 현재 Router 구조는 홈 스택이 항상 쌓입니다. 따라서, 설정 및 북마크에서 뒤로가기 이벤트 발생 시 홈으로 돌아가는데 의도된 플로우일까요?

Screenshot

Case 1 Case 2 Case 3 Case 4
홈 -> 설정 -> 홈 홈 -> 북마크 -> 홈 홈 -> 세션 목록 -> 홈 세션 목록 -> 세션 상세보기 -> 세션 목록

Links

s9hn added 17 commits June 5, 2025 17:40
…함수 수정 및 사용하지 않는 MainNavigator 코드 제거
@github-actions
Copy link

github-actions bot commented Jun 8, 2025

Test Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit 644dda6.

♻️ This comment has been updated with latest results.

@taehwandev
Copy link
Member

바텀 탭 루트 객체도 각각 feature-api 모듈로 분리 해야할까요?

  • 탭 이동시에는 불필요해서 분리하지 않아도 됩니다.
    현재 Router 모듈의 Navigate 데이터 클래스의 생성자에 launchSingleTop을 추가했습니다. 확장성을 고려해 navOption을 받는게 좋을까요?
  • 뒤쪽에서 수정된 PR 이 있는데, 지운걸로 보여서 요건 좀 더 고민해보겠습니다
    현재 Router 구조는 홈 스택이 항상 쌓입니다. 따라서, 설정 및 북마크에서 뒤로가기 이벤트 발생 시 홈으로 돌아가는데 의도된 플로우일까요?
  • 뒤쪽에서 수정된 PR 이 있습니다.

@taehwandev
Copy link
Member

@s9hn ./gradlew/detekt 로컬에서 체크 부탁드립니다.

restoreState = true
}
restoreState = sideEffect.saveState
launchSingleTop = sideEffect.launchSingleTop
Copy link
Member

Choose a reason for hiding this comment

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

👍

navController: NavHostController = rememberNavController(),
): MainNavigator = remember(navController) {
MainNavigator(navController)
MainNavigator(navController, onTabClick)
Copy link
Member

Choose a reason for hiding this comment

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

이 라우터 작업은 이 부분을 제거하기 위함이긴 합니다.

view > viewModel에서 각각의 navigate 처리를 하기 위함.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

안녕하세요 태환님 리뷰 감사드립니다.
구현을 고민해보며 MainNavigator에서 MainViewModel을 통한 Navigate가 아닌,
탭 선택 시 상태를 각 스크린에 전파하고 각 스크린의 뷰모델이 Navigate 책임을 이행하도록 수정했습니다.
MainNavigator는 내부적으로 NavController와 MainTab을 활용해 탭 네비게이팅 관련 API를 잘 캡슐화하고 있다고 판단해 그대로 두었습니다.

@taehwandev
Copy link
Member

@s9hn 컴플릭 한번 더 확인 부탁드려요. 수정이 많아서 뒷단에서...

@s9hn
Copy link
Contributor Author

s9hn commented Jun 10, 2025

리베이스를 잘못건드려서 삽질을 좀..했습니다
현재 정상동작합니다!

@taehwandev
Copy link
Member

@s9hn 고생하셨습니다! 생소한 패턴이였을건데 이 부분 이해하시고 적용에 감사드립니다!

@taehwandev
Copy link
Member

머지 후 수정하겠습니다.

@taehwandev taehwandev merged commit 906d62f into droidknights:2025/app Jun 12, 2025
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants