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

kuring-168 메인 화면을 compose로 migrate #235

Merged
merged 10 commits into from
Jun 19, 2024

Conversation

mwy3055
Copy link
Member

@mwy3055 mwy3055 commented Jun 18, 2024

https://kuring.atlassian.net/browse/KURING-168?atlOrigin=eyJpIjoiNWY0MTI0YmMzYjZmNGFlZWI3MGZkNDE5ZjI5YWEyODAiLCJwIjoiaiJ9

작업 내용

메인 화면에 compose type-safe navigation을 적용하였습니다.

Route 선언 및 navigation graph 생성 모두 기본적인 API만으로 작업하였습니다. 더 좋은 방법론이 있다면 적극 리뷰 부탁드립니다.

  • 73c69d7: Route 4개 선언
  • 2f473fb: Bottom navigation bar 선언
  • 5f5ddef: 루트 4개에 대한 navigation graph 생성
  • c09f2cf: MainActivity에서 XML 대신 MainScreen composable을 사용

이후 작업

  1. 메인 탭 4개의 navigation 선언
  2. 캠퍼스맵 화면을 Compose로 migrate
  3. SettingScreen도 type-safe navigation으로 전환 (현재는 String 기반)
  4. 사용하지 않는 XML 등 제거

Copy link
Contributor

@l2hyunwoo l2hyunwoo left a comment

Choose a reason for hiding this comment

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

리뷰 확인해주시고, 구현 이후에 바로 머지해주세요

Comment on lines 117 to 119
override fun <V : AnimationVector> vectorize(typeConverter: TwoWayConverter<Float, V>): VectorizedDecayAnimationSpec<V> {
TODO("Not yet implemented by mwy3055")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

이거는 아직 구현 안된건가요?

Copy link
Member Author

@mwy3055 mwy3055 Jun 19, 2024

Choose a reason for hiding this comment

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

이번에 Navigation 버전을 업그레이드할 때 androidx.compose.foundation:foundation-android가 함께 업글되면서 추가된 매개변수인데, 용례를 거의 찾아보기 힘든 클래스라 일단 TODO로 놔뒀습니다. (이후 PR에서 구현할 예정이었음)

하루 정도 검색해본 결과, 사용 예시가 거의 없는 반면 스위치 애니메이션에 미치는 영향은 미미하다고 판단하여 da662ae 에서 가장 basic한 형태로만 구현했습니다.

}
}
composable<MainScreenRoute.Settings> {
// TODO by mwy3055: SettingScreen 내부도 navigation으로 migrate해야 함
Copy link
Contributor

Choose a reason for hiding this comment

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

여긴 아직 마이그레이션이 안되어있군요ㅋㅋㅋ 나중에 진행하시죠

Copy link
Member Author

Choose a reason for hiding this comment

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

Activity와 기존 string 기반 내비게이션이 섞여 있습니다. 전부 compose + type-safe로 통일하는 게 목표.

@mwy3055 mwy3055 merged commit c610f27 into develop Jun 19, 2024
1 check passed
@mwy3055 mwy3055 deleted the feature/kuring-168_main_screen_navigation branch June 19, 2024 05:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants