-
Notifications
You must be signed in to change notification settings - Fork 0
fix: 서재소소 최종 QA 사항 반영 #743
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
Conversation
Walkthrough전반적으로 QA 관련 수정으로 값 매핑 교정, 라벨/카피 텍스트 수정, 컴포넌트 레이아웃 제약 조정, 그리고 필터 칩의 바인딩 교차 변경이 포함되었습니다. 공개 API 시그니처 변화나 제어 흐름 변경은 없습니다. Changes
Sequence Diagram(s)(생략) Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 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: 1
🧹 Nitpick comments (2)
feature/library/src/main/java/com/into/websoso/feature/library/component/LibraryFilterEmptyView.kt (1)
42-44: 문구 업데이트 LGTM. 문자열 리소스화(i18n) 권장하드코딩된 문자열 대신 stringResource를 사용하면 다국어/카피 변경에 유리합니다.
적용 예시(라인 범위 내 변경):
- text = "해당하는 작품이 없어요\n" + - "검색의 범위를 더 넓혀보세요", + text = stringResource(id = R.string.library_filter_empty_message),라인 범위 밖 보완:
- import 추가
import androidx.compose.ui.res.stringResource
- strings.xml 예시
<string name="library_filter_empty_message">해당하는 작품이 없어요\n검색의 범위를 더 넓혀보세요</string>feature/library/src/main/java/com/into/websoso/feature/library/component/LibraryGridListItem.kt (1)
14-15: Badge 폭 제약 및 한 줄 처리 보완 제안
- .widthIn(48.dp)는 최솟값 의도이므로 명시적으로 min 파라미터를 쓰면 가독성이 좋아집니다.
- maxLines = 1만 설정하면 기본 overflow가 Clip이라 글자가 잘릴 수 있습니다. Ellipsis를 추가해 UX를 보완하는 것을 권장합니다.
- .widthIn(48.dp) + .widthIn(min = 48.dp)Text( text = readStatusUiModel.readStatus.label, color = White, style = WebsosoTheme.typography.label2, - maxLines = 1, + maxLines = 1, + overflow = TextOverflow.Ellipsis, )Also applies to: 136-150
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/src/main/java/com/into/websoso/ui/novelRating/NovelRatingViewModel.kt(1 hunks)domain/library/src/main/java/com/into/websoso/domain/library/model/ReadStatus.kt(1 hunks)feature/library/src/main/java/com/into/websoso/feature/library/component/LibraryFilterEmptyView.kt(1 hunks)feature/library/src/main/java/com/into/websoso/feature/library/component/LibraryGridListItem.kt(3 hunks)feature/library/src/main/java/com/into/websoso/feature/library/component/LibrayFilterTopBar.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
app/src/main/java/com/into/websoso/ui/novelRating/NovelRatingViewModel.kt (1)
308-312: NovelRatingEntity 필드 매핑 적절 확인 완료
userNovelRating에는 호출된 파라미터(updateUserNovelRating의novelRating)가,
novelRating에는NovelDetailModel.userRating.novelRating(전역 소설 평점)이 Named‐Argument로 올바르게 전달되고 있습니다.- 스왑 이슈 없으며, 파라미터명 변경(
userRating,userNovelRating등)은 가독성 차원에서 선택 검토 가능합니다.feature/library/src/main/java/com/into/websoso/feature/library/component/LibrayFilterTopBar.kt (1)
112-116: 필터칩 라벨/선택 바인딩 교차 변경 확인‘매력포인트’와 ‘평점’ 칩의 라벨/선택 바인딩을 서로 교체한 변경으로 보입니다. 의도된 QA 반영이라면 문제 없습니다. 다만 다음을 확인해 주세요:
- UX 스펙 상 칩 순서/라벨이 기대와 일치하는지
- 분석/로깅/딥링크가 칩 인덱스나 텍스트에 의존하지 않는지 (이전 매핑과의 호환성)
필요 시 UI 테스트 또는 스냅샷 갱신을 권장합니다.
Also applies to: 118-121
| WATCHING("WATCHING", "보는중"), | ||
| WATCHED("WATCHED", "봤어요"), | ||
| QUIT("QUIT", "하차함"), | ||
| QUIT("QUIT", "하차"), | ||
| ; |
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.
💡 Verification agent
❓ Verification inconclusive
레이블 변경(“하차함” → “하차”)은 일관성 있어 보입니다. 잔존 문자열과 레이어 분리 검토 요청
- 변경 자체는 문제 없어 보입니다. 다만 기존 문자열 “하차함”이 다른 화면/테스트/스냅샷에 하드코딩되어 있지 않은지 확인 필요합니다.
- 도메인 레이어에서 사용자 노출용 레이블을 보관하면 i18n/디자인 시스템과의 결합이 커집니다. key만 도메인에 두고, UI에서 stringResource로 매핑하는 구조로 분리하는 것을 권장합니다.
다음 스크립트로 레포 내 잔존 문자열을 점검하세요:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# "하차함" 사용 흔적 검색
rg -n --hidden -S "하차함" -A 2 -B 2Length of output: 89
“하차” 레이블 적용 및 문자열 잔존 여부 확인 완료
- “하차함” 문자열은 전체 레포지토리에서 더 이상 발견되지 않습니다.
- 도메인 레이어에 UI 표시용 문자열을 직접 보관하면 i18n/디자인 시스템과 결합도가 높아집니다. 도메인에는 key만 두고, presentation 계층에서 stringResource 등으로 매핑하는 구조로 분리하는 것을 권장합니다.
🤖 Prompt for AI Agents
domain/library/src/main/java/com/into/websoso/domain/library/model/ReadStatus.kt
lines 7-10: 현재 enum에 UI용 한글 라벨을 직접 보관하고 있으니 도메인에서 UI 문자열을 제거하고 키만 남기도록 변경하세요 — 즉
enum은 WATCHING, WATCHED, QUIT 같은 식별자(및 필요 시 영문 key)만 유지하고 "보는중", "봤어요", "하차" 등의
한글 레이블은 모두 삭제합니다; 이후 presentation 계층에서 stringResource/번역 맵핑으로 각 key에 대응되는 로컬라이즈된
문자열을 제공하도록 호출부를 변경(또는 어댑터/mapper 추가)해 도메인과 i18n/디자인 시스템 의존도를 분리하세요.
yeonjeen
left a comment
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.
땡큐쏘머치입니다:)
📌𝘐𝘴𝘴𝘶𝘦𝘴
📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯
Summary by CodeRabbit