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.

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

[UI] #82 링크 수정 2차 기획 반영 #85

Merged
merged 17 commits into from
Dec 8, 2024
Merged

[UI] #82 링크 수정 2차 기획 반영 #85

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
3528779
[ui] 리마인드 UI 삭제
jiwon2724 Nov 22, 2024
e22a59e
[ui] 포킷 추가 버튼 삭제
jiwon2724 Nov 22, 2024
f3a2c56
[chore] 포킷 버튼 이미지 추가
jiwon2724 Nov 22, 2024
0f915e6
[ui] 포킷 추가하기 UI 영역 추가
jiwon2724 Nov 22, 2024
b216750
[chore] 주석 및 height값 수정
jiwon2724 Dec 3, 2024
8e0e1ab
[chore] Pokit data class 수정
jiwon2724 Dec 3, 2024
9b7a889
[chore] coil 라이브러리 추가
jiwon2724 Dec 3, 2024
f12ba11
[feature] 링크 수정 포킷 리스트 v2 구현
jiwon2724 Dec 3, 2024
80a1a39
[ui] icon_24_xs 추가 및 UI 적용
jiwon2724 Dec 3, 2024
17fa8a4
[chore] 코딩 컨벤션 적용
jiwon2724 Dec 3, 2024
8166db5
[chore] PokitInput 롤백
jiwon2724 Dec 7, 2024
9f382af
[feature] 아이콘 클릭 시 title, url clear 구현
jiwon2724 Dec 7, 2024
75e77c6
[chore] idle 상태 추가
jiwon2724 Dec 7, 2024
0f2b075
[ui] 디자인 시스템 컬러 값 파라미터 추가
jiwon2724 Dec 7, 2024
29f4d5b
[chore] 코딩 컨벤션 반영
jiwon2724 Dec 7, 2024
f0404bc
[chore] 디자인 시스템 컬러 값 파라미터 롤백
jiwon2724 Dec 8, 2024
a78e076
[chore] 코딩 컨벤션 반영
jiwon2724 Dec 8, 2024
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
1 change: 1 addition & 0 deletions core/ui/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ dependencies {
implementation(libs.androidx.ui.graphics)
implementation(libs.androidx.ui.tooling.preview)
implementation(libs.androidx.material3)
implementation(libs.coil.compose)
testImplementation(libs.junit)
androidTestImplementation(libs.androidx.junit)
androidTestImplementation(libs.androidx.espresso.core)
Expand Down
15 changes: 13 additions & 2 deletions core/ui/src/main/java/pokitmons/pokit/core/ui/components/atom/input/PokitInput.kt
View file
Open in desktop
jiwon2724 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Boolean타입의 applyInputDesignSystem 보다는 Color?타입의 tintColor라는 인자를 사용하는 건 어떠신가요?
(만약 tintColor가 null이 아니라면 해당 색상으로 tintColor를 지정하고 null이라면 기존 tint를 적용하는 방향으로)

현재 applyInputDesignSystem 역할이 PokitInputIcon의 getColor에서 state가 PokitInputState.INPUT일 때 아이콘 색상을 PokitTheme.colors.iconPrimary로 적용해주는 것 밖에 없어서 범용성이 떨어질 수 있고 약간 변수명하고 역할이 맞지 않는다고 느껴집니다

Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ fun PokitInput(
isError: Boolean = false,
keyboardActions: KeyboardActions = KeyboardActions.Default,
focusRequester: FocusRequester? = null,
applyInputDesignSystem: Boolean = true,
) {
var focused by remember { mutableStateOf(false) }
val state = remember(focused, isError, readOnly, enable) {
Expand Down Expand Up @@ -80,7 +81,12 @@ fun PokitInput(
}

if (icon?.position == PokitInputIconPosition.LEFT) {
PokitInputIcon(state = state, resourceId = icon.resourceId, onClick = onClickIcon)
PokitInputIcon(
state = state,
resourceId = icon.resourceId,
onClick = onClickIcon,
applyInputDesignSystem = applyInputDesignSystem
)
Box(modifier = Modifier.width(8.dp))
}

Expand All @@ -93,7 +99,12 @@ fun PokitInput(
}

if (icon?.position == PokitInputIconPosition.RIGHT) {
PokitInputIcon(state = state, resourceId = icon.resourceId, onClick = onClickIcon)
PokitInputIcon(
state = state,
resourceId = icon.resourceId,
onClick = onClickIcon,
applyInputDesignSystem = applyInputDesignSystem
)
}
}
}
Expand Down
9 changes: 7 additions & 2 deletions ...n/java/pokitmons/pokit/core/ui/components/atom/input/subcomponents/icon/PokitInputIcon.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@ import pokitmons.pokit.core.ui.theme.PokitTheme
internal fun PokitInputIcon(
state: PokitInputState,
resourceId: Int,
applyInputDesignSystem: Boolean = true,
onClick: (() -> Unit)? = null,
) {
val iconColor = getColor(state = state)
val iconColor = getColor(
state = state,
applyInputDesignSystem = applyInputDesignSystem
)
Copy link
Contributor

Choose a reason for hiding this comment

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

위에 적어놓은 tintColor라는 Color? 타입의 인자를 사용했을 때 예시입니다!

internal fun PokitInputIcon(
    state: PokitInputState,
    resourceId: Int,
    tintColor: Color? = null,
    onClick: (() -> Unit)? = null,
) {
    val iconColor = tintColor ?: getColor(state = state)

Copy link
Member Author

Choose a reason for hiding this comment

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

bb 매우 고민했던 부분입니당ㅎㅎ tintColor가 null이 아닌경우(지금 같이 X아이콘이 회색인 경우)일 때, getColor 함수를 호출 안하게 되어서 PokitInputState 상태에 따라서 컬러 값이 변동이 없을 것 같은데 나잇스한 방법이 있을까요!

코멘트 남긴 부분에선 매우 동의합니다~~! 다만 tint 색상 적용 + 다른 상태일 때도 PokitInputState에 적용된 상태를 따라 갔으면 좋겠어욤

Copy link
Contributor

Choose a reason for hiding this comment

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

아 그러네요, 제 방식으로 하면 내부 State에 따라 tint색상이 안변하네요 ㅠ

떠오르는 몇 가지 아이디어들은 다음과 같습니다!

  1. Map<PokitInputState, Color> 인자를 추가하는 방법
  • 단, 이 PokitInputState는 PokitInput 내에서만 사용하는것이 제 의도였기에 internal로 선언되어 있는데 이 방식을 사용하게 된다면 이 state를 public으로 변경해야 합니다.
  • 또한 PokitInput에서는 인자를 PokitInputState가 아닌 readOnly, enable, isError와 같은 boolean 타입 인자들을 받고 있으며 이 변수들을 통해 내부에서 PokitState를 계산하는 구조이기에 PokitInput을 사용하는 개발자 입장에서 색상을 설정할 때는 PokitInputState를 사용하지만 PokitInput에는 각 Boolean타입의 변수를 설정해야 하는(PokitInput에서 PokitState를 인자로 전달받지 않으므로) 부분에서 약간 혼란이 있을 것 같습니다.
    • 그렇다고 PokitInput을 PokitInputState 인자로 전달받는 구조로 변경하기에는 이를 사용하는 ViewModel들에서 이 PokitInputState를 계산하는 로직을 추가적으로 수행해줘야 해서 불편함이 클 것 같습니다
internal fun PokitInputIcon(
    state: PokitInputState,
    resourceId: Int,
    onClick: (() -> Unit)? = null,
    tintColorMap: Map<PokitInputState, Color>? = null,
) {
   val iconColor = map?.get(state) ?: getColor(state = state)
  1. baseIconColor, readOnlyIconColor, disableIconColor, errorIconColor의 Color? 인자들 추가 후 PokitInput 내부에서 Map<PokitInputState, Color>를 생성하는 방법
  • 이 방식의 경우 인자의 개수가 조금 많이 추가될 수 있다는 단점이 있기는 하지만, 새로 추가된 변수명이 기존 변수명과 연관성이 높아서(enable과 enableIconColor라는 이름) 사용하는 입장에서 조금 더 편할 것 같습니다!
  • 이 경우 PokitInputState를 외부에 노출시키지 않은 상태를 유지할 수 있습니다.
  • 개인적으로는 이 방식이 더 좋지 않을까 생각합니다!
fun PokitInput(
    text: String,
    // 다른 인자 생략,
   readOnly: Boolean = false,
    enable: Boolean = true,
    isError: Boolean = false,
    // 아래부터 추가되는 인자
    baseIconColor: Color? = null,
    readOnlyIconColor: Color? = null,
    disableIconColor: Color? =  null,
    errorIconColor: Color? = null,
)  {
    // 이걸 PokitInputIcon에 전달
    val colorStateMap = remember(focused, isError, readOnly, enable, baseIconColor, readOnlyIconColor, disableIconColor, errorIconColor) {
        // 아래 함수는 새로 생성해야 합니다
        getColorStateMap(
            enabled = enable,
            readOnly = readOnly,
            focused = focused,
            error = isError,
            text = text,
            baseIconColor = baseIconColor,
            readOnlyIconColor = readOnlyIconColor,
            disableIconColor = disableIconColor,
            errorIconColor = errorIconColor,
        )
    }

Copy link
Member Author

Choose a reason for hiding this comment

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

음 세환 이건 어때 상태에 따라서 tintColor를 강제하지말고, 그냥 resourceId(아이콘 drawble)자체를 받게하는 케이스

위 처럼 했을 경우에, 기본 아이콘 drawble은 가져가되, PokitInputState 상태에 따라서 다 충족 시킬 수 있을 것 같은데, 함 생각해주셔유

Copy link
Contributor

Choose a reason for hiding this comment

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

살짝 이해가 안되는데 형이 말한 방법 사용한다면 PokitInputState에 따라서 tint를 변경시키려면 어떻게 작성해야 해?

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
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.

반영완 approval 부탁함미다


Icon(
painter = painterResource(id = resourceId),
Expand All @@ -40,11 +44,12 @@ internal fun PokitInputIcon(
@Composable
private fun getColor(
state: PokitInputState,
applyInputDesignSystem: Boolean = true,
): Color {
return when (state) {
PokitInputState.DEFAULT -> PokitTheme.colors.iconSecondary

PokitInputState.INPUT -> PokitTheme.colors.iconPrimary
PokitInputState.INPUT -> if (applyInputDesignSystem) PokitTheme.colors.iconPrimary else PokitTheme.colors.iconSecondary

PokitInputState.ACTIVE -> PokitTheme.colors.iconPrimary

Expand Down
20 changes: 19 additions & 1 deletion core/ui/src/main/java/pokitmons/pokit/core/ui/components/block/labeledinput/LabeledInput.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import androidx.compose.ui.res.painterResource
import androidx.compose.ui.unit.dp
import pokitmons.pokit.core.ui.R
import pokitmons.pokit.core.ui.components.atom.input.PokitInput
import pokitmons.pokit.core.ui.components.atom.input.attributes.PokitInputIcon
import pokitmons.pokit.core.ui.components.atom.input.attributes.PokitInputIconPosition
import pokitmons.pokit.core.ui.components.atom.input.attributes.PokitInputState
import pokitmons.pokit.core.ui.theme.PokitTheme

Expand All @@ -37,8 +39,11 @@ fun LabeledInput(
readOnly: Boolean = false,
enable: Boolean = true,
isError: Boolean = false,
onClickRemove: () -> Unit = {},
applyInputDesignSystem: Boolean = true,
) {
var focused by remember { mutableStateOf(false) }

val state = remember(focused, isError, readOnly, enable) {
getState(
enabled = enable,
Expand Down Expand Up @@ -67,7 +72,20 @@ fun LabeledInput(
text = inputText,
hintText = hintText,
onChangeText = onChangeText,
icon = null,
applyInputDesignSystem = applyInputDesignSystem,
icon = if (inputText.isNotEmpty()) {
PokitInputIcon(
position = PokitInputIconPosition.RIGHT,
resourceId = R.drawable.icon_24_xs
)
} else {
null
},
onClickIcon = remember {
{
onClickRemove()
}
},
isError = isError,
enable = enable,
readOnly = readOnly
Expand Down
97 changes: 97 additions & 0 deletions core/ui/src/main/java/pokitmons/pokit/core/ui/components/block/pokitlist/PokitListVer2.kt
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

이 파일 아예 Ver2로 분리하지 않고 기존 PokitList에 imageUrl을 optional하게 추가하는 건 어떻게 생각하시나유

Copy link
Member Author

Choose a reason for hiding this comment

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

일단 피그마 디자인 네이밍 따왔는데, 기존 PokitList UI를 더이상 사용 안한다면 imageUrl 프로퍼티 넣어두 괜찮을 것 같아여

Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
package pokitmons.pokit.core.ui.components.block.pokitlist

import androidx.compose.foundation.clickable
import androidx.compose.foundation.interaction.MutableInteractionSource
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.Spacer
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.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.layout.ContentScale
import androidx.compose.ui.text.style.TextOverflow
import androidx.compose.ui.unit.dp
import coil.compose.AsyncImage
import pokitmons.pokit.core.ui.components.block.pokitlist.attributes.PokitListState
import pokitmons.pokit.core.ui.theme.PokitTheme

@Composable
fun <T> PokitListVer2(
item: T,
title: String,
sub: String,
imageUrl: String,
onClickItem: (T) -> Unit,
modifier: Modifier = Modifier,
state: PokitListState = PokitListState.DEFAULT,
) {
val titleTextColor = getTitleTextColor(state = state)
val subTextColor = getSubTextColor(state = state)

Row(
modifier = modifier
.clickable(
enabled = state != PokitListState.DISABLE,
onClick = { onClickItem(item) },
indication = null,
interactionSource = remember { MutableInteractionSource() }
)
.padding(horizontal = 20.dp, vertical = 12.dp),
verticalAlignment = Alignment.CenterVertically
) {
AsyncImage(
model = imageUrl,
contentDescription = null,
contentScale = ContentScale.Crop,
modifier = Modifier.size(60.dp)
)

Spacer(modifier = Modifier.width(12.dp))

Column(
modifier = Modifier.weight(1f)
) {
Text(
text = title,
style = PokitTheme.typography.body1Bold.copy(color = titleTextColor),
overflow = TextOverflow.Ellipsis
)

Spacer(modifier = Modifier.height(4.dp))

Text(
text = sub,
style = PokitTheme.typography.detail1.copy(color = subTextColor)
)
}
}
}

@Composable
private fun getTitleTextColor(
state: PokitListState,
): Color {
return when (state) {
PokitListState.DEFAULT -> PokitTheme.colors.textPrimary
PokitListState.ACTIVE -> PokitTheme.colors.textPrimary
PokitListState.DISABLE -> PokitTheme.colors.textDisable
}
}

@Composable
private fun getSubTextColor(
state: PokitListState,
): Color {
return when (state) {
PokitListState.DEFAULT -> PokitTheme.colors.textTertiary
PokitListState.ACTIVE -> PokitTheme.colors.textTertiary
PokitListState.DISABLE -> PokitTheme.colors.textDisable
}
}
10 changes: 10 additions & 0 deletions core/ui/src/main/res/drawable/icon_24_xs.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="24dp"
android:height="24dp"
android:viewportWidth="24"
android:viewportHeight="24">
<path
android:pathData="M6.564,6.563C6.915,6.212 7.485,6.212 7.836,6.563L12,10.727L16.164,6.563C16.515,6.212 17.085,6.212 17.436,6.563C17.788,6.915 17.788,7.485 17.436,7.836L13.273,12L17.436,16.163C17.788,16.515 17.788,17.085 17.436,17.436C17.085,17.788 16.515,17.788 16.164,17.436L12,13.273L7.836,17.436C7.485,17.788 6.915,17.788 6.564,17.436C6.212,17.085 6.212,16.515 6.564,16.163L10.727,12L6.564,7.836C6.212,7.485 6.212,6.915 6.564,6.563Z"
android:fillColor="#9D9D9D"
android:fillType="evenOdd"/>
</vector>
18 changes: 18 additions & 0 deletions core/ui/src/main/res/drawable/image_add_pokit.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="40dp"
android:height="40dp"
android:viewportWidth="40"
android:viewportHeight="40">
<path
android:pathData="M0.5,20C0.5,9.23 9.23,0.5 20,0.5C30.77,0.5 39.5,9.23 39.5,20C39.5,30.77 30.77,39.5 20,39.5C9.23,39.5 0.5,30.77 0.5,20Z"
android:fillColor="#ffffff"/>
<path
android:strokeWidth="1"
android:pathData="M0.5,20C0.5,9.23 9.23,0.5 20,0.5C30.77,0.5 39.5,9.23 39.5,20C39.5,30.77 30.77,39.5 20,39.5C9.23,39.5 0.5,30.77 0.5,20Z"
android:fillColor="#00000000"
android:strokeColor="#D9D9D9"/>
<path
android:pathData="M20,12.5C20.46,12.5 20.833,12.873 20.833,13.333V19.167H26.667C27.127,19.167 27.5,19.54 27.5,20C27.5,20.46 27.127,20.833 26.667,20.833H20.833V26.667C20.833,27.127 20.46,27.5 20,27.5C19.54,27.5 19.167,27.127 19.167,26.667V20.833H13.333C12.873,20.833 12.5,20.46 12.5,20C12.5,19.54 12.873,19.167 13.333,19.167H19.167V13.333C19.167,12.873 19.54,12.5 20,12.5Z"
android:fillColor="#D9D9D9"
android:fillType="evenOdd"/>
</vector>
Loading
Loading