[Feat] 1차 QA 수정사항 반영 - #217
Hidden character warning
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough공통 Changes텍스트 및 UI 개선
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/flint/presentation/onboarding/component/OnboardingContentItem.kt (1)
120-125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win중복된 줄바꿈 헬퍼는 공용 유틸로 합쳐주세요.
같은
addKoreanLineBreaks()구현이CollectionFileItem.kt에도 별도로 들어가 있어서, 이후 문자 범위나 zero-width space 처리 방식을 바꿀 때 두 파일이 쉽게 어긋납니다.presentation공용 확장 함수로 올리고 두 컴포넌트가 같이 쓰도록 정리하는 편이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/flint/presentation/onboarding/component/OnboardingContentItem.kt` around lines 120 - 125, The addKoreanLineBreaks() logic is duplicated in OnboardingContentItem and CollectionFileItem, so move this String extension into a shared presentation-level utility/extension file and have both components use the same implementation. Keep the symbol name addKoreanLineBreaks() recognizable in the new shared location, then remove the local copies from each component so the Hangul range and zero-width space behavior stay consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@app/src/main/java/com/flint/presentation/onboarding/component/OnboardingContentItem.kt`:
- Around line 120-125: The addKoreanLineBreaks() logic is duplicated in
OnboardingContentItem and CollectionFileItem, so move this String extension into
a shared presentation-level utility/extension file and have both components use
the same implementation. Keep the symbol name addKoreanLineBreaks() recognizable
in the new shared location, then remove the local copies from each component so
the Hangul range and zero-width space behavior stay consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 21cb3dfc-1225-470f-8a73-8597821f6d83
📒 Files selected for processing (4)
app/src/main/java/com/flint/presentation/collectionlist/component/CollectionFileItem.ktapp/src/main/java/com/flint/presentation/onboarding/OnboardingContentScreen.ktapp/src/main/java/com/flint/presentation/onboarding/component/OnboardingContentItem.ktapp/src/main/java/com/flint/presentation/profile/component/ProfileKeywordSection.kt
kimjw2003
left a comment
kimjw2003
left a comment
There was a problem hiding this comment.
QA 수정사항 잘 확인했습니다 👍
로딩 인디케이터는 searchContents()가 IME onSearch에만 물려 있어서 타이핑 중 깜빡임 없이 초기 진입 / 장르 칩 / 검색 실행 시점에만 뜨는 것 확인했습니다. 기존 Empty/Failure 브랜치와 GridItemSpan(3) + 300.dp도 맞춰져 있어서 좋네요.
한글 줄바꿈 관련해서 몇 가지만 남깁니다.
1. addKoreanLineBreaks() 중복 정의 → 공용 확장으로 빼주세요
CollectionFileItem.kt:211 과 OnboardingContentItem.kt:120 에 완전히 동일한 함수가 각각 private으로 복사돼 있습니다. (설명 주석은 OnboardingContentItem 쪽에만 있고요.)
이미 com.flint.core.common.extension 패키지가 있으니 StringExt.kt 하나 만들어서 옮기는 게 좋을 것 같습니다. 특히 지금 고친 두 곳 말고도 한글 + maxLines + TextOverflow.Ellipsis 조합이 아래에도 똑같이 있어서, 공용화 안 하면 QA 돌 때마다 복붙이 늘어날 것 같아요.
savedcontent/SavedContentListItem.kthome/component/RecommendCollectionCard.ktcollectiondetail/component/CollectionDetailThumbnail.ktcollectioncreate/component/CollectionCreateContentSection.ktprofile/component/CollectionCreateContentBookmark.kt
이번 PR에서 전부 적용할 필요는 없지만, 최소한 함수 위치는 공용으로 빼두면 좋겠습니다.
2. ZWSP를 raw 비가시 문자로 넣지 말고 '\u200B' 이스케이프로
if (char.code in 0xAC00..0xD7A3) append('')od로 확인해보니 실제 U+200B 문자가 소스에 그대로 박혀 있습니다 (0xE2 0x80 0x8B). 동작은 하는데,
- GitHub diff나 에디터에서 뭐가 들어갔는지 눈으로 확인이 안 됩니다 (지금 이 리뷰 쓰면서도
od떠서 확인했어요) - 복붙 / 에디터 인코딩 설정에 따라 U+FEFF 같은 다른 비가시 문자로 바뀌어도 아무도 눈치 못 챕니다
private const val ZERO_WIDTH_SPACE = '\u200B'이렇게 상수로 빼주시면 좋겠습니다. 매직 넘버 0xAC00..0xD7A3도 같이 이름 붙여주면 (HANGUL_SYLLABLE_RANGE) 더 읽기 좋을 것 같아요.
3. recomposition마다 문자열 재생성 → remember
OnboardingContentItem.kt:88, CollectionFileItem.kt:69,77 모두 composable body에서 매번 호출돼서, LazyVerticalGrid 스크롤 중 보이는 아이템마다 원본 길이의 최대 2배짜리 String을 새로 할당합니다.
val displayTitle = remember(title) { title.addKoreanLineBreaks() }체감될 정도는 아닐 수 있지만 순수 함수라 캐싱 비용이 거의 없어서 넣어두면 좋을 것 같습니다.
4. import 순서 (ktlint)
OnboardingContentScreen.kt:52 — ...component.indicator.FlintLoadingIndicator가 ...component.topappbar.FlintBackTopAppbar 뒤에 들어가 있어서 ktlint import-ordering 위반입니다. CI에서 ktlintCheck가 주석 처리돼 있어 안 걸리긴 하는데, ./gradlew ktlintFormat 한 번 돌려주시면 좋겠습니다.
확인만 부탁드려요
20개텍스트: 클라에는 임계값 상수 없이 서버가 내려주는isRecalculatable플래그로 버튼이 게이팅되고 있어서 코드상 충돌은 없습니다. 서버 기준도 20으로 같이 바뀌었는지만 확인 부탁드립니다 🙏- 줄바꿈 단위: ZWSP를 모든 한글 음절 뒤에 넣으면 어절이 아니라 글자 단위로 끊깁니다 (
저장한 작/품들에서). 한글 조판상 허용되는 방식이긴 한데, 디자이너분이 의도한 게 맞는지 한 번 봐주시면 좋을 것 같아요. - 문자열이 한글로 끝나면 맨 뒤에도 ZWSP가 하나 붙는데, 렌더링엔 영향 없지만 불필요하긴 합니다.
미구현으로 남기신 온보딩 done 회원 정보 저장은 별도 이슈로 트래킹하는 게 좋을 것 같습니다!
|
코드리뷰 반영했습니다! 👍 |
2 participants
📮 관련 이슈
📌 작업 내용
😅 미구현ㅂ- [ ] 온보딩 done 회원 정보 저장
🫛 To. 리뷰어
Summary by CodeRabbit
새 기능
버그 수정