Conversation
- 선택 여부(state_checked)에 따라 primary_100 및 gray_80 색상을 적용하는 `bg_novel_detail_rating_read_status_icon_selector.xml` 추가
- `novel_rating_star_title` (별점) 스트링 리소스 추가 - `novel_rating_keyword_button` 문구를 "이 작품에 대해 소개해주세요!"에서 "작품을 나타내는 키워드는?"으로 변경
- 작품 평가 화면(`NovelRatingActivity`)의 매력 포인트 선택 방식을 칩(Chip) 형태에서 아이콘과 텍스트가 결합된 커스텀 레이아웃으로 변경
- 매력 포인트 아이콘 및 텍스트의 선택 상태에 따른 색상 변경 로직 추가 (ColorFilter 및 TextColor 제어)
- `NovelRatingActivity` 내 매력 포인트 뷰 바인딩 및 관리를 위한 `CharmPointItem`, `CharmPointViews` 내부 데이터 클래스 정의
- `activity_novel_rating.xml` 레이아웃 수정
- 매력 포인트 섹션을 `WebsosoChipGroup`에서 `LinearLayout` 기반의 개별 아이콘 버튼 구조로 전면 개편
- 별점 및 키워드 섹션의 타이틀 스타일을 `body1`에서 `title3`로 변경
- 별점 영역 상단에 타이틀(`tv_novel_rating_star_title`) 추가 및 간격 조정
- 읽기 상태 버튼의 아이콘 틴트 색상 선택자(`bg_novel_detail_rating_read_status_icon_selector`) 적용 중복 수정
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough매력 포인트 선택 UI를 칩 기반 동적 구성에서 6개 고정 뷰(컨테이너/아이콘/타이틀) 목록으로 전환하고, Activity 초기화 호출·선택 동기화 로직·레이아웃·색상·문자열 리소스를 함께 갱신했습니다. Changes작품 평가 화면 UI 구조 개선
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/src/main/res/layout/activity_novel_rating.xml (1)
181-341: 💤 Low value매력 포인트 뷰의 초기 색상이 코드에서 즉시 재설정됩니다.
레이아웃 XML에서 모든 아이콘에
app:tint="@color/gray_80_DDDDE3", 모든 텍스트에android:textColor="@color/gray_200_949399"를 하드코딩했지만,NovelRatingActivity.kt의setupCharmPointItems()메서드(308번 줄)에서updateCharmPointItems(emptyList())를 호출하여 동일한 색상으로 즉시 재설정됩니다.기능적으로는 문제없지만, XML의 하드코딩된 색상 값이 실제로 사용자에게 표시되지 않으므로 유지보수 시 혼란을 줄 수 있습니다.
♻️ 대안적 접근
XML에서 색상을 명시하지 않거나, 코드 주석으로 "초기 색상은 코드에서 설정됨"을 명시하는 것을 고려하세요.
🤖 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/res/layout/activity_novel_rating.xml` around lines 181 - 341, The XML hardcodes icon tints and textColors for charm-point views but NovelRatingActivity.setupCharmPointItems() calls updateCharmPointItems(emptyList()) which immediately overrides those colors; remove the hardcoded attributes (app:tint and android:textColor) from the ImageView and TextView elements (e.g., iv_novel_rating_charm_point_*, tv_novel_rating_charm_point_*), or add a concise XML comment near these views stating that colors are initialized in code by setupCharmPointItems()/updateCharmPointItems(), so future maintainers won't be confused.app/src/main/java/com/into/websoso/ui/novelRating/NovelRatingActivity.kt (1)
222-229: ⚡ Quick win아이콘과 텍스트의
isSelected설정이 중복됩니다.225-226번 줄에서
icon.isSelected와title.isSelected를 설정하지만, 227-228번 줄에서setColorFilter()와setTextColor()를 직접 호출하여 색상을 수동으로 재설정합니다.
isSelected속성은 뷰에 state-dependent drawable이나 ColorStateList가 설정되어 있을 때만 효과가 있는데, 현재 레이아웃 XML에서는 이러한 state selector가 아이콘이나 텍스트에 정의되어 있지 않습니다. 따라서 225-226번 줄의isSelected설정은 실제로 아무 효과가 없으며, 수동 색상 설정으로 인해 무시됩니다.♻️ 제안: 불필요한 코드 제거
옵션 1: isSelected 호출 제거 (현재 구조 유지)
charmPointItems.forEach { item -> val isSelected = item.charmPoint in selectedCharmPoints item.container.isSelected = isSelected - item.icon.isSelected = isSelected - item.title.isSelected = isSelected item.icon.setColorFilter(if (isSelected) selectedColor else defaultIconColor) item.title.setTextColor(if (isSelected) selectedColor else defaultTextColor) }옵션 2: ColorStateList 사용 (더 선언적인 접근)
색상 리소스에 state selector를 추가하고 수동 색상 설정을 제거:
charmPointItems.forEach { item -> val isSelected = item.charmPoint in selectedCharmPoints item.container.isSelected = isSelected item.icon.isSelected = isSelected item.title.isSelected = isSelected - item.icon.setColorFilter(if (isSelected) selectedColor else defaultIconColor) - item.title.setTextColor(if (isSelected) selectedColor else defaultTextColor) }그리고 레이아웃 XML에서 아이콘과 텍스트에 state selector 리소스를 적용합니다.
🤖 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/into/websoso/ui/novelRating/NovelRatingActivity.kt` around lines 222 - 229, The isSelected assignments on item.icon and item.title are redundant because no state-dependent drawables/ColorStateList are defined; remove the two lines that set item.icon.isSelected and item.title.isSelected and keep the explicit color setting logic (item.icon.setColorFilter(...) and item.title.setTextColor(...)) so selection visuals continue to be driven by selectedCharmPoints, selectedColor, defaultIconColor and defaultTextColor; alternatively, if you prefer a declarative approach, replace the manual color calls with a ColorStateList resource and apply it in XML to icon/title and then remove the manual setColorFilter/setTextColor calls instead.
🤖 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.
Inline comments:
In `@app/src/main/java/com/into/websoso/ui/novelRating/NovelRatingActivity.kt`:
- Around line 259-301: setupCharmPointItems() relies on the order of
toWrappedCharmPoint() matching the hardcoded charmPointViews list and zips them
positionally, which breaks if resource/enum order changes; change to an explicit
map-based mapping keyed by the charm-point enum (or unique id) instead of using
zip: build a Map<CharmPointType, CharmPointViews> from the CharmPointViews list
(reference CharmPointViews, charmPointViews), then iterate the list returned by
toWrappedCharmPoint() (reference toWrappedCharmPoint) and for each charmPoint
lookup its views in that map and create the CharmPointItem, assigning title.text
and other fields; update charmPointItems construction (reference
charmPointItems) to use these lookups so mapping remains correct regardless of
ordering.
---
Nitpick comments:
In `@app/src/main/java/com/into/websoso/ui/novelRating/NovelRatingActivity.kt`:
- Around line 222-229: The isSelected assignments on item.icon and item.title
are redundant because no state-dependent drawables/ColorStateList are defined;
remove the two lines that set item.icon.isSelected and item.title.isSelected and
keep the explicit color setting logic (item.icon.setColorFilter(...) and
item.title.setTextColor(...)) so selection visuals continue to be driven by
selectedCharmPoints, selectedColor, defaultIconColor and defaultTextColor;
alternatively, if you prefer a declarative approach, replace the manual color
calls with a ColorStateList resource and apply it in XML to icon/title and then
remove the manual setColorFilter/setTextColor calls instead.
In `@app/src/main/res/layout/activity_novel_rating.xml`:
- Around line 181-341: The XML hardcodes icon tints and textColors for
charm-point views but NovelRatingActivity.setupCharmPointItems() calls
updateCharmPointItems(emptyList()) which immediately overrides those colors;
remove the hardcoded attributes (app:tint and android:textColor) from the
ImageView and TextView elements (e.g., iv_novel_rating_charm_point_*,
tv_novel_rating_charm_point_*), or add a concise XML comment near these views
stating that colors are initialized in code by
setupCharmPointItems()/updateCharmPointItems(), so future maintainers won't be
confused.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d08b06ce-84d4-4fc8-8ab6-d13a468e4759
📒 Files selected for processing (4)
app/src/main/java/com/into/websoso/ui/novelRating/NovelRatingActivity.ktapp/src/main/res/color/bg_novel_detail_rating_read_status_icon_selector.xmlapp/src/main/res/layout/activity_novel_rating.xmlcore/resource/src/main/res/values/strings.xml
- `CharmPointViews`를 리스트에서 맵(Map) 구조로 변경하여 데이터 순서 의존성 제거 - `orderedCharmPoints`를 기반으로 뷰를 직접 매핑하도록 수정하여 안정성 향상
- `NovelRatingActivity`에서 매력 포인트 아이템 리스트를 빈 리스트로 명시적으로 초기화하던 불필요한 코드 제거
- `NovelRatingActivity`에서 매력 포인트 아이템의 `isSelected` 상태 값을 직접 변경하던 로직 삭제 - 선택 여부에 따른 아이콘 색상(`setColorFilter`) 및 텍스트 색상(`setTextColor`) 변경 로직만 유지하도록 수정
|
|
||
| private val novelRatingViewModel: NovelRatingViewModel by viewModels() | ||
| private val charmPoints: List<CharmPoint> = CharmPoint.entries.toList() | ||
| private lateinit var charmPointItems: List<CharmPointItem> |
There was a problem hiding this comment.
기존 매력포인트가 칩으로 된 UI에서는 CharmPoint 값만 순회해서 칩을 생성하면 됐는데 아이콘 UI로 변경되면서 각 CharmPoint를 XML에 정의된 container/icon/title 뷰와 연결해야 해서 charmPointItems를 추가했습니다!
다만 기존 charmPoints 필드는 클릭 처리에서만 쓰이고 있었고 전달받은 charmPoint를 다시 find할 필요는 없어 보여서 바로 사용할 수 있도록 정리했습니다!
| private data class CharmPointItem( | ||
| val charmPoint: CharmPoint, | ||
| val container: View, | ||
| val icon: ImageView, | ||
| val title: TextView, | ||
| ) | ||
|
|
||
| private data class CharmPointViews( | ||
| val container: View, | ||
| val icon: ImageView, | ||
| val title: TextView, | ||
| ) |
There was a problem hiding this comment.
리뷰를 보고 코드를 다시 보니 CharmPointViews와 CharmPointItem은 charmPoint 포함 여부만 다르고 역할이 겹쳐 보여서 분리할 필요가 없을 것 같더라고요..
CharmPointViews는 제거하고 각 CharmPoint가 대응되는 container/icon/title 뷰를 CharmPointItem으로 변환하도록 toCharmPointItem()으로 정리했습니다!
- `NovelRatingActivity` 내 불필요한 `charmPoints` 변수 제거 - `handleCharmPointClick`에서 매력 포인트 데이터를 ViewModel에 직접 전달하도록 수정하여 불필요한 리스트 탐색 로직 제거
- `CharmPointViews` 데이터 클래스를 제거하고 `CharmPointItem`으로 통합 - 매력 포인트 아이템 생성 방식을 `map` 기반 조회에서 `when` 분기 처리로 변경하여 가독성 개선 - 아이템 생성 및 클릭 리스너 설정 로직 분리 및 구조화
📌𝘐𝘴𝘴𝘶𝘦𝘴
📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯
기존 : 아이콘과 글자 색 통일
별점타이틀 추가칩에서아이콘으로 변경작품을 나타내는 키워드는?으로 변경📷𝘚𝘤𝘳𝘦𝘦𝘯𝘴𝘩𝘰𝘵
💬𝘛𝘰 𝘙𝘦𝘷𝘪𝘦𝘸𝘦𝘳𝘴
Summary by CodeRabbit