[Feat/#5] Design System Setting #8

Roel4990 · 2025-01-04T08:12:08Z

Related issue 🛠

closed [FEAT] 디자인시스템 세팅 #5

Work Description ✏️

디자인 시스템 세팅했습니다.
- Color.kt
- Theme.kt
- Type.kt

Preview

To Reviewers 📢

SOPT 안드로이드 레포 참고했습니다. 아래 링크 참고해주세요.
SOPT-Android

chattymin

너무잘하셔서 더이상 손댈게 없어보이네요 ㅎㅎㅎ
처음 하시는거라 경험하지 못하지면 놓칠 수 밖에 없는 부분이라 한번만 더 확인 부탁드릴게요!
확인하시고 한번 더 리퀘스트 해주세요~ :)

chattymin · 2025-01-04T08:41:37Z

app/src/main/java/com/spoony/spoony/core/designsystem/theme/Type.kt

이렇게 작성하시면 Android Compose의 기본 설정때문에 lineHeight가 적용이 안돼요.
관련 링크 확인하시고 적용 부탁드릴게요~

관련 링크 참고해서 수정했습니다! Preview도 만들었어요!!

Hyobeen-Park

우왕 디자인시스템 세팅 처음이신데도 너무 잘하셨는데요!?!?!??
값이 살짝 잘못 들어간 부분들이 있어서 그 부분만 살짝쿵 수정 부탁드려요😊

Hyobeen-Park · 2025-01-04T14:22:04Z

app/src/main/java/com/spoony/spoony/core/designsystem/theme/Color.kt

+val green400 = Color(0xFF00EB92)
+val blue400 = Color(0xFF6A92EF)


Suggested change

val green400 = Color(0xFF00EB92)

val blue400 = Color(0xFF6A92EF)

val green400 = Color(0xFF00CB92)

val blue400 = Color(0xFF6A92FF)

요기 살짝만 수정 해주세요!!

수정했습니다 감사해요!!!

수정했습니다!

Hyobeen-Park · 2025-01-04T15:02:30Z

app/src/main/java/com/spoony/spoony/core/designsystem/theme/Type.kt

+            lineHeight = 23.2.sp,
+            letterSpacing = 0.32.sp,


lineHeight와 letterSpacing 다시 확인해봐야 할 것 같아요!! 값이 살짝 잘못된 것 같네요..!!

sp => em으로 수정했습니다..!

수정완료했어요!!

chattymin

고생했습니다~~~~
채고채고! 👍

chattymin · 2025-01-05T13:24:59Z

app/src/main/java/com/spoony/spoony/core/designsystem/theme/Type.kt

+private fun SpoonyTextStyle(
+    fontFamily: FontFamily,
+    fontSize: TextUnit,
+    lineHeight: TextUnit,
+    letterSpacing: TextUnit
+): TextStyle = TextStyle(
+    fontFamily = fontFamily,
+    fontSize = fontSize,
+    lineHeight = lineHeight,
+    letterSpacing = letterSpacing,
+    lineHeightStyle = LineHeightStyle(
+        alignment = LineHeightStyle.Alignment.Center,
+        trim = LineHeightStyle.Trim.None


Suggested change

private fun SpoonyTextStyle(

fontFamily: FontFamily,

fontSize: TextUnit,

lineHeight: TextUnit,

letterSpacing: TextUnit

): TextStyle = TextStyle(

fontFamily = fontFamily,

fontSize = fontSize,

lineHeight = lineHeight,

letterSpacing = letterSpacing,

lineHeightStyle = LineHeightStyle(

alignment = LineHeightStyle.Alignment.Center,

trim = LineHeightStyle.Trim.None

private fun SpoonyTextStyle(

fontFamily: FontFamily,

fontSize: TextUnit,

lineHeight: TextUnit = 1.45.em,

letterSpacing: TextUnit = (-0.02).em

): TextStyle = TextStyle(

fontFamily = fontFamily,

fontSize = fontSize,

lineHeight = lineHeight,

letterSpacing = letterSpacing,

lineHeightStyle = LineHeightStyle(

alignment = LineHeightStyle.Alignment.Center,

trim = LineHeightStyle.Trim.None

이렇게 디폴트 값을 넣는건 어떨까요??
아래 코드를 보니 모두 저 값이 디폴트로 들어가는 것 같아용

좋은데요!! 수정하겠습니다!!

chattymin · 2025-01-05T13:27:01Z

app/src/main/java/com/spoony/spoony/core/designsystem/theme/Theme.kt

+@Stable
+class SpoonyColors(


Color와 관련된 내용을 Color가 아닌 Theme에 둔 이유가 있나요??
Typography는 해당 파일에 뒀는데 Color는 여기 두는게 맞나? 라는 생각이 드네요..

혹시 다들 어떻게 생각하시나요??

오호 저도 미처 생각하지 못했는데 SpoonyColors는 Color에 두는게 더 어울릴 것 같기도 하네요!!

저는 Color.kt에 옮기는게 맞다고 생각해요. Theme.kt는 전반적인 테마 구성과 Provider 설정에 집중하고, 색상 정의와 관리는 별도의 관심사이므로 분리하는 것이 코드 구조상 더 명확하다고 생각합니다

넵 수정할게요!

Hyobeen-Park

린트도 한번만 체크 부탁드립니다~~!!

angryPodo · 2025-01-05T14:48:13Z

app/src/main/java/com/spoony/spoony/core/designsystem/theme/Theme.kt

+        gray900 = other.gray900
+        isLight = other.isLight
+    }
+}


P2: 각 색상마다 mutableStateOf와 private set이 반복되어 코드가 불필요하게 길어지는 것 같아요.
컴포즈 컬러 시스템 세팅 아티클
이거 보내드리는걸 깜빡했네요. 위의 글처럼 적용한다면 어노테이션으로 불변성을 보장하고 data class로 copy,equals를 생성할 필요가 없어요. 이 방법으로 적용해보시면 코드가 더 간결해질 것 같습니다.

이 부분은 저희와 다른 목적이 있어서 그런 것 같네요
예시로 보내주신 프로젝트를 보시면 color 시스템의 dark 모드를 가정하고 있지 않아요.
저희도 물론 현재는 다크모드를 지원하지 않고있지만 추후 확장성을 생각한다면 다크모드 지원이 가능한 형태로 초기 설정을 하는게 좋을 것 같아요.

예시 아티클의 경우 Immutable로 제한을 걸어주고 있고, val로 읽기 전용으로 설정해주고 있어요. 그렇기때문에 색상을 변경하려면 객체를 다시 생성해줘야 하는 문제가 있어요. 그렇게 된다면 앱 내에서 테마 색상이 변경된다거나 앱을 사용하는 중에 사용자가 시스템 테마를 변경하는 상황등에는 대응이 되지 않아요. 이를 방지하기 위해 mutableStateOf와 private set을 사용한거에요

data class를 사용한다고 해도 주된 목적인 update는 구현되어있지 않아 직접 구현해줘야 해요.

그렇기에 지금과 같은 형태를 유지해도 좋을 것 같아요. 어떻게 생각하시나요??
@angryPodo @Roel4990

두분 의견 확인 후 남겨봅니다..!
저는 앱잼 이후 Dark 모드의 확장성까지 고려한다면 기존 방식대로 유지하는건 어떨까 싶습니다!

angryPodo · 2025-01-05T14:55:31Z

app/src/main/java/com/spoony/spoony/core/designsystem/theme/Theme.kt

+
+@Composable
+fun SpoonyAndroidTheme(darkTheme: Boolean = false, content: @Composable () -> Unit) {
+    val colors = SpoonyLightColors()
+    val typography = SpoonyTypography()
+    ProvideSpoonyColorsAndTypography(colors, typography) {
+        MaterialTheme(content = content)
+    }
+}


P2: Provider 함수가 Colors와 Typography를 함께 처리하여 유연성이 부족한 것 같아요. 이 부분도 아티클 참고하시면 될 것 같습니다.
예시는 아래같이 작성하면 좋을 것 같아요.

@Composable fun SpoonyTheme( darkTheme: Boolean = false, colors: SpoonyColors = defaultSpoonyColors, typography: SpoonyTypography = defaultSpoonyTypography, content: @Composable () -> Unit ) { ProvideSpoonyTheme( colors = colors, typography = typography ) { MaterialTheme(content = content) } }

P5: 'Android'라는 이름을 붙이는게 불필요한 플랫폼 지정자라고 생각해요 그래서 제거하는게 좋은것 같아요. 다른 분들 생각은 어떠실지..!

이것도 현재 상황에서는 맞는 이야기에요! 하지만 추후 다크모드를 가정하게 된다면 지금과 같은 방식을 사용하는 것이 좋아보입니다.

다크모드를 적용하게 된다면 코드가 아래처럼 바뀌게 될거에요

Suggested change

@Composable

fun SpoonyAndroidTheme(darkTheme: Boolean = false, content: @Composable () -> Unit) {

val colors = SpoonyLightColors()

val typography = SpoonyTypography()

ProvideSpoonyColorsAndTypography(colors, typography) {

MaterialTheme(content = content)

}

}

@Composable

fun SpoonyAndroidTheme(

darkTheme: Boolean = isSystemInDarkTheme(),

content: @Composable () -> Unit

) {

val colors = if (darkTheme) spoonyDarkColors() else spoonyLightColors()

val typography = SpoonyTypography()

ProvideSpoonyColorsAndTypography(colors, typography) {

MaterialTheme(content = content)

}

}

typography의 경우 변경될 일이 없을 것 같아서 제시해주신 방법대로 하는 것이 더 좋을 것 같네요!!

Android prefix의 경우에도 없는것이 더 좋아보입니다! 꼼꼼한 리뷰 최고~

좋은 리뷰 감사합니다..!

angryPodo · 2025-01-05T15:03:43Z

app/src/main/java/com/spoony/spoony/core/designsystem/theme/Theme.kt

+    val provideColors = remember { colors.copy() }
+    provideColors.update(colors)
+    val provideTypography = remember { typography.copy() }.apply { update(typography) }
+    CompositionLocalProvider(
+        LocalSpoonyColors provides provideColors,
+        LocalSpoonyTypography provides provideTypography,


P2: Provider구현 방식에서 colors와 typography를 복사하고 업데이트하는게 복잡한 로직이라고 생각해요.

@Composable fun ProvideSpoonyTheme( colors: SpoonyColors = defaultSpoonyColors, typography: SpoonyTypography = defaultSpoonyTypography, content: @Composable () -> Unit ) { CompositionLocalProvider( LocalSpoonyColors provides colors, LocalSpoonyTypography provides typography ) { content() } } // LocalComposition 정의 개선 이 부분은 Color.kt, Type.kt에 추가하시면 될 것 같습니다. internal val LocalSpoonyColors = staticCompositionLocalOf { defaultSpoonyColors } internal val LocalSpoonyTypography = staticCompositionLocalOf { defaultSpoonyTypography }

위처럼 개선하면

간소화된 Provider 구현

기본값 제공으로 사용성 향상

불변 객체 사용으로 안정성 확보
가 가능할 것 같습니다!

이 부분도 위와 같은 이유에요!

color와 typography가 변경되지 않는다면 상관 없겠지만, 실행중 시스템 컬러나 테마가 바뀌었을 때를 대비하여 remember로 묶어둔거라 필요한 코드인 것 같네요

좋은 의견 감사합니다!

angryPodo

이것저것 신경쓸게 꽤 많아서 귀찮으셨을텐데 컬러 시스템 적용하시느라 고생하셨습니다!
세홍님께 아티클 보내드리는걸 깜빡했었어요. 다시 첨부드리니 확인해보시고 맘에 드신다면 아티클 버전으로 수정해보셔도 좋을 것 같습니다!
컬러 시스템 컬러 적용
컬러 시스템 폰트 적용

…Color.kt 파일로 이동 및 Color Preview 추가

Hyobeen-Park

캬캬 수고하셨습니다!!! 어푸푸어푸푸~~

Roel4990 added 4 commits January 4, 2025 14:48

[FEAT/#5] Pretendard Font 추가

8fe6171

[FEAT/#5] SpoonyTypography 추가

01df115

[FEAT/#5] Color 및 Theme 구현

9e7a4eb

[FEAT/#5] Spoony Typography 방식 수정

7600ec6

Roel4990 added 🧡세홍🧡 🧡세홍🧡 FEAT✨ 새로운 기능 구현 labels Jan 4, 2025

Roel4990 requested review from chattymin, Hyobeen-Park and angryPodo January 4, 2025 08:12

Roel4990 self-assigned this Jan 4, 2025

Roel4990 linked an issue Jan 4, 2025 that may be closed by this pull request

[FEAT] 디자인시스템 세팅 #5

Closed

1 task

chattymin requested changes Jan 4, 2025

View reviewed changes

Hyobeen-Park requested changes Jan 4, 2025

View reviewed changes

Roel4990 added 3 commits January 5, 2025 14:06

[FEAT/#5] Color.kt 잘못된 입력값 수정

5ad817e

[FEAT/#5] lineHeight, letterSpacing 단위 sp 에서 em 으로 수정

b0746e7

[FEAT/#5] lineHeight 적용 안되는 오류 수정(SpoonyTextStyle 추가)

d783662

Roel4990 requested review from Hyobeen-Park and chattymin January 5, 2025 08:23

chattymin approved these changes Jan 5, 2025

View reviewed changes

Hyobeen-Park reviewed Jan 5, 2025

View reviewed changes

angryPodo reviewed Jan 5, 2025

View reviewed changes

angryPodo approved these changes Jan 5, 2025

View reviewed changes

[FEAT/#5] SpoonyTextStyle 고정되는 디폴트값 추가

3e80e21

Roel4990 requested a review from a team as a code owner January 6, 2025 05:19

Roel4990 added 2 commits January 6, 2025 16:39

[FEAT/#5] SpoonyTextStyle 고정되는 디폴트값 추가 및 Theme.kt 안에 있는 SpoonyColors …

fe88396

…Color.kt 파일로 이동 및 Color Preview 추가

[FEAT/#5] ktLintFormat 및 ktLintCheck 처리 완료

a632ed5

Roel4990 requested a review from Hyobeen-Park January 6, 2025 09:10

Hyobeen-Park approved these changes Jan 6, 2025

View reviewed changes

Roel4990 merged commit 920d72d into develop Jan 6, 2025
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feat/#5] Design System Setting #8

[Feat/#5] Design System Setting #8

Roel4990 commented Jan 4, 2025 •

edited

Loading

chattymin left a comment

chattymin Jan 4, 2025 •

edited

Loading

Roel4990 Jan 5, 2025

Hyobeen-Park left a comment

Hyobeen-Park Jan 4, 2025

Roel4990 Jan 5, 2025

Roel4990 Jan 5, 2025

Hyobeen-Park Jan 4, 2025

Roel4990 Jan 5, 2025

Roel4990 Jan 5, 2025

chattymin left a comment

chattymin Jan 5, 2025

Roel4990 Jan 5, 2025

chattymin Jan 5, 2025

Hyobeen-Park Jan 5, 2025

angryPodo Jan 5, 2025

Roel4990 Jan 6, 2025

Hyobeen-Park left a comment

angryPodo Jan 5, 2025

chattymin Jan 6, 2025

Roel4990 Jan 6, 2025

angryPodo Jan 5, 2025

chattymin Jan 6, 2025

Roel4990 Jan 6, 2025

angryPodo Jan 5, 2025

chattymin Jan 6, 2025

Roel4990 Jan 6, 2025

angryPodo left a comment

Hyobeen-Park left a comment

		val green400 = Color(0xFF00EB92)
		val blue400 = Color(0xFF6A92EF)

		@Stable
		class SpoonyColors(

[Feat/#5] Design System Setting #8

[Feat/#5] Design System Setting #8

Conversation

Roel4990 commented Jan 4, 2025 • edited Loading

Related issue 🛠

Work Description ✏️

Preview

To Reviewers 📢

chattymin left a comment

Choose a reason for hiding this comment

chattymin Jan 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hyobeen-Park left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chattymin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hyobeen-Park left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angryPodo left a comment

Choose a reason for hiding this comment

Hyobeen-Park left a comment

Choose a reason for hiding this comment

Roel4990 commented Jan 4, 2025 •

edited

Loading

chattymin Jan 4, 2025 •

edited

Loading