Skip to content

[MOB1-1742] 배너 스펙 확장 - left, right icon을 구분하여 설정할 수 있돌고 수정 #106

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: exp
Choose a base branch
from
Open

[MOB1-1742] 배너 스펙 확장 - left, right icon을 구분하여 설정할 수 있돌고 수정 #106

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
71a0c77
배너 스펙 확장 - left, right icon을 구분하여 설정할 수 있돌고 수정
vvvvvoin Sep 15, 2025
5da2f14
리뷰 반영 - 네임드파라미터 사용하도록 수정
vvvvvoin Sep 19, 2025
a3b640f
리뷰 반영 - 의도하지 않은 deprecated 표시되지 않도록 함수 추가
vvvvvoin Oct 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 102 additions & 24 deletions bezier/src/main/java/io/channel/bezier/component/Banner.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ import androidx.compose.ui.unit.sp
import io.channel.bezier.BezierTheme
import io.channel.bezier.compose.R

@Deprecated(
message = "Use leftIcon and leftIconColor parameters instead",
replaceWith = ReplaceWith("Banner(type = type, description = description, modifier = modifier, title = title, leftIcon = icon, leftIconColor = iconColor, onClick = onClick, onRemove = onRemove, content = content)"),
)
@Composable
fun Banner(
type: BannerType,
Expand All @@ -62,14 +66,19 @@ fun Banner(
},
modifier = modifier,
title = title,
icon = icon,
iconColor = iconColor,
leftIcon = icon,
leftIconColor = iconColor,
rightIcon = null,
onClick = onClick,
onRemove = onRemove,
content = content,
)
}

@Deprecated(
message = "Use leftIcon and leftIconColor parameters instead",
replaceWith = ReplaceWith("Banner(type = type, description = description, modifier = modifier, title = title, leftIcon = icon, leftIconColor = iconColor, onClick = onClick, onRemove = onRemove, content = content)"),
)
@Composable
fun Banner(
type: BannerType,
Expand All @@ -93,8 +102,75 @@ fun Banner(
},
modifier = modifier,
title = title,
icon = icon,
iconColor = iconColor,
leftIcon = icon,
leftIconColor = iconColor,
rightIcon = null,
onClick = onClick,
onRemove = onRemove,
content = content,
)
}

@Composable
fun Banner(
type: BannerType,
description: AnnotatedString,
modifier: Modifier = Modifier,
title: String? = null,
leftIcon: Painter? = null,
leftIconColor: Color = Color.Unspecified,
rightIcon: Painter? = null,
onClick: (() -> Unit)? = null,
Comment on lines +148 to +151
Copy link
Contributor

@malibinYun malibinYun Sep 19, 2025

Choose a reason for hiding this comment

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

rightIconColor는 필요 없나요? 넣는 게 나아보여서요

Copy link
Contributor Author

@vvvvvoin vvvvvoin Sep 19, 2025

Choose a reason for hiding this comment

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

라이트는 버튼 스펙을 따라가서 고정으로 생각해서 색상은 따로 넣지는 않았습니당
그래도 혹시 모르니 요건 디자인팀에 확인해볼게요

onRemove: (() -> Unit)? = null,
content: (@Composable () -> Unit)? = null,
) {
BannerLayout(
type = type,
description = {
Text(
text = description,
color = colorResource(id = type.descriptionColor),
fontSize = 14.sp,
)
},
modifier = modifier,
title = title,
leftIcon = leftIcon,
leftIconColor = leftIconColor,
rightIcon = rightIcon,
onClick = onClick,
onRemove = onRemove,
content = content,
)
}

@Composable
fun Banner(
type: BannerType,
description: String,
modifier: Modifier = Modifier,
title: String? = null,
leftIcon: Painter? = null,
leftIconColor: Color = Color.Unspecified,
rightIcon: Painter? = null,
onClick: (() -> Unit)? = null,
onRemove: (() -> Unit)? = null,
content: (@Composable () -> Unit)? = null,
) {
Comment on lines +175 to +187
Copy link
Contributor

@onebone onebone Sep 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

기본 파라미터 값 때문에 deprecated 된 Banner의 오버로드와 호출부의 형태가 겹쳐버리는 것 같아요. title, descripion, type만 값을 지정하면 의도와 달리 deprecated된 오버로드로 가는 것 같습니다!

기존에 라이브러리를 사용하는 곳에서 icon이 사용되지 않는 경우가 많이 없다면 이대로 두는 것도 방법일 것 같은데, 그렇지 않으면 @Deprecated 지정한 오버로드에 level = DeprecationLevel.HIDDEN을 달아두는 것도 고려해봐야 할 것 같네요. 다만 이러면 source-breaking change가 돼서 minor 버전을 올리거나 해야 할 것 같습니다

BannerLayout(
type = type,
description = {
Text(
text = description,
color = colorResource(id = type.descriptionColor),
fontSize = 14.sp,
)
},
modifier = modifier,
title = title,
leftIcon = leftIcon,
leftIconColor = leftIconColor,
rightIcon = rightIcon,
onClick = onClick,
onRemove = onRemove,
content = content,
Expand All @@ -107,8 +183,9 @@ private fun BannerLayout(
description: @Composable () -> Unit,
modifier: Modifier,
title: String?,
icon: Painter?,
iconColor: Color,
leftIcon: Painter?,
leftIconColor: Color,
rightIcon: Painter?,
onClick: (() -> Unit)?,
onRemove: (() -> Unit)?,
Comment on lines +216 to 218
Copy link
Contributor

@malibinYun malibinYun Sep 19, 2025

Choose a reason for hiding this comment

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

rightIcon를 커스텀할 수 있음으로써 onClick과 onRemove의 차이에 대해 인지하기가 어려울 것으로 우려돼요.
onRemove를 지우고 onClick으로 통일하는 건 어떠신가요?
앞으로 아이콘은 onRemove나 onClick의 nullable여부로 판단해서 표시해주는 게 아닌, 그냥 커스텀 아이콘으로만 표시하도록이요.

Copy link
Contributor Author

@vvvvvoin vvvvvoin Sep 19, 2025

Choose a reason for hiding this comment

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

onClick으로 통합하면 X버튼을 눌러서 닫는 느낌이 아니라 배너 어디든 클릭해서 닫힐거 같아서 뭔가 구분이 필요해 보입니다.
요것도 디자인 의도가 좀 더 명확해야할거 같기도 하네요

content: (@Composable () -> Unit)?,
Expand Down Expand Up @@ -137,13 +214,13 @@ private fun BannerLayout(
}
.padding(vertical = 10.dp, horizontal = 8.dp),
) {
if (icon != null) {
if (leftIcon != null) {
Icon(
modifier = Modifier
.padding(start = 6.dp, top = 5.dp, bottom = 5.dp)
.size(20.dp),
painter = icon,
tint = iconColor.takeOrElse { BezierTheme.colors.txtBlackDark },
painter = leftIcon,
tint = leftIconColor.takeOrElse { BezierTheme.colors.txtBlackDark },
contentDescription = null,
)
}
Expand Down Expand Up @@ -195,7 +272,7 @@ private fun BannerLayout(
modifier = Modifier
.size(30.dp)
.padding(5.dp),
painter = painterResource(id = R.drawable.icon_chevron_small_right),
painter = rightIcon ?: painterResource(id = R.drawable.icon_chevron_small_right),
tint = BezierTheme.colors.txtBlackDark,
contentDescription = null,
)
Expand Down Expand Up @@ -233,8 +310,8 @@ private fun FloatingBannerPreview() {
type = BannerType.Floating,
title = null,
description = "componet ia properies, parent cmobil iespetc",
icon = painterResource(id = R.drawable.icon_lock),
iconColor = BezierTheme.colors.txtBlackDark,
leftIcon = painterResource(id = R.drawable.icon_lock),
leftIconColor = BezierTheme.colors.txtBlackDark,
onRemove = {},
)
}
Expand All @@ -247,8 +324,8 @@ private fun CardBannerPreview() {
type = BannerType.Card,
title = null,
description = "componet ia properies, parent cmobil iespetc",
icon = painterResource(id = R.drawable.icon_lock),
iconColor = BezierTheme.colors.txtBlackDark,
leftIcon = painterResource(id = R.drawable.icon_lock),
leftIconColor = BezierTheme.colors.txtBlackDark,
onRemove = {},
)
}
Expand All @@ -262,8 +339,8 @@ private fun InnerBannerPreview() {
type = BannerType.Inner,
title = "componet ia properies, palent",
description = "shape viw imlaouy",
icon = painterResource(id = R.drawable.icon_lock),
iconColor = BezierTheme.colors.txtBlackDark,
leftIcon = painterResource(id = R.drawable.icon_lock),
leftIconColor = BezierTheme.colors.txtBlackDark,
onRemove = {},
) {
Box(
Expand All @@ -285,8 +362,8 @@ private fun InnerBannerWithoutTitlePreview() {
type = BannerType.Inner,
title = null,
description = "shape viw imlaouy",
icon = painterResource(id = R.drawable.icon_lock),
iconColor = BezierTheme.colors.txtBlackDark,
leftIcon = painterResource(id = R.drawable.icon_lock),
leftIconColor = BezierTheme.colors.txtBlackDark,
onRemove = {},
)
}
Expand Down Expand Up @@ -318,8 +395,8 @@ private fun BannerOnRemovePreview() {
type = BannerType.Inner,
title = "componet ia properies, palent",
description = "shape viw imlaouy",
icon = painterResource(id = R.drawable.icon_lock),
iconColor = BezierTheme.colors.txtBlackDark,
leftIcon = painterResource(id = R.drawable.icon_lock),
leftIconColor = BezierTheme.colors.txtBlackDark,
onRemove = { visibleState = false },
) {
Box(
Expand All @@ -345,8 +422,9 @@ private fun BannerOnClickPreview() {
type = BannerType.Inner,
title = "componet ia properies, palent",
description = "shape viw imlaouy",
icon = painterResource(id = R.drawable.icon_lock),
iconColor = BezierTheme.colors.txtBlackDark,
leftIcon = painterResource(id = R.drawable.icon_lock),
leftIconColor = BezierTheme.colors.txtBlackDark,
rightIcon = painterResource(id = R.drawable.icon_chat_edit),
onClick = {},
) {
Box(
Expand All @@ -372,8 +450,8 @@ private fun BannerOnClickOnRemovePreview() {
type = BannerType.Inner,
title = "componet ia properies, palent",
description = "shape viw imlaouy",
icon = painterResource(id = R.drawable.icon_lock),
iconColor = BezierTheme.colors.txtBlackDark,
leftIcon = painterResource(id = R.drawable.icon_lock),
leftIconColor = BezierTheme.colors.txtBlackDark,
onClick = {},
onRemove = {},
) {
Expand Down