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

컴포즈로 마이그레이션 및 기능 구현 #4

Merged
merged 10 commits into from
Oct 30, 2024
Merged

컴포즈로 마이그레이션 및 기능 구현 #4

merged 10 commits into from
Oct 30, 2024

Conversation

towitty
Copy link
Collaborator

@towitty towitty commented Sep 28, 2024

What is this PR? 💬

  • Compose Migration

Changes 👀

  • [migration] FAB 기능
  • [migration] bottom navigation

Screenshot 📷

Question 🤔

onClick = {
navController.navigate(it.name) {
popUpTo(navController.graph.findStartDestination().id) {
saveState = true
}
launchSingleTop = true
restoreState = true
}
}

saveState, launchSingleTop, restoreState, 상태에 대해 제대로 이해한 것이 맞는지 잘 모르겠습니다. saveState 는 현재 백스택을 저장한 후 이동하는 Destination 을 SingleTop 으로 놓고 해당 Destination 의 예전 백스택 상태를 복원하는 것으로 이해했는데, 백스택 상태가 Navigation 의 Backstack 을 말하는 것인지, Composable 에서 remember 로 보관하고 있는 상태들을 말하는 것인지 잘 모르겠습니다...


Comment

프로젝트 구현 속도가 느려, 구현에 집중하였고 별도의 PR로 분리하지 못했습니다.
구현한 내용에 대해서 어떻게 정리하여 전달드리면 좋을지 잘 모르겠습니다.
이후 구현사항에 대해서는 Jira 티켓 별로 브랜치를 나누어 PR 요청 하도록 하겠습니다. 또 괜찮으시다면 현재 브랜치를 main 브랜치에 merge 후 새로운 브랜치룰 나누어 이후 요청드릴 PR과의 충돌을 예방하고 싶습니다.

@towitty towitty changed the title Scrum 69 Migrating to Compose Sep 28, 2024
@lan-at-flab
Copy link

lan-at-flab commented Sep 30, 2024

대화를 위해서는 커멘트를 남겨두는 게 좋아요. 궁금하시 부분에 대해서는 저도 리서치를 해 봐야 할 것 같네요. 백스택에 관련된 내용으로 보입니다.

@towitty
Copy link
Collaborator Author

towitty commented Sep 30, 2024

대화를 위해서는 커멘트를 남겨두는 게 좋아요. 궁금하시 부분에 대해서는 저도 리서치를 해 봐야 할 것 같네요. 백스택에 관련된 내용으로 보입니다.

말씀하신 '커멘트' 가 코드에 대해서 무엇을 하는 코드 인지, 어떤 의도로 작성했는지 등을 코드와 함께 말씀드리는 것일까요 ?

@lan-at-flab
Copy link

PR은 주제에 맞게 별도로 생성하는 게 좋아요. 파일이 너무 크면 리뷰하기가 어렵거든요.

app/build.gradle.kts Outdated Show resolved Hide resolved
@Composable
fun BookInfoDetailTopAppbar(onBack: () -> Unit, onSelection: () -> Unit, modifier: Modifier = Modifier) {
TopAppBar(
title = {},

Choose a reason for hiding this comment

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

title 관련한 function을 하나 만드는 게 어때요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

title 관련한 function을 하나 만드는 게 어때요?

좋은 아이디어 감사합니다... 전에 확인하고 다른 구현을 하느라 미쳐 신경쓰지 못했는데, 지금 보니 AppBar 의 title 은 Composable 함수를 인자를 받고 있네요. 지금은 Screen 마다 별도의 AppBar를 구현하여 사용하고 있는데 중복되는 부분이나 AppBar 만들거나 처리하는 곳을 하나의 function 으로 하면 좋겠다는 생각은 있지만 아직 어떻게 나누고 구현하면 좋을지 잘 모르겠습니다...😥

@towitty towitty changed the title Migrating to Compose 컴포즈로 마이그레이션 및 기능 구현 Oct 24, 2024
@towitty towitty added the enhancement New feature or request label Oct 24, 2024
}
}

fun findBookByIsbn(isbn: String) {

Choose a reason for hiding this comment

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

이 기능이 repository에서 하는 게 좋을 것 같아요

) {
val bitmap: MutableState<Bitmap?> = remember { mutableStateOf(null) }

Glide.with(LocalContext.current).asBitmap().load(bookItem.image).into(object : CustomTarget<Bitmap>() {

Choose a reason for hiding this comment

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

coil이랑 비교선택하면 어떤가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

coil이랑 비교선택하면 어떤가요?

이전에 가볍게 고민해 본적이 었습니다. Glide 공식 홈페이지에 이미지 목록을 가능한 한 부드럽고 빠르게 스크롤할 수 있도록 하는 것이 주된 초점이라고 소개되어 있습니다. Coill vs Glide 의 내용을 보면 제 프로젝트는 반복되는 이미지 처리가 있지 않아 캐싱할 데이터가 없습니다. Coil, Glide 모두 Compose를 위한 라이브러리도 존재하지만 Glide는 beta 버전으로 아직 stable한 버전이 없는 것으로 알고 있습니다. Coil은 Glide 보다 비교적 가볍고 심플함과 최소한의 boilerplate를 위하여 Kotlin의 기능을 활용합니다. 때문에 Coil 이 제 프로젝트에 좀 더 적합하다고 판단되는데요.

Glide 를 선택한것은 프로젝트를 기획하기 전 채용 공고나 회사에서 사용하는 라이브러리들을 찾아 보았을때 아주 조금 더 Glide 를 사용하고 있었습니다. 경험해보면 좋겠다는 생각이 들어 사용했는데 아직 프로젝트에서 Glide 의 장점을 활용하거나 의미있게 사용해보진 못하고 있습니다.

@towitty towitty merged commit 6dd12e5 into main Oct 30, 2024
1 check failed
@towitty towitty deleted the SCRUM-69 branch November 14, 2024 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants