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

[jiwon2724]

Key Changes

피그마에 정의된 2차 기획 링크 수정 반영
Resolves: #82

PR 유형

어떤 변경 사항이 있나요?

• 새로운 기능 추가
• 버그 수정
• CSS 등 사용자 UI 디자인 변경
• 코드에 영향을 주지 않는 변경사항(오타 수정, 탭 사이즈 변경, 변수명 변경)
• 코드 리팩토링
• 주석 추가 및 수정
• 문서 수정
• 테스트 추가, 테스트 리팩토링
• 빌드 부분 혹은 패키지 매니저 수정
• 파일 혹은 폴더명 수정
• 파일 혹은 폴더 삭제

To Reviewers

• 이상하다 싶으면 바루 말해주세여

PR Checklist

PR이 다음 요구 사항을 충족하는지 확인하세요.

• 커밋 메시지 컨벤션에 맞게 작성했습니다.
• 정해진 코딩 컨벤션에 맞게 작성했습니다.
• 변경 사항에 대한 테스트를 했습니다.(버그 수정/기능에 대한 테스트)

Etc.

• 저번에 구두로 말한 바텀시트 포킷 추가 화면에 약 1초정도 머무르다 사라지는 현상 어디를 수정해야하는지 모르겠어요ㅠㅠ 피드백 주시면 감사하겟슴미다
• PokitInput에 X버튼 추가는 했는데, X버튼 클릭 시 text를 빈 문자열로 초기화 하는 나잇스한 방법이 있을까욤 remember로 해봤는데 잘 안되네여 피드백좀 plz ㅠㅠ

[l5x5l]

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

[jiwon2724]

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

[l5x5l]

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

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

[l5x5l]

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

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

[jiwon2724]

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

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

[l5x5l]

아 그러네요, 제 방식으로 하면 내부 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)

2. 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,
        )
    }

[jiwon2724]

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

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

[l5x5l]

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

[jiwon2724]

아 안될듯..ㅋ 그냥 이거 하지말까?ㅋㅋㅋㅋㅋ

[l5x5l]

ㅋㅋㅋㅋ 그럼 수정하지 말고 일단 냅둡시다

[jiwon2724]

반영완 approval 부탁함미다

[jiwon2724]

ㅠㅠ 실버 불릿은 없고만 좋은 의견 베리굿 고민해보구 코멘트 남길겡 2024년 12월 7일 (토) 오후 7:27, Sehwan Yun ***@***.***>님이 작성:

…

***@***.**** commented on this pull request. ------------------------------ In core/ui/src/main/java/pokitmons/pokit/core/ui/components/atom/input/subcomponents/icon/PokitInputIcon.kt <#85 (comment)> : > + val iconColor = getColor( + state = state, + applyInputDesignSystem = applyInputDesignSystem + ) 아 그러네요, 제 방식으로 하면 내부 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) 2. 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, ) } — Reply to this email directly, view it on GitHub <#85 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AQXC3FEWWYJU37P7TYEPQHL2ELES5AVCNFSM6AAAAABS5U4KL2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIOBWGQ3TINZZGM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[l5x5l]

고생많았어! bb