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/week2/compsose] 컴포즈 2주차 필수과제 #12

Merged
merged 24 commits into from
May 3, 2024

Conversation

chanubc
Copy link
Member

@chanubc chanubc commented Apr 19, 2024

📌𝘐𝘴𝘴𝘶𝘦𝘴

📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯

  • 멀티 리사이클러뷰(프로필,생일,친구,디바이더) 구현
  • 바텀 네비, 그래프 구현
  • 마이페이지로 로직 이동

📷𝘚𝘤𝘳𝘦𝘦𝘯𝘴𝘩𝘰𝘵

KakaoTalk_20240419_222551547.mp4

💬𝘛𝘰 𝘙𝘦𝘷𝘪𝘦𝘸𝘦𝘳𝘴

아직 그래프 개념이 안잡혔네요
컴포즈 어렵다., xml하고파

@chanubc chanubc requested a review from a team April 19, 2024 13:26
@chanubc chanubc self-assigned this Apr 19, 2024
@chanubc chanubc requested review from cacaocoffee and OliviaYJH and removed request for a team April 19, 2024 13:26
@chanubc chanubc requested review from kez-lab and hyoeunjoo April 19, 2024 13:26
Copy link
Member

@OliviaYJH OliviaYJH left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!😊


@Composable
fun TitleWithDivider(title: String) {
HorizontalDivider(
Copy link
Member

Choose a reason for hiding this comment

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

구분선을 어떻게 추가해야할 지 몰랐는데 HorizontalDivider을 사용하면 되네요! 알아가요!

Copy link
Member Author

Choose a reason for hiding this comment

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

저도 이번에 처음 알았습니다

Copy link

@hyoeunjoo hyoeunjoo 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 +77 to +85
maxLines = 1,
modifier = modifier
.border(
width = 1.dp,
color = Color(0xFF60D5A4),
shape = RoundedCornerShape(50.dp)
) // 외곽선 추가
.padding(horizontal = 12.dp, vertical = 4.dp) // 외곽선과 텍스트 사이 간격 조정
)

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.

앗 전 xml은 마음에 들지만 컴포즈는 내놓은 자식이에요 진짜..

.padding(horizontal = 16.dp),
text = data.name,
fontSize = 14.sp,
color = Color(0xFF0E0E0E)

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.

좋습니다!! 적용할게요

contentDescription = "profile",
modifier = Modifier
.size(130.dp)
.aspectRatio(1f / 1f)

Choose a reason for hiding this comment

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

이거 아까 제가 다른 분 코리도 보고 저도 따로 찾아봤는데
1:1 비율이면 그냥 .aspectRatio(1f)로만 써도 되나 보더라구요!

Copy link
Member

Choose a reason for hiding this comment

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

효은이 리뷰보고 defalut 값이나 내부 연산 찾으려다가

부모 크기에 따라 비율 설정도 가능하더라구요 그래서 자료만 좀 남겨봐요

 modifier = Modifier
            .aspectRatioReference(
                aspectRatioWidth = 1f,
                aspectRatioHeight = 1f,
                aspectRatioReference = AspectRatioReference.MIN_PARENT_WIDTH_PARENT_HEIGHT
            )

parent의 크기와 aspectRatio

Copy link
Member Author

Choose a reason for hiding this comment

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

이거 제가 어디다 코드리뷰 남긴것 같은데 이렇게 자세한 레퍼런스는 처음보네요ㅎㅎ 감사합니다!1

Copy link
Member

@cacaocoffee cacaocoffee left a comment

Choose a reason for hiding this comment

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

코드가 깔끔해서 이해가 잘되요 얼른 공부 많이해서 많이 달아볼게요
bottomnavgraph로 화면 전환하는게 신기했어요!
수고하셨습니다!

.fillMaxSize(),
topBar = {
PageTitle("마이 페이지")
bottomBar = {
Copy link
Member

Choose a reason for hiding this comment

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

이렇게 개별적으로 파일만들어서 navigation 연결하는 건 생각 못해봤는데 이렇게도 쓸 수 있군요!

Copy link
Member Author

Choose a reason for hiding this comment

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

저도 이게 맞나 고민입니다..
컴포즈는 정말......

contentDescription = "profile",
modifier = Modifier
.size(130.dp)
.aspectRatio(1f / 1f)
Copy link
Member

Choose a reason for hiding this comment

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

효은이 리뷰보고 defalut 값이나 내부 연산 찾으려다가

부모 크기에 따라 비율 설정도 가능하더라구요 그래서 자료만 좀 남겨봐요

 modifier = Modifier
            .aspectRatioReference(
                aspectRatioWidth = 1f,
                aspectRatioHeight = 1f,
                aspectRatioReference = AspectRatioReference.MIN_PARENT_WIDTH_PARENT_HEIGHT
            )

parent의 크기와 aspectRatio

Copy link
Member

@leeeyubin leeeyubin 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 +34 to +39
@Composable
fun FriendRowOnCard(
modifier: Modifier = Modifier,
data: HomeSealedItem.Friend,
content: @Composable RowScope.() -> Unit = {}
) {
Copy link
Member

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.

이거 잘못쓴거에요ㅠㅠㅠㅠㅠ 컴포즈는 진짜 모르겠네요..

Comment on lines +81 to +86

fun filterAndSortBirthList(birthList: List<HomeSealedItem.Friend>): List<HomeSealedItem.Friend> =
birthList.filter { friendList ->
friendList.date == today || friendList.date == yesterday
}.sortedBy { friendList ->
DateUtils.getDateOrder(friendList.date)
Copy link
Member

Choose a reason for hiding this comment

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

우아,, List에 filter라는 속성이 있는 건가요?

Copy link
Member

Choose a reason for hiding this comment

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

넹~ 원하는대로 filter 후 객체를 반환해줘요~

Copy link
Member Author

Choose a reason for hiding this comment

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

네 마자용 kotlin진짜 별의별게 다있더라구요
filter로 어제랑 오늘 생일인 사람 거르고
그 거른 리스트에서 오늘, 어제 순으로 재 정렬하는 코드입니당

Comment on lines 2 to 14

object KeyStorage {
const val USER_INPUT = "user_input_list"
const val HOME = "home"
const val SEARCH = "search"
const val MY_PAGE = "my_page"
}

object DateKeyStorage {
const val TODAY = 0
const val YESTERDAY = 1
const val NOTHING = 2
}
Copy link
Member

Choose a reason for hiding this comment

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

속성 별로 나눠둔 거 보기 좋네요!

Copy link
Member

@chattymin chattymin left a comment

Choose a reason for hiding this comment

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

xml도 잘하고 컴포즈도 잘하고...
장난없네요 이남자

Comment on lines +57 to +59
.size(50.dp)
.clip(CircleShape)
.aspectRatio(1f),
Copy link
Member

Choose a reason for hiding this comment

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

사이즈를 지정하셨다면 aspectRatio는 설정하지 않으셔도 자동으로 됩니다

Copy link
Member Author

Choose a reason for hiding this comment

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

앗 그렇군요..?!!!

Comment on lines +65 to +66
modifier = modifier
.padding(horizontal = 16.dp),
Copy link
Member

Choose a reason for hiding this comment

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

content에 padding을 주는방식과, spacer를 주는 방식 중 저는 spacer를 주는 방식을 선호합니다.

컴포넌트 자체에 padding이 존재한다면 padding을 주는게 좋지만, 단순 컴포넌트 사이의 공백이라면 spacer로 둬야 유지보수 및 직관적으로 코드를 이해하는데 도움이 된다고 생각합니다

Copy link
Member Author

Choose a reason for hiding this comment

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

이것도 꿀팁이네용 감사합니다!!

Comment on lines +91 to +95
fun BirthRow(
modifier: Modifier = Modifier,
data: HomeSealedItem.Friend,
content: @Composable RowScope.() -> Unit = {}
) {
Copy link
Member

Choose a reason for hiding this comment

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

매개변수를 선언할 때 순서를 지키면 좋을 것 같습니다
Composable 함수의 일관성 때문에 modifier는 optional param의 첫번째 위치로, optional param은 required param 다음에 와야한다고 합니다
image

Copy link
Member Author

Choose a reason for hiding this comment

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

꿀팁 감사합니다!!
xml은 ktlint가 자동정렬 해주는데 compose는 안되나 봐여ㅠ

.fillMaxWidth()
.padding(horizontal = 16.dp),
verticalAlignment = Alignment.CenterVertically,
horizontalArrangement = Arrangement.SpaceBetween
Copy link
Member

Choose a reason for hiding this comment

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

SpaceBetween을 사용하면서 내부에 Spacer를 넣게 된다면 Spacer의 양 옆에도 SpaceBetween으로 인한 공백이 생기게 됩니다.

의도하신 것이라면 괜찮지만, 불필요한 공백이 생길 수도 있습니다

Copy link
Member Author

Choose a reason for hiding this comment

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

우측의 텍스트가 길어짐에 따라 좌측의 텍스트를 침범하지 않도록 spacer를 주었습니다!
이 방법이 맞는지는 모르겠네요.. xml에선 서로 chain준 다음에 margin 줬던 것 같은데..

Comment on lines +149 to +157
Image(
modifier = modifier
.size(64.dp)
.clip(CircleShape)
.aspectRatio(1f),
painter = painterResource(id = data.myProfileImg),
contentDescription = null,
contentScale = ContentScale.Crop
)
Copy link
Member

Choose a reason for hiding this comment

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

compose에서도 이미지 로딩시 coil과 glide를 지원하고있습니다.
적절한 라이브러리를 활용한다면 더 좋겠죠~
참고로 glide는 beta버전이긴 합니다

Copy link
Member Author

Choose a reason for hiding this comment

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

음 그려면 컴포즈는 coil이 조금더 안전하겠네요?
xml과는 반대 느낌이네요 xml은 glide가 좀더 오래 된 걸로 알고 있어서

@@ -18,3 +22,21 @@ fun PageTitle(title: String) {
fontWeight = FontWeight.Bold
)
}

@Composable
fun TitleWithDivider(title: String) {
Copy link
Member

Choose a reason for hiding this comment

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

HorizontalDivider과 Text를 배치해뒀는데 어떤 방향으로 정렬되는 걸까요?
해당 컴포넌트에서 Text와 Divider를 같이 사용하려면 HorizontalDivider인것으로 미루어보아 Column으로 묶어주면 더욱 좋을 것 같습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

그냥 2개를 동시에 놓을시에 수직으로 배치된다고 알고 있었는데 column으로 명시하는게 더 낫나요?
수정하겠습니다~

Copy link
Member

Choose a reason for hiding this comment

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

동시에 놓으면 해당 컴포넌트를 호출한 곳의 정렬 방식을 따라갑니다!!
명시적으로 적어두는게 조아요~

Comment on lines +35 to +36
modifier = Modifier.fillMaxWidth(),
textAlign = TextAlign.Start,
Copy link
Member

Choose a reason for hiding this comment

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

Column으로 묶어주게 되면 maxWidth 후 Start로 설정하는 것이 아니라, Column의 정렬 방향을 통해서 정렬할 수 있게 되겠죵~

Copy link
Member Author

Choose a reason for hiding this comment

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

아하 이러한 뜻이였군요!

.fillMaxSize(),
verticalArrangement = Arrangement.spacedBy(16.dp)
) {
val mockList = viewModel.mockBirthList
Copy link
Member

Choose a reason for hiding this comment

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

이러한 값의 선언은 가능한 UI와 분리하여 해주시는 것을 권장합니다.
viewModel밑에 두거나, Route와 Screen을 분리하여 Route에서 추출 후 Screen으로 넘겨주는 것이 좋아보입니다

Copy link
Member Author

Choose a reason for hiding this comment

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

음 아직 컴포즈에서 viewmodle다루는 법이 감이 안오네요..
좀 더 공부해서 수정해볼게요!!

Copy link
Member Author

Choose a reason for hiding this comment

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

viewmodel을 screen의 파라미터에 넘겨도 되나요? nav 초기화 시에요!

Copy link
Member

Choose a reason for hiding this comment

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

넵 그렇게 해도 됩니담~
물론 전 앞서 말씀드렸듯 Route-Screen 구조를 애용해요

Comment on lines +81 to +86

fun filterAndSortBirthList(birthList: List<HomeSealedItem.Friend>): List<HomeSealedItem.Friend> =
birthList.filter { friendList ->
friendList.date == today || friendList.date == yesterday
}.sortedBy { friendList ->
DateUtils.getDateOrder(friendList.date)
Copy link
Member

Choose a reason for hiding this comment

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

넹~ 원하는대로 filter 후 객체를 반환해줘요~

.fillMaxSize()
.padding(
top = paddingValues.calculateTopPadding(),
bottom = paddingValues.calculateBottomPadding()
Copy link
Member

Choose a reason for hiding this comment

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

bottomPadding은 왜 주고있느걸까요??
Scaffold 내부에서 지금 Bottom과 관련된 값이 없어 주지 않아도 될 것 같습니다.

Copy link
Member Author

@chanubc chanubc Apr 25, 2024

Choose a reason for hiding this comment

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

네 필요 없는 부분이 맞습니다! 지워야겠네요 감사합니다!!

@chanubc chanubc merged commit fd1f4eb into develop-compose May 3, 2024
@chanubc chanubc deleted the feat/week2/compsose_02_essential branch May 3, 2024 12:04
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.

6 participants