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/#34] 장소 상세 페이지 StoreInfo, StoreInfoItem 구현 #35

Merged
merged 12 commits into from
Jan 16, 2025
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
package com.spoony.spoony.presentation.placeDetail.component

import androidx.compose.foundation.Canvas
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.runtime.Composable
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.blur
import androidx.compose.ui.draw.clip
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.graphics.PathEffect
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import com.spoony.spoony.core.designsystem.theme.SpoonyAndroidTheme

@Composable
fun StoreInfo(
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1: 상호명이 빠졌어요!! 인자로 추가해주세요~~

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
Member

Choose a reason for hiding this comment

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

p1) 너무 외부까지 slot으로 뚫은 것 같아요.
내부에서 일부 선언해서 컴포넌트로 사용할 수 있게 해주세요

현재 Preview를 보시면 컴포넌트를 사용하는데 최소 120줄을 사용하고 있어요.
이것은 컴포넌트라고 볼 수 없을 것 같아요

Copy link
Member Author

Choose a reason for hiding this comment

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

넵 수정했습니다!

menuItems: List<String>,
locationItems: List<String>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2: locationItems가 리스트인 이유가 무엇일까요?? 프리뷰를 보니 주소를 의미하는 인자인 것 같은데 주소는 스트링으로 받아도 충분할 것 같아요!

Copy link
Member

Choose a reason for hiding this comment

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

p2) 리스트를 사용하지 말고, immutableList로 추후 수정해주세요

Copy link
Member Author

Choose a reason for hiding this comment

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

구조가 바뀌어서 수정한 부분 확인 부탁드립니다!

modifier: Modifier = Modifier,
isBlurred: Boolean = true
Copy link
Member

Choose a reason for hiding this comment

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

p2) isBlurred의 default값이 true인 이유가 있을까요?
서버에서 내려주는 값일텐데 디폴트가 존재하는 이유가 없다고 생각돼요

Copy link
Member Author

Choose a reason for hiding this comment

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

보이다가 안보이는 것보단 안보이다가 서버에서 isBlurred 가 false 라는것이 떴을 때 보이도록하는게 좋아보였습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

default값을 없애는 것이 더 좋아보입니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

수정완료했습니다.

) {
Column(
modifier = modifier
.clip(RoundedCornerShape(8.dp))
.then(
if (isBlurred) {
Modifier.blur(16.dp)
} else {
Modifier
}
)
) {
StoreInfoItem(
title = "Menu",
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2: 이 부분 스트링 추출해주면 좋을 것 같네요!

Copy link
Member Author

Choose a reason for hiding this comment

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

추출완료했습니다!

items = menuItems,
isMenu = true
)
HorizontalDashedLine()
StoreInfoItem(
title = "Location",
subTitle = "이키",
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1: 이키?!??!?!?!?!?!??!? 잡았다!!

Copy link
Member Author

Choose a reason for hiding this comment

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

어라라! 수정했습니다! 구조가 많이 바뀌어서 다시한번 확인 부탁드려요!

items = locationItems,
isMenu = false
)
}
}

@Composable
private fun HorizontalDashedLine(
modifier: Modifier = Modifier,
color: Color = SpoonyAndroidTheme.colors.gray200,
strokeWidth: Float = 2f,
dashLengths: FloatArray = floatArrayOf(30f, 30f)
) {
Box(
modifier = modifier
.fillMaxWidth()
.height(1.dp)
) {
Canvas(modifier = Modifier.matchParentSize()) {
val pathEffect = PathEffect.dashPathEffect(
intervals = dashLengths,
phase = 0f
)
val horizontalPadding = 25.dp.toPx()
val yPos = size.height / 2
drawLine(
color = color,
start = Offset(x = horizontalPadding, y = yPos),
end = Offset(x = size.width - horizontalPadding, y = yPos),
strokeWidth = strokeWidth,
pathEffect = pathEffect
)
}
}
}

@Preview(showBackground = true)
@Composable
private fun StoreInfoPreview() {
SpoonyAndroidTheme {
Column(
horizontalAlignment = Alignment.CenterHorizontally,
verticalArrangement = Arrangement.spacedBy(20.dp)
) {
StoreInfo(
modifier = Modifier
.padding(horizontal = 20.dp),
isBlurred = true,
menuItems = listOf(
"고등어봉초밥",
"크렘브륄레"
),
locationItems = listOf(
"서울 서대문구 연희로11가길 39"
)
)
StoreInfo(
modifier = Modifier
.padding(horizontal = 20.dp),
isBlurred = false,
menuItems = listOf(
"고등어봉초밥",
"크렘브륄레"
),
locationItems = listOf(
"서울 서대문구 연희로11가길 39"
)
)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
package com.spoony.spoony.presentation.placeDetail.component

import androidx.compose.foundation.background
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.layout.width
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.material3.Icon
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.remember
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.graphics.vector.ImageVector
import androidx.compose.ui.res.vectorResource
import androidx.compose.ui.text.TextStyle
import androidx.compose.ui.text.style.TextOverflow
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import com.spoony.spoony.R
import com.spoony.spoony.core.designsystem.theme.SpoonyAndroidTheme

@Composable
fun StoreInfoItem(
Copy link
Member

Choose a reason for hiding this comment

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

p1) StoreInfoItem이라는 컴포넌트를 재활용 하기에 불편한 것 같습니다.
isMenu라는 변수로 두가지 상태를 제어한다는 것이 추후 재사용에 상당히 불리한 것 같습니다.

Base를 하나 만들고 이를 활용한 두개의 컴포넌트를 만드는 방법도 있으니 고민해보시면 좋을 것 같습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

구조 수정했습니다 확인 부탁드립니다!

title: String,
subTitle: String? = null,
items: List<String>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2: 필수인자는 위로!!

Copy link
Member Author

Choose a reason for hiding this comment

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

수정했습니다!

isMenu: Boolean = true,
modifier: Modifier = Modifier
Copy link
Collaborator

Choose a reason for hiding this comment

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

P4: modifier는 옵셔널 중 가장 위로 올려주세요😊

Copy link
Member Author

Choose a reason for hiding this comment

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

수정했습니다!

) {
val (shape, paddingValues) = remember(isMenu) {
when (isMenu) {
true -> RoundedCornerShape(
topStart = 8.dp,
topEnd = 8.dp,
bottomStart = 20.dp,
bottomEnd = 20.dp
) to PaddingValues(
top = 20.dp,
bottom = 28.dp,
start = 16.dp,
end = 16.dp
)
false -> RoundedCornerShape(
topStart = 20.dp,
topEnd = 20.dp,
bottomStart = 8.dp,
bottomEnd = 8.dp
) to PaddingValues(
vertical = 22.dp,
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1: 패딩값 다시 한번 확인 부탁드려요~!

Copy link
Member Author

Choose a reason for hiding this comment

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

디쌤분들과 협의하에 수정완료했습니다!

horizontal = 16.dp
)
}
}
Column(
modifier = modifier
.fillMaxWidth()
.background(
shape = shape,
color = SpoonyAndroidTheme.colors.gray0
)
.padding(paddingValues)
) {
Text(
title,
style = SpoonyAndroidTheme.typography.body1b
)
Spacer(modifier = Modifier.height(12.dp))
if (subTitle != null) {
Text(
subTitle,
style = SpoonyAndroidTheme.typography.title2sb
)
Spacer(modifier = Modifier.height(11.dp))
}
Column(
verticalArrangement = Arrangement.spacedBy(12.dp)
) {
items.forEach { item ->
Copy link
Member

Choose a reason for hiding this comment

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

p2) 여기도 key를 추가하면 좋을 것 같습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

수정했습니다!

IconText(
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1: 메뉴일 때랑 상호명일 때 아이콘 색상 달라서 색상도 따로 지정해주세요!

Copy link
Member Author

Choose a reason for hiding this comment

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

같아보이는데 어떤부분 말씀일까요?!

icon = if (isMenu) R.drawable.ic_spoon_24 else R.drawable.ic_pin_24,
iconSize = 20,
text = item,
textStyle = SpoonyAndroidTheme.typography.body2m,
modifier = Modifier.fillMaxWidth()
)
}
}
}
}

@Composable
private fun IconText(
icon: Int,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
icon: Int,
@DrawableRes icon: Int,

P3: 필수는 아니지만,, 이렇게 Drawable id만 받을 수 있도록 설정해주는 것도 좋을 것 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

ImageVector로 수정했습니다!

iconSize: Int,
Copy link
Member

Choose a reason for hiding this comment

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

p3) size를 나타내려면 dp로 받아오는게 직관적인 것 같아요.

Copy link
Member Author

Choose a reason for hiding this comment

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

수정했습니다!

text: String,
textStyle: TextStyle,
modifier: Modifier = Modifier,
textColor: Color = SpoonyAndroidTheme.colors.black,
iconTint: Color = SpoonyAndroidTheme.colors.gray600
) {
Row(
modifier = modifier,
verticalAlignment = Alignment.CenterVertically
) {
Icon(
imageVector = ImageVector.vectorResource(icon),
contentDescription = null,
modifier = Modifier.size(iconSize.dp),
tint = iconTint
)
Spacer(modifier = Modifier.width(4.dp))
Text(
text = text,
color = textColor,
style = textStyle,
maxLines = 1,
overflow = TextOverflow.Ellipsis
)
}
}

@Preview
@Composable
private fun StoreInfoItemMenuPreview() {
SpoonyAndroidTheme {
StoreInfoItem(
title = "Menu",
items = listOf(
"고등어봉초밥고등어봉초밥고등어봉초밥고등어봉초밥고등어봉초밥고등어봉초밥고등어봉초밥고등어봉초밥",
"크렘브륄레"
),
isMenu = true
)
}
}

@Preview
@Composable
private fun StoreInfoItemLocationPreview() {
SpoonyAndroidTheme {
StoreInfoItem(
title = "Location",
subTitle = "이키",
items = listOf(
"서울 서대문구 연희로11가길 39"
),
isMenu = false
)
}
}
Loading