Conversation
Nico1eKim
left a comment
There was a problem hiding this comment.
수고하셨습니닷 ~~ 전반적으로 string 추출이 안된게 많은거같은데 다 추출해서 리팩부탁드립니둥 ~~
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardAlarm.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardAlarm.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardAlarm.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardBookList.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardInputBook.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardRoomBook.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/theme/common/form/BookPageTextField.kt
Show resolved
Hide resolved
|
Caution Review failedThe pull request is closed. Walkthrough여러 Jetpack Compose 기반의 카드 및 폼 UI 컴포넌트가 새로 추가되었으며, 관련 벡터 드로어블 리소스와 색상, 문자열 리소스도 함께 추가되었습니다. 일부 IDE 설정 파일도 변경 또는 추가되었습니다. 기존 코드의 변경 없이 신규 UI 요소와 리소스가 중심입니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ComposeUI
participant Callback
User->>ComposeUI: 카드/폼 컴포넌트 클릭 또는 입력
ComposeUI->>Callback: onClick / onChange 등 콜백 호출
Callback-->>ComposeUI: 상태 변경 (예: 읽음 처리, 북마크 토글)
ComposeUI-->>User: UI 갱신 및 피드백 표시
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (12)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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.
Actionable comments posted: 6
♻️ Duplicate comments (6)
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardInputBook.kt (1)
86-98: 공통 컴포넌트 사용 권장사항이전 리뷰 댓글과 동일한 의견입니다. 공통 컴포넌트가 머지되면 해당 컴포넌트를 사용해 주세요.
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardAlarm.kt (4)
37-44: 파라미터 순서 조정 필요이전 리뷰 댓글과 동일한 이슈입니다. modifier를 첫 번째 파라미터로 배치해 주세요.
50-50: 하드코딩된 색상 값 개선 필요이전 리뷰 댓글과 동일한 이슈입니다. 하드코딩된 색상 값들을 전역 변수로 선언해서 사용해 주세요.
68-69: 디자인 시스템 정렬 권장사항이전 리뷰 댓글과 동일한 의견입니다. 홀수 단위보다는 4배수나 짝수 단위로 맞춰주시면 좋겠습니다.
78-78: 문자열 추출 필요이전 리뷰 댓글과 동일한 이슈입니다. 하드코딩된 문자열을 추출해 주세요.
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardRoomBook.kt (1)
75-79: 피그마 아이콘 사용이 필요합니다.이전 리뷰 코멘트에서 언급된 대로 기본 아이콘 대신 피그마에서 다운로드한 아이콘을 사용해주세요.
🧹 Nitpick comments (9)
.idea/git_toolbox_prj.xml (1)
3-14: GitToolBox 플러그인 설정 버전 관리 여부 검토 필요
GitToolBoxProjectSettings 컴포넌트가 커밋 메시지 유효성 검사 오버라이드를 활성화하고 있습니다. 모든 팀원이 해당 플러그인을 사용하나요? 필요 없는 경우 .gitignore에 추가하거나, README에 플러그인 사용 가이드를 명시해 주세요.app/src/main/java/com/texthip/thip/ui/theme/common/card/CardBookSearch.kt (1)
29-35: 파라미터 순서를 Compose 관례에 맞게 조정해 주세요.Compose 관례에 따라 modifier를 첫 번째 파라미터로 배치하는 것이 좋습니다.
@Composable fun CardBookSearch( + modifier: Modifier = Modifier, number: Int, title: String, imageRes: Int? = R.drawable.bookcover_sample, // 기본 이미지 리소스 onClick: () -> Unit = {}, - modifier: Modifier = Modifier ) {app/src/main/java/com/texthip/thip/ui/theme/common/form/FormTextFieldDefault.kt (1)
28-31: 파라미터 순서를 Compose 관례에 맞게 조정해 주세요.modifier를 첫 번째 파라미터로 배치하는 것이 좋습니다.
@Composable fun BaseInputTextField( + modifier: Modifier = Modifier, hint: String, - modifier: Modifier = Modifier ) {app/src/main/java/com/texthip/thip/ui/theme/common/card/CardInputBook.kt (1)
33-39: 파라미터 순서를 Compose 관례에 맞게 조정해 주세요.modifier를 첫 번째 파라미터로 배치하는 것이 좋습니다.
@Composable fun CardInputBook( + modifier: Modifier = Modifier, title: String, author: String, imageRes: Int? = R.drawable.bookcover_sample, // 기본 이미지 리소스 onChangeClick: () -> Unit = {}, - modifier: Modifier = Modifier ) {app/src/main/java/com/texthip/thip/ui/theme/common/form/BorderedTextField.kt (2)
28-31: 컴포넌트 재사용성 개선을 제안합니다.현재 컴포넌트는 내부적으로만 텍스트 상태를 관리하고 있어 외부에서 값을 제어할 수 없습니다. 더 나은 재사용성을 위해 상태를 외부에서 제어할 수 있도록 개선하는 것을 고려해보세요.
@Composable fun BorderedTextField( hint: String, + value: String = "", + onValueChange: (String) -> Unit = {}, modifier: Modifier = Modifier ) { - var text by rememberSaveable { mutableStateOf("") } + var text by rememberSaveable { mutableStateOf(value) } val myStyle = typography.menu_r400_s14_h24.copy(lineHeight = 14.sp)
46-46: 고정 크기 대신 유연한 크기 조정을 고려하세요.현재
320.dp고정 너비는 다양한 화면 크기에서 문제가 될 수 있습니다.fillMaxWidth()또는maxWidth를 사용하여 더 반응형으로 만드는 것을 고려해보세요.app/src/main/java/com/texthip/thip/ui/theme/common/card/CardRoomBook.kt (1)
42-42: 이미지 리소스 파라미터 개선을 제안합니다.현재
imageRes가 nullable이지만 기본값이 있어서 실제로는 null이 될 가능성이 낮습니다. 더 명확하게 non-null로 하거나 실제 null 처리가 필요한지 검토해보세요.- imageRes: Int? = R.drawable.bookcover_sample, + imageRes: Int = R.drawable.bookcover_sample,그리고 아래 코드를 단순화:
- imageRes?.let { - Image( - painter = painterResource(id = it), + Image( + painter = painterResource(id = imageRes), contentDescription = null, modifier = Modifier.fillMaxSize(), contentScale = ContentScale.Crop - ) - } + )app/src/main/java/com/texthip/thip/ui/theme/common/card/CardBookList.kt (1)
47-47: 이미지 리소스 처리 개선을 제안합니다.다른 카드 컴포넌트와 마찬가지로 nullable 타입보다는 기본값을 가진 non-null 타입을 사용하는 것이 더 일관성 있고 안전합니다.
- imageRes: Int? = R.drawable.bookcover_sample, // 기본 이미지 리소스 + imageRes: Int = R.drawable.bookcover_sample, // 기본 이미지 리소스app/src/main/java/com/texthip/thip/ui/theme/common/form/WarningTextField.kt (1)
52-52: 고정 크기 대신 유연한 크기 조정을 고려하세요.다른 텍스트 필드 컴포넌트들과 마찬가지로 고정 너비(
320.dp) 대신 더 유연한 크기 조정을 고려해보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/src/main/res/drawable/bookcover_sample.pngis excluded by!**/*.png
📒 Files selected for processing (14)
.idea/deploymentTargetSelector.xml(1 hunks).idea/git_toolbox_prj.xml(1 hunks).idea/material_theme_project_new.xml(1 hunks).idea/misc.xml(0 hunks)app/src/main/java/com/texthip/thip/ui/theme/common/card/CardAlarm.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/theme/common/card/CardBookList.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/theme/common/card/CardBookSearch.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/theme/common/card/CardInputBook.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/theme/common/card/CardRoomBook.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/theme/common/form/BookPageTextField.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/theme/common/form/BorderedTextField.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/theme/common/form/FormTextFieldDefault.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/theme/common/form/WarningTextField.kt(1 hunks)app/src/main/res/drawable/ic_x_circle_white.xml(1 hunks)
💤 Files with no reviewable changes (1)
- .idea/misc.xml
🔇 Additional comments (7)
.idea/deploymentTargetSelector.xml (1)
5-5: runConfigName 변경사항 동기화 확인 요청
SelectionState의 runConfigName이"app"에서"Thip.app"으로 변경되었습니다. 모든 개발 환경 및 CI/CD 설정과 일치하는지 확인하고, 문제가 없다면 문서화 해주세요.app/src/main/res/drawable/ic_x_circle_white.xml (1)
1-16: 벡터 드로어블 구현이 올바르게 되어 있습니다.XML 구조와 경로 데이터가 정확하며, clear 아이콘으로 사용하기에 적절합니다. 다만 25x24 dp로 너비와 높이가 다른 점이 의도된 것인지 확인해 보시기 바랍니다.
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardBookSearch.kt (1)
36-76: 구현이 깔끔하고 기능적으로 올바릅니다.레이아웃 구조와 스타일링이 잘 되어 있으며, 클릭 인터랙션도 적절하게 구현되었습니다.
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardRoomBook.kt (1)
36-45: 컴포넌트 구조가 잘 설계되었습니다.매개변수 구조와 기본값 설정이 잘 되어 있고, 클릭 핸들러도 적절히 구현되어 있습니다.
app/src/main/java/com/texthip/thip/ui/theme/common/form/WarningTextField.kt (1)
30-89: 경고 상태 관리가 잘 구현되었습니다.경고 상태에 따른 조건부 스타일링과 메시지 표시가 적절히 구현되어 있습니다. 다른 텍스트 필드 컴포넌트들과 일관성도 유지되고 있습니다.
app/src/main/java/com/texthip/thip/ui/theme/common/form/BookPageTextField.kt (2)
116-141: 커스텀 VisualTransformation 구현이 우수합니다.
SuffixTransformation클래스가 잘 구현되어 있습니다. 접미사 스타일링과 커서 매핑 로직이 정확하고, 사용자 경험을 고려한 좋은 구현입니다.
41-44: 컴포넌트 설계가 특화된 용도에 적합합니다.페이지 번호 입력이라는 특정 용도에 맞게 잘 설계된 컴포넌트입니다. 매개변수가 간단하면서도 필요한 기능을 모두 포함하고 있습니다.
| <component name="MaterialThemeProjectNewConfig"> | ||
| <option name="metadata"> | ||
| <MTProjectMetadataState> | ||
| <option name="migrated" value="true" /> | ||
| <option name="pristineConfig" value="false" /> | ||
| <option name="userId" value="2561a451:1955783b6f8:-7ffe" /> | ||
| </MTProjectMetadataState> | ||
| </option> | ||
| </component> |
There was a problem hiding this comment.
개인별 userId 포함된 테마 설정 파일은 버전 관리에서 제외 필요
MaterialThemeProjectNewConfig에 userId 옵션이 포함되어 있는데, 이는 개인별 IDE 설정으로 팀 공통 저장소에 커밋하기에 부적합합니다. .gitignore에 추가하거나 userId를 제거해 주세요.
🤖 Prompt for AI Agents
In .idea/material_theme_project_new.xml around lines 3 to 11, the userId option
contains personal user information that should not be committed to the shared
repository. To fix this, either remove the userId option from this file or add
this file to .gitignore to exclude it from version control, ensuring personal
IDE settings are not shared.
| } else { | ||
| Icon( | ||
| painter = painterResource(id = R.drawable.ic_x_circle), | ||
| contentDescription = "Clear text" | ||
| ) | ||
| } |
There was a problem hiding this comment.
존재하지 않는 drawable 리소스를 참조하고 있습니다.
R.drawable.ic_x_circle이 존재하지 않을 가능성이 높습니다. 이 PR에서는 ic_x_circle_white만 추가되었습니다.
} else {
Icon(
- painter = painterResource(id = R.drawable.ic_x_circle),
+ painter = painterResource(id = R.drawable.ic_x_circle_white),
contentDescription = "Clear text"
+ tint = colors.Grey02
)
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/theme/common/form/FormTextFieldDefault.kt
between lines 64 and 69, the code references a drawable resource
R.drawable.ic_x_circle which does not exist. Replace this reference with the
existing drawable resource R.drawable.ic_x_circle_white to fix the missing
resource issue.
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardAlarm.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/theme/common/form/BorderedTextField.kt
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/theme/common/form/BookPageTextField.kt
Show resolved
Hide resolved
rbqks529
left a comment
There was a problem hiding this comment.
pr 확인했고 수정 완료 했습니다
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardAlarm.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardAlarm.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardAlarm.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardAlarm.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardRoomBook.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/theme/common/card/CardInputBook.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/theme/common/form/BookPageTextField.kt
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/theme/common/form/BorderedTextField.kt
Show resolved
Hide resolved
Branch 최신화
| @Composable | ||
| fun BaseInputTextField( | ||
| modifier: Modifier = Modifier, | ||
| hint: String |
There was a problem hiding this comment.
boolean 값으로 show limit, show icon 구분 가능하게끔 부탁드려요잇~
| contentAlignment = Alignment.Center | ||
| ) { | ||
| Text( | ||
| text = stringResource(R.string.group), |
There was a problem hiding this comment.
요기 group 말고도 댓글, 좋아요 등으로 활용되는 것 같아용 (내정보 페이지 참고해주세욥)
파라미터로 받을 수 있도록 부탁드립니당
|
|
||
| Text( | ||
| text = timeAgo + stringResource(R.string.time_ago), | ||
| style = typography.view_m500_s12_h20, |
There was a problem hiding this comment.
'~시간 전' 으로만 시간 표기 가능하게 돼있는 것 같은데, 날짜로도 표시 가능하게끔 해주세욥 ( 요것도 내정보 페이지 참고 바랍니닷.. ㅎㅎ)
➕ 이슈 링크
🔎 작업 내용
📸 스크린샷
TextFieldDefault


BookPageTextField


BorderedTextField

CardBookList

CardBookSearch

CardInputBook

CardRoomBook

CardAlarm

😢 해결하지 못한 과제
[] TASK
📢 리뷰어들에게
Summary by CodeRabbit