-
Notifications
You must be signed in to change notification settings - Fork 0
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
🔀 :: (#132) - 바텀네비게이션바를 구현하였습니다. #133
Conversation
Walkthrough이 변경 사항은 Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (3)
core/design-system/src/main/java/com/school_of_company/design_system/theme/ExpoTheme.kt (1)
8-12
: typography 매개변수에 기본값 추가는 좋은 개선사항입니다.
ExpoTypography
를 기본값으로 설정함으로써:
- 코드 사용성이 향상되었습니다
- 기존 호출 코드와의 호환성이 유지됩니다
- 디자인 시스템의 일관성이 강화되었습니다
ExpoColor
와 동일한 패턴을 따르고 있어 일관된 API를 제공합니다.향후 새로운 테마 속성을 추가할 때도 이와 같은 패턴을 따르는 것을 권장합니다.
core/design-system/src/main/res/values/strings.xml (1)
28-29
: "append_description" 문자열의 설명을 더 직관적으로 수정하는 것을 고려해 주세요."출력 아이콘"이라는 설명이 사용자에게 다소 모호할 수 있습니다. "추가" 또는 "첨부"와 같은 더 직관적인 용어를 사용하는 것이 좋을 것 같습니다.
다음과 같이 수정하는 것을 제안드립니다:
- <string name="append_description">출력 아이콘</string> + <string name="append_description">추가 아이콘</string>core/design-system/src/main/java/com/school_of_company/design_system/component/navigation/ExpoNavigationBar.kt (1)
49-69
: 디자인 시스템 상수 사용을 권장합니다.구분선의 두께가 하드코딩되어 있습니다. 디자인 시스템의 일관성을 위해 상수로 분리하는 것이 좋습니다.
다음과 같이 수정하는 것을 제안합니다:
+private val NavigationDividerThickness = 1.dp + @Composable fun ExpoNavigationBar( modifier: Modifier = Modifier, content: @Composable RowScope.() -> Unit ) { ExpoAndroidTheme { colors, _ -> Column { HorizontalDivider( - thickness = 1.dp, + thickness = NavigationDividerThickness, color = colors.gray200 )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- core/design-system/src/main/java/com/school_of_company/design_system/component/navigation/ExpoNavigationBar.kt (1 hunks)
- core/design-system/src/main/java/com/school_of_company/design_system/icon/ExpoIcon.kt (1 hunks)
- core/design-system/src/main/java/com/school_of_company/design_system/theme/ExpoTheme.kt (1 hunks)
- core/design-system/src/main/res/drawable/ic_append.xml (1 hunks)
- core/design-system/src/main/res/drawable/ic_house.xml (1 hunks)
- core/design-system/src/main/res/values/strings.xml (1 hunks)
🧰 Additional context used
🔇 Additional comments (8)
core/design-system/src/main/java/com/school_of_company/design_system/theme/ExpoTheme.kt (1)
3-6
: 필요한 임포트문이 모두 포함되어 있습니다.모든 필수 컴포넌트들이 올바르게 임포트되어 있으며, 불필요한 임포트는 없습니다.
core/design-system/src/main/res/drawable/ic_house.xml (2)
1-5
: 벡터 드로어블 구조가 잘 정의되어 있습니다!벡터 드로어블의 크기와 뷰포트가 일관성있게 설정되어 있습니다.
1-9
: 아이콘 최적화를 검토해주세요.path 데이터가 최적화되어 있는지 확인해보시면 좋을 것 같습니다. Android Studio의 Vector Asset Studio를 사용하면 자동으로 최적화된 path 데이터를 생성할 수 있습니다.
core/design-system/src/main/res/drawable/ic_append.xml (1)
1-5
: 벡터 드로어블 구조가 잘 구성되어 있습니다!벡터 크기와 뷰포트가 Material Design 아이콘 표준인 24dp로 올바르게 설정되어 있습니다.
core/design-system/src/main/res/values/strings.xml (1)
28-29
: 아이콘 설명의 일관성 검증이 필요합니다.새로 추가된 아이콘 설명이 기존 아이콘들의 명명 규칙과 일치하는지 확인이 필요합니다.
✅ Verification successful
검색 결과를 분석해보니, 모든 아이콘 설명이 일관된 패턴을 따르고 있습니다. 새로 추가된 두 문자열도 기존의 패턴을 잘 따르고 있어 추가 검증이 필요하지 않습니다.
아이콘 설명이 기존 패턴과 일치합니다
기존 아이콘 설명들과 새로 추가된 아이콘 설명들이 모두 다음과 같은 일관된 패턴을 따르고 있습니다:
[기능/의미]
+아이콘
형식 사용- 모든 설명이 한글로 작성됨
- 설명이 간결하고 명확함
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 설명: 아이콘 관련 문자열 리소스의 일관성 확인 # 모든 strings.xml 파일에서 아이콘 관련 문자열 검색 rg -l "description.*아이콘" -g "strings.xml" # 검색된 파일들에서 아이콘 설명 패턴 분석 rg "description.*아이콘" -g "strings.xml"Length of output: 2899
core/design-system/src/main/java/com/school_of_company/design_system/component/navigation/ExpoNavigationBar.kt (1)
81-84
: 리소스 파일 존재 여부를 확인해주세요.참조된 아이콘 리소스들이 실제로 존재하는지 확인이 필요합니다.
다음 스크립트로 리소스 파일들의 존재 여부를 확인할 수 있습니다:
✅ Verification successful
모든 아이콘 리소스가 정상적으로 존재합니다
실행된 스크립트를 통해 다음 리소스 파일들이 모두 존재함을 확인했습니다:
- ic_house.xml
- ic_append.xml
- ic_expo.xml
- ic_user.xml
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if all required drawable resources exist fd -e xml "ic_(house|append|expo|user).xml" core/design-system/src/main/res/drawableLength of output: 301
core/design-system/src/main/java/com/school_of_company/design_system/icon/ExpoIcon.kt (2)
235-242
: 구현이 잘 되어있습니다!
HomeIcon
구현이 파일의 기존 아이콘 패턴을 잘 따르고 있으며, 리소스 참조도 적절합니다.
244-250
: 리소스 확인 및 이름 검토가 필요합니다.구현은 올바르게 되어 있으나, 다음 사항들을 확인해 주시기 바랍니다:
- 'append'라는 이름이 내비게이션 아이템의 목적을 명확하게 전달하는지 검토해 주세요.
- 필요한 리소스가 모두 존재하는지 확인이 필요합니다.
리소스 존재 여부를 확인하기 위해 다음 스크립트를 실행합니다:
✅ Verification successful
Let me generate one more verification to check the actual usage context of this icon:
This will help us understand if the name 'append' is appropriate in the context where it's being used, and what the icon actually looks like.
리소스가 모두 정상적으로 존재하며, 아이콘 이름이 적절합니다.
검증 결과:
ic_append.xml
벡터 드로어블이 존재하며, 체크마크가 있는 캘린더/문서 형태의 아이콘으로 '출력' 기능을 잘 나타내고 있습니다.append_description
문자열 리소스가 "출력 아이콘"으로 올바르게 정의되어 있습니다.- 아이콘 이름과 설명이 기능을 명확하게 전달하고 있어 수정이 필요하지 않습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 필요한 리소스 파일들이 존재하는지 확인 echo "벡터 드로어블 리소스 확인:" fd "ic_append.xml|ic_house.xml" core/design-system/src/main/res/drawable echo "\n문자열 리소스 확인:" rg "append_description|home_description" core/design-system/src/main/res/values/strings.xmlLength of output: 460
Script:
#!/bin/bash # AppendIcon 사용처 확인 echo "AppendIcon 사용 컨텍스트 확인:" rg "AppendIcon\(" -A 3 -B 3 # ic_append 아이콘 내용 확인 echo "\n아이콘 벡터 내용 확인:" cat core/design-system/src/main/res/drawable/ic_append.xmlLength of output: 1679
.../src/main/java/com/school_of_company/design_system/component/navigation/ExpoNavigationBar.kt
Show resolved
Hide resolved
.../src/main/java/com/school_of_company/design_system/component/navigation/ExpoNavigationBar.kt
Show resolved
Hide resolved
.../src/main/java/com/school_of_company/design_system/component/navigation/ExpoNavigationBar.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stable 한 compose인 것 같습니다.
💡 개요
📃 작업내용
🔀 변경사항
🙋♂️ 질문사항
🍴 사용방법
🎸 기타
Summary by CodeRabbit
새로운 기능
버그 수정
ExpoAndroidTheme
함수에서typography
매개변수에 기본값 추가로 사용 간소화.문서화
스타일
ic_house
및ic_append
.