[Fix] 버그 제보 이미지 첨부 안되는 버그 수정 (#20)#21
Open
leejh08 wants to merge 4 commits into
Open
Conversation
- BugReportViewModel: 이미지 업로드 응답 타입 [String] → ImageUploadResponse로 수정 - AndroidManifest: READ_MEDIA_IMAGES, READ_EXTERNAL_STORAGE 권한 추가 close #20
|
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 due to trivial changes (1)
워크스루이미지 업로드의 응답 디코딩을 변경 사항버그 제보 이미지 업로드 수정
학생 출석 상태 버튼 스타일링 개선
추정 코드 리뷰 난이도🎯 3 (Moderate) | ⏱️ ~20 minutes 시
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Sources/PiCKAdmin/Features/SelfStudyCheck/Components/SelfStudyCheckComponents.swift (1)
43-86: ⚡ Quick win코드 중복: 상태 색상 관리 로직을 통합하는 것을 고려하세요.
statusColor,statusBackgroundColor,statusBorderColor세 함수가 동일한 구조의 switch 문을 사용하고 있습니다. 새로운 상태가 추가되거나 색상이 변경될 때 세 곳을 모두 수정해야 하므로 유지보수가 어렵고 불일치 위험이 있습니다.상태별 색상 정보를 하나의 구조체나 튜플로 반환하는 단일 함수로 통합하면 코드 중복을 제거하고 유지보수성을 개선할 수 있습니다.
♻️ 리팩토링 제안: 색상 정보 통합
+ private struct StatusColors { + let text: Color + let background: Color + let border: Color + } + + private func statusColors(_ status: String) -> StatusColors { + switch status { + case "출석": + return StatusColors( + text: .Normal.white, + background: .Primary.primary500, + border: .Primary.primary500 + ) + case "이동": + return StatusColors( + text: .Normal.white, + background: .Gray.gray700, + border: .Gray.gray700 + ) + case "귀가", "외출": + return StatusColors( + text: .Error.error, + background: .Error.errorLight, + border: .Error.error + ) + case "현체", "취업": + return StatusColors( + text: .Gray.gray900, + background: .Gray.gray200, + border: .Gray.gray600 + ) + default: + return StatusColors( + text: .Normal.black, + background: .Gray.gray100, + border: .Gray.gray400 + ) + } + }그런 다음 사용 시:
+let colors = statusColors(student.status) Text(student.status) - .pickText(type: .body2, textColor: statusColor(student.status)) + .pickText(type: .body2, textColor: colors.text) .padding(.horizontal, 16) .padding(.vertical, 8) .background( RoundedRectangle(cornerRadius: 8) - .fill(statusBackgroundColor(student.status)) + .fill(colors.background) ) .overlay( RoundedRectangle(cornerRadius: 8) - .stroke(statusBorderColor(student.status), lineWidth: 1) + .stroke(colors.border, lineWidth: 1) )🤖 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 `@Sources/PiCKAdmin/Features/SelfStudyCheck/Components/SelfStudyCheckComponents.swift` around lines 43 - 86, The three nearly identical switch-based helpers statusColor(_:), statusBackgroundColor(_:), and statusBorderColor(_:) should be consolidated into a single source of truth: define a small value type (e.g., StatusColors with text/background/border Color properties) and implement one function statusColors(_ status: String) -> StatusColors that returns the triple for each case (출석, 이동, 귀가/외출, 현체/취업, default); then replace callers of statusColor/statusBackgroundColor/statusBorderColor to read the appropriate property from statusColors(_:), and remove the old functions to eliminate duplication and keep future status changes in one place.
🤖 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
`@Sources/PiCKAdmin/Features/SelfStudyCheck/Components/SelfStudyCheckComponents.swift`:
- Around line 25-32: The border is invisible for statuses where
statusBackgroundColor(student.status) equals statusBorderColor(student.status)
(e.g., "출석" and "이동"); update the statusBorderColor function (or wherever
statusBorderColor is defined/returned) to return a contrasting color for those
specific status values (use student.status matching "출석"/"이동" or the enum cases)
so the RoundedRectangle.stroke(...) overlay is visually distinct from the
RoundedRectangle.fill(...) background; ensure the change affects all places
using statusBorderColor(student.status) (including the other occurrence
referenced) so the border becomes visible.
- Around line 46-48: Switch cases for "출석" and "이동" currently return
.Normal.white which yields contrast ratios below WCAG AA (4.5:1) against
Primary.primary500 and Gray.gray700; update the return values for the "출석" and
"이동" branches so the text color (or the background color used with them) meets
at least 4.5:1 contrast—either pick a lighter text color token that achieves
≥4.5:1 on Primary.primary500 and Gray.gray700, or darken those background tokens
and verify with a contrast checker; ensure the changes are applied where the
code returns .Normal.white for the "출석" and "이동" cases.
---
Nitpick comments:
In
`@Sources/PiCKAdmin/Features/SelfStudyCheck/Components/SelfStudyCheckComponents.swift`:
- Around line 43-86: The three nearly identical switch-based helpers
statusColor(_:), statusBackgroundColor(_:), and statusBorderColor(_:) should be
consolidated into a single source of truth: define a small value type (e.g.,
StatusColors with text/background/border Color properties) and implement one
function statusColors(_ status: String) -> StatusColors that returns the triple
for each case (출석, 이동, 귀가/외출, 현체/취업, default); then replace callers of
statusColor/statusBackgroundColor/statusBorderColor to read the appropriate
property from statusColors(_:), and remove the old functions to eliminate
duplication and keep future status changes in one place.
🪄 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: 9f9f703a-562a-4bb3-8dd1-36eb649a418a
📒 Files selected for processing (3)
Android/app/src/main/AndroidManifest.xmlSources/PiCKAdmin/Features/BugReport/ViewModels/BugReportViewModel.swiftSources/PiCKAdmin/Features/SelfStudyCheck/Components/SelfStudyCheckComponents.swift
Comment on lines
+25
to
+32
| .background( | ||
| RoundedRectangle(cornerRadius: 8) | ||
| .fill(statusBackgroundColor(student.status)) | ||
| ) | ||
| .overlay( | ||
| RoundedRectangle(cornerRadius: 8) | ||
| .stroke(statusBorderColor(student.status), lineWidth: 1) | ||
| ) |
There was a problem hiding this comment.
"출석"과 "이동" 상태의 테두리가 보이지 않습니다.
"출석" 상태는 배경색과 테두리색이 모두 Primary.primary500로 동일하고, "이동" 상태는 둘 다 Gray.gray700입니다. 배경과 테두리가 같은 색상이면 테두리가 시각적으로 구분되지 않습니다.
의도된 디자인인지 확인이 필요합니다. 만약 테두리를 표시하려는 의도였다면, 테두리 색상을 배경색과 구분되는 색상으로 변경해야 합니다.
🎨 테두리를 보이게 하려면 색상 수정 예시
private func statusBorderColor(_ status: String) -> Color {
switch status {
case "출석":
- return .Primary.primary500
+ return .Primary.primary600 // 또는 더 진한 색상
case "이동":
- return .Gray.gray700
+ return .Gray.gray800 // 또는 더 진한 색상
case "귀가", "외출":
return .Error.errorAlso applies to: 73-86
🤖 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
`@Sources/PiCKAdmin/Features/SelfStudyCheck/Components/SelfStudyCheckComponents.swift`
around lines 25 - 32, The border is invisible for statuses where
statusBackgroundColor(student.status) equals statusBorderColor(student.status)
(e.g., "출석" and "이동"); update the statusBorderColor function (or wherever
statusBorderColor is defined/returned) to return a contrasting color for those
specific status values (use student.status matching "출석"/"이동" or the enum cases)
so the RoundedRectangle.stroke(...) overlay is visually distinct from the
RoundedRectangle.fill(...) background; ensure the change affects all places
using statusBorderColor(student.status) (including the other occurrence
referenced) so the border becomes visible.
Comment on lines
+46
to
+48
| return .Normal.white | ||
| case "이동": | ||
| return .Gray.gray700 | ||
| return .Normal.white |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: 디자인 시스템의 색상 정의를 찾아 Primary.primary500, Gray.gray700, Normal.white 값 확인
rg -n "primary500|gray700" --type swift -A 2 -B 2Repository: DSM-PICK/PiCK_Android_ADMIN
Length of output: 40673
🏁 Script executed:
# Find the Normal.white color definition
rg -n "Normal.*white|white.*=" --type swift Sources/PiCKAdmin/DesignSystem/ -A 1 -B 1Repository: DSM-PICK/PiCK_Android_ADMIN
Length of output: 937
🏁 Script executed:
# Also examine the specific file and lines mentioned in the review
head -80 Sources/PiCKAdmin/Features/SelfStudyCheck/Components/SelfStudyCheckComponents.swift | tail -40Repository: DSM-PICK/PiCK_Android_ADMIN
Length of output: 1090
🏁 Script executed:
# Calculate contrast ratios for the color combinations
python3 << 'EOF'
from fractions import Fraction
def hex_to_rgb(hex_color):
hex_color = hex_color.lstrip('#')
return tuple(int(hex_color[i:i+2], 16) for i in (0, 2, 4))
def get_relative_luminance(r, g, b):
"""Calculate relative luminance according to WCAG formula"""
r = r / 255
g = g / 255
b = b / 255
r = r / 12.92 if r <= 0.03928 else ((r + 0.055) / 1.055) ** 2.4
g = g / 12.92 if g <= 0.03928 else ((g + 0.055) / 1.055) ** 2.4
b = b / 12.92 if b <= 0.03928 else ((b + 0.055) / 1.055) ** 2.4
return 0.2126 * r + 0.7152 * g + 0.0722 * b
def get_contrast_ratio(lum1, lum2):
"""Calculate contrast ratio"""
lighter = max(lum1, lum2)
darker = min(lum1, lum2)
return (lighter + 0.05) / (darker + 0.05)
# Colors
white_rgb = (255, 255, 255) # Normal.white
primary500_rgb = hex_to_rgb("`#9650FA`")
gray700_rgb = hex_to_rgb("`#79777C`")
# Calculate luminances
white_lum = get_relative_luminance(*white_rgb)
primary500_lum = get_relative_luminance(*primary500_rgb)
gray700_lum = get_relative_luminance(*gray700_rgb)
# Calculate contrast ratios
ratio_white_on_primary = get_contrast_ratio(white_lum, primary500_lum)
ratio_white_on_gray = get_contrast_ratio(white_lum, gray700_lum)
print(f"White (`#FFFFFF`) on Primary.primary500 (`#9650FA`): {ratio_white_on_primary:.2f}:1")
print(f"White (`#FFFFFF`) on Gray.gray700 (`#79777C`): {ratio_white_on_gray:.2f}:1")
print()
print("WCAG AA Requirements:")
print("- Normal text: 4.5:1")
print("- Large text (18pt+/bold 14pt+): 3:1")
print()
if ratio_white_on_primary < 4.5:
print(f"✗ Primary.primary500: FAILS (Current: {ratio_white_on_primary:.2f}:1)")
if ratio_white_on_gray < 4.5:
print(f"✗ Gray.gray700: FAILS (Current: {ratio_white_on_gray:.2f}:1)")
EOFRepository: DSM-PICK/PiCK_Android_ADMIN
Length of output: 345
접근성: 흰색 텍스트의 명암비가 WCAG AA 기준을 충족하지 못합니다.
"출석" 상태는 흰색 텍스트를 Primary.primary500(#9650FA) 배경에 표시하고, "이동" 상태는 흰색 텍스트를 Gray.gray700(#79777C) 배경에 표시합니다.
실제 명암비 계산 결과:
- 흰색 on Primary.primary500: 4.35:1 (필요: 4.5:1)
- 흰색 on Gray.gray700: 4.43:1 (필요: 4.5:1)
두 조합 모두 일반 텍스트 기준(4.5:1)을 0.07~0.15 포인트 하회합니다. 텍스트 색상을 밝게 변경하거나 배경색을 어둡게 하여 명암비를 4.5:1 이상으로 개선해야 합니다.
🤖 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
`@Sources/PiCKAdmin/Features/SelfStudyCheck/Components/SelfStudyCheckComponents.swift`
around lines 46 - 48, Switch cases for "출석" and "이동" currently return
.Normal.white which yields contrast ratios below WCAG AA (4.5:1) against
Primary.primary500 and Gray.gray700; update the return values for the "출석" and
"이동" branches so the text color (or the background color used with them) meets
at least 4.5:1 contrast—either pick a lighter text color token that achieves
≥4.5:1 on Primary.primary500 and Gray.gray700, or darken those background tokens
and verify with a contrast checker; ensure the changes are applied where the
code returns .Normal.white for the "출석" and "이동" cases.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
1 participant
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.
Summary
BugReportViewModel: 이미지 업로드 응답 타입[String]→ImageUploadResponse로 수정 (서버 응답{"file_name": [...]}형식에 맞게)AndroidManifest:READ_MEDIA_IMAGES(Android 13+),READ_EXTERNAL_STORAGE(Android ≤12) 권한 추가Test plan
close #20
🤖 Generated with Claude Code
Summary by CodeRabbit
릴리스 노트
Style
Bug Fixes
Chores