Conversation
Walkthrough매력포인트에 "필력"을 추가하고, 매력포인트 칩을 한 행에서 두 행으로 분리하는 레이아웃·렌더링 로직 및 선택 처리(두 칩 행 모두)를 리팩토링했습니다. 관련 리소스와 모델(enum)도 업데이트되었습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 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/into/websoso/ui/novelRating/NovelRatingActivity.kt (1)
255-268: 행 분할이 문자열 순서와 3개 고정 가정에 너무 묶여 있습니다.지금은 6개라 맞아 보이지만,
novel_rating_charm_points순서가 바뀌거나 항목이 하나만 더 늘어도 두 번째 row에 전부 몰립니다. 게다가 XML 쪽 두 row가 모두singleLine=true라 이후 칩이 잘릴 가능성이 큽니다. 표시 순서는 타입 안전한 목록으로 두고, 행은chunked(3)처럼 계산하는 쪽이 안전합니다.♻️ 예시
- val firstRow = charmPoints.take(3) - val secondRow = charmPoints.drop(3) + val rows = charmPoints.chunked(3) binding.wcgNovelRatingCharmPointsRow1.removeAllViews() binding.wcgNovelRatingCharmPointsRow2.removeAllViews() - firstRow.forEach { charmPoint -> + rows.getOrNull(0).orEmpty().forEach { charmPoint -> binding.wcgNovelRatingCharmPointsRow1.addChip(createCharmPointChip(charmPoint)) } - secondRow.forEach { charmPoint -> + rows.getOrNull(1).orEmpty().forEach { charmPoint -> binding.wcgNovelRatingCharmPointsRow2.addChip(createCharmPointChip(charmPoint)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/into/websoso/ui/novelRating/NovelRatingActivity.kt` around lines 255 - 268, 현재 NovelRatingActivity에서 charmPoints를 고정 2행(take(3)/drop(3))으로 나누고 있어 항목 수나 순서 변화에 취약합니다; charmPoints = getString(novel_rating_charm_points).toWrappedCharmPoint()로 받은 리스트를 고정 가정 대신 charmPoints.chunked(3)로 행 단위로 분할하고 각 행(chunk)을 순회하면서 createCharmPointChip을 사용해 칩을 추가하세요; 기존 binding.wcgNovelRatingCharmPointsRow1/Row2는 먼저 removeAllViews()로 비운 뒤 chunk 인덱스에 따라 0이면 Row1, 1이면 Row2에 추가하고 더 많은 chunk가 나오면 새 ChipGroup(또는 XML에서 미리 준비한 추가 행)을 인플레이트하거나 동적으로 생성해 추가하도록 변경해 대처하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/com/into/websoso/ui/novelRating/NovelRatingActivity.kt`:
- Around line 255-268: 현재 NovelRatingActivity에서 charmPoints를 고정
2행(take(3)/drop(3))으로 나누고 있어 항목 수나 순서 변화에 취약합니다; charmPoints =
getString(novel_rating_charm_points).toWrappedCharmPoint()로 받은 리스트를 고정 가정 대신
charmPoints.chunked(3)로 행 단위로 분할하고 각 행(chunk)을 순회하면서 createCharmPointChip을 사용해
칩을 추가하세요; 기존 binding.wcgNovelRatingCharmPointsRow1/Row2는 먼저 removeAllViews()로 비운
뒤 chunk 인덱스에 따라 0이면 Row1, 1이면 Row2에 추가하고 더 많은 chunk가 나오면 새 ChipGroup(또는 XML에서 미리
준비한 추가 행)을 인플레이트하거나 동적으로 생성해 추가하도록 변경해 대처하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8196f223-0560-4242-99c7-114ed65fdeef
📒 Files selected for processing (4)
app/src/main/java/com/into/websoso/ui/novelRating/NovelRatingActivity.ktapp/src/main/java/com/into/websoso/ui/novelRating/model/CharmPoint.ktapp/src/main/res/layout/activity_novel_rating.xmlcore/resource/src/main/res/values/strings.xml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/into/websoso/ui/main/myPage/model/AttractivePoints.kt (1)
15-15:fromString()은 표시 문자열까지 함께 받게 해두는 편이 안전합니다.이 함수는 API 응답 문자열을 바로 받아 변환하는 진입점인데, 현재는 enum name 형태만 매칭해서 한글 라벨이 들어오면 호출부의
mapNotNull에서 조용히 누락됩니다. 같은 개념이 다른 모델에서는 값/표시명을 따로 들고 있으니, 여기서도korean까지 허용해 두면 포맷 변화에 더 견고합니다.예시 수정안
- fun fromString(value: String): AttractivePoints? = entries.find { it.name.equals(value, ignoreCase = true) } + fun fromString(value: String): AttractivePoints? = + entries.find { point -> + point.name.equals(value, ignoreCase = true) || point.korean == value + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/into/websoso/ui/main/myPage/model/AttractivePoints.kt` at line 15, fromString currently only matches enum constant names so API responses containing the Korean label get dropped; update AttractivePoints.fromString to also compare the incoming value against the enum's display label (e.g., the korean property) in a case-insensitive/trimmed way so it returns the correct AttractivePoints for either the enum name or its korean label (keep the signature AttractivePoints?.fromString(value: String) and search entries by name.equals(value, ignoreCase = true) || korean.equals(value, ignoreCase = true)).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/com/into/websoso/ui/main/myPage/model/AttractivePoints.kt`:
- Line 15: fromString currently only matches enum constant names so API
responses containing the Korean label get dropped; update
AttractivePoints.fromString to also compare the incoming value against the
enum's display label (e.g., the korean property) in a case-insensitive/trimmed
way so it returns the correct AttractivePoints for either the enum name or its
korean label (keep the signature AttractivePoints?.fromString(value: String) and
search entries by name.equals(value, ignoreCase = true) || korean.equals(value,
ignoreCase = true)).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3b6c62ce-8e33-4c8b-9e6a-136a2bf73b9b
📒 Files selected for processing (1)
app/src/main/java/com/into/websoso/ui/main/myPage/model/AttractivePoints.kt
| chip.isSelected = previousSelectedCharmPoints.contains( | ||
| charmPoints.find { charmPoint -> charmPoint.title == chip.text.toString() }, | ||
| ) | ||
| val selectedTitles = previousSelectedCharmPoints.map { it.title }.toSet() |
Contributor
There was a problem hiding this comment.
previousSelectedCharmPoints가 이미 unique한 값들을 가지며 개수가 적다면 toList가 더 빠른 것으로 알고있습니다!
그리고 데이터가 많다면 mapTo(HashSet()) 구문을 써보는 것도 고려하시면 좋을 것 같아요!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌𝘐𝘴𝘴𝘶𝘦𝘴
📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯
📷𝘚𝘤𝘳𝘦𝘦𝘯𝘴𝘩𝘰𝘵
Screen_recording_20260310_182431.mp4
💬𝘛𝘰 𝘙𝘦𝘷𝘪𝘦𝘸𝘦𝘳𝘴
작품 평가 화면의 매력포인트 부분을 '필력'을 추가 하면서 2줄 구조로 변경했습니다.
기존 방식은 줄바꿈 시 각 줄이 start 정렬되어 피그마와 다르게 보였고, 이번에는 각 row를 별도 그룹으로 분리해 가운데 정렬되도록 수정했습니다.
따라서 매력포인트 칩 영역을 row1, row2 두 개의 WebsosoChipGroup으로 분리하고, 이를 LinearLayout으로 감싸center_horizontal 정렬되도록 변경했습니다.
이를 통해 각 줄이 독립적으로 가운데 정렬된 형태로 보이도록 맞췄습니다.
추가로 필력이 노출/매핑되도록 novel_rating_charm_points와 CharmPoint enum을 함께 업데이트했습니다.
추가로, 기존 선택 상태를 갱신하던 로직은 단일 wcgNovelRatingCharmPoints 기준이었는데, row1/row2 구조로 변경되면서 두 그룹을 모두 순회하도록 수정했습니다.
작품 평가 화면과 마이페이지가 매력포인트를 각각 다른 enum/매핑으로 관리하고 있어, 필력 값이 마이페이지에서 누락되고 있었습니다. 따라서 해당 매핑도 함께 추가했습니다.