Conversation
soseoyo12
requested review from
sum130,
wotjs020708 and
y-eonee
and removed request for
a team
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthrough홈 및 온보딩 UI의 레이아웃·이미지 스케일 조정, 온보딩 밑줄의 동적 너비 측정(PrefKey 추가), CherrishPicker의 Changes
Sequence Diagram(s)(생성 조건 미충족 — 생략) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 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.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Onboarding/OnboardingPage1.swift`:
- Around line 65-74: The hidden measurement view using TypographyText within the
background (GeometryReader setting Line1WidthKey) currently uses .opacity(0)
which leaves it in the accessibility tree and will be read by VoiceOver; update
the background measurement view (the VStack containing TypographyText and
GeometryReader that writes Line1WidthKey) to also call
.accessibilityHidden(true) so the measurement text is excluded from
accessibility without changing layout.
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/CherrishPicker.swift`:
- Line 13: selection이 range 밖으로 설정되면 makeUIView 및 updateUIView에서 selectRow에 전달되는
row가 음수거나 범위를 초과해 크래시가 발생할 수 있으니, makeUIView(...)와 updateUIView(...)에서
selectRow를 호출하기 전에 selection을 range 내로 클램프하거나 검증하여 유효한 Int 인덱스로 변환하세요 (예: let
safeRow = min(max(selection, range.lowerBound), range.upperBound) 형태로),
scrollViewDidScroll과 동일한 검증 로직을 재사용하거나 별도 유틸(예: clampedSelection)으로 추출해 중복 제거하고
selectRow 호출 시 safeRow를 사용하도록 수정하세요.
In
`@Cherrish-iOS/Cherrish-iOS/Resources/Assets.xcassets/home_icon/home_lv.2.imageset/Contents.json`:
- Around line 4-14: The imageset folder is named home_lv.2.imageset but the
image files it contains are named home_lv.1.png, home_lv.1@2x.png, and
home_lv.1@3x.png, creating a naming mismatch. To resolve this, choose one
approach: either rename all three image files to home_lv.2.png,
home_lv.2@2x.png, and home_lv.2@3x.png to match the folder name and update their
references in Contents.json accordingly, or rename the imageset folder to
home_lv.1.imageset to match the actual file names.
| .background( | ||
| VStack(alignment: .leading, spacing: 0) { | ||
| TypographyText("시술 후 불편감이 남을 수 있는 기간", style: .title1_sb_18, color: .clear) | ||
| .background( | ||
| GeometryReader { geometry in | ||
| Color.clear.preference(key: Line1WidthKey.self, value: geometry.size.width) | ||
| } | ||
| ) | ||
| } | ||
| .opacity(0) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the OnboardingPage1.swift file
find . -name "OnboardingPage1.swift" -type fRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Read the OnboardingPage1.swift file to see the code context around lines 65-74
sed -n '55,85p' "./Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Onboarding/OnboardingPage1.swift" | cat -nRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 1567
🏁 Script executed:
#!/bin/bash
# Check the full file to see if there are any other accessibility configurations
grep -n "accessibilityHidden\|isHidden\|accessibility" "./Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Onboarding/OnboardingPage1.swift"Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 52
🌐 Web query:
SwiftUI opacity accessibility hidden VoiceOver separate from visual rendering
💡 Result:
Short answer: setting .opacity(0) only changes visual rendering — it does not reliably remove a SwiftUI view from the accessibility tree or stop it receiving hits. To keep a view visually present/transparent but hide it from VoiceOver and user interaction, combine accessibility and interaction modifiers (or remove the view from the tree). Example:
- Hide from VoiceOver: .accessibilityHidden(true) or wrap in a container and use .accessibilityElement(children: .ignore/.combine) as appropriate. [1][2]
- Prevent taps/gestures: .allowsHitTesting(false) or .disabled(true). [3]
- If you need the view completely gone (no layout space), conditionally remove it with if/else rather than using opacity. [4]
Example:
MyView()
.opacity(0) // visual transparency
.accessibilityHidden(true) // hide from VoiceOver
.allowsHitTesting(false) // disable interaction
References:
[1] SwiftUI accessibility modifiers (accessibilityHidden / accessibilityElement).
[2] Guide/discussion on hiding elements for VoiceOver and using .accessibilityElement(children: .ignore/.combine).
[3] Practical advice to combine .opacity(0) with .disabled / .allowsHitTesting to stop interaction (StackOverflow).
[4] Notes on .hidden()/.opacity vs removing view (layout and accessibility differences).
🏁 Script executed:
#!/bin/bash
# Verify the exact line numbers by checking the file again more precisely
wc -l "./Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Onboarding/OnboardingPage1.swift"Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 154
🏁 Script executed:
#!/bin/bash
# Get the exact lines 65-74 to verify the line numbers in the review comment
sed -n '65,74p' "./Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Onboarding/OnboardingPage1.swift" | cat -nRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 662
숨겨진 측정 텍스트가 VoiceOver에 노출됩니다.
opacity(0)은 시각적 렌더링만 변경하며 접근성 트리에서 제거되지 않아 VoiceOver 사용자에게 보이지 않는 텍스트가 읽혀집니다. 측정용 뷰를 접근성에서 숨기기 위해 .accessibilityHidden(true)을 추가해주세요.
제안 수정
}
.opacity(0)
+ .accessibilityHidden(true)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .background( | |
| VStack(alignment: .leading, spacing: 0) { | |
| TypographyText("시술 후 불편감이 남을 수 있는 기간", style: .title1_sb_18, color: .clear) | |
| .background( | |
| GeometryReader { geometry in | |
| Color.clear.preference(key: Line1WidthKey.self, value: geometry.size.width) | |
| } | |
| ) | |
| } | |
| .opacity(0) | |
| .background( | |
| VStack(alignment: .leading, spacing: 0) { | |
| TypographyText("시술 후 불편감이 남을 수 있는 기간", style: .title1_sb_18, color: .clear) | |
| .background( | |
| GeometryReader { geometry in | |
| Color.clear.preference(key: Line1WidthKey.self, value: geometry.size.width) | |
| } | |
| ) | |
| } | |
| .opacity(0) | |
| .accessibilityHidden(true) |
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Onboarding/OnboardingPage1.swift`
around lines 65 - 74, The hidden measurement view using TypographyText within
the background (GeometryReader setting Line1WidthKey) currently uses .opacity(0)
which leaves it in the accessibility tree and will be read by VoiceOver; update
the background measurement view (the VStack containing TypographyText and
GeometryReader that writes Line1WidthKey) to also call
.accessibilityHidden(true) so the measurement text is excluded from
accessibility without changing layout.
| struct CherrishPicker: View { | ||
| @Binding var selection: Int | ||
| let range: ClosedRange<Int> | ||
| var range: ClosedRange<Int> = 0...30 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "CherrishPicker.swift" -type fRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 149
🏁 Script executed:
cat -n ./Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/CherrishPicker.swiftRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 7294
프로그래매틱 업데이트 시 range 범위 검증 누락으로 크래시 위험
selection이 range 범위 밖으로 설정될 때, makeUIView(라인 43)와 updateUIView(라인 66)에서 selectRow에 전달하는 row 값이 음수이거나 범위를 초과할 수 있습니다. scrollViewDidScroll(라인 152)에서는 이미 clamping으로 범위를 검증하고 있으나, 프로그래매틱 업데이트 경로에서 이 검증이 누락되어 런타임 오류가 발생할 수 있습니다.
🔧 제안 수정
func makeUIView(context: Context) -> UIPickerView {
let picker = UIPickerView()
picker.delegate = context.coordinator
picker.dataSource = context.coordinator
picker.subviews.forEach { subview in
subview.backgroundColor = .clear
}
- let row = selection - range.lowerBound
+ let rawRow = selection - range.lowerBound
+ let row = max(0, min(rawRow, range.count - 1))
+ if rawRow != row {
+ selection = range.lowerBound + row
+ }
picker.selectRow(row, inComponent: 0, animated: false) func updateUIView(_ uiView: UIPickerView, context: Context) {
guard !context.coordinator.isScrolling else { return }
- let row = selection - range.lowerBound
+ let rawRow = selection - range.lowerBound
+ let row = max(0, min(rawRow, range.count - 1))
+ if rawRow != row {
+ selection = range.lowerBound + row
+ }
if uiView.selectedRow(inComponent: 0) != row {
uiView.selectRow(row, inComponent: 0, animated: true)
}
}🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/CherrishPicker.swift`
at line 13, selection이 range 밖으로 설정되면 makeUIView 및 updateUIView에서 selectRow에
전달되는 row가 음수거나 범위를 초과해 크래시가 발생할 수 있으니, makeUIView(...)와 updateUIView(...)에서
selectRow를 호출하기 전에 selection을 range 내로 클램프하거나 검증하여 유효한 Int 인덱스로 변환하세요 (예: let
safeRow = min(max(selection, range.lowerBound), range.upperBound) 형태로),
scrollViewDidScroll과 동일한 검증 로직을 재사용하거나 별도 유틸(예: clampedSelection)으로 추출해 중복 제거하고
selectRow 호출 시 safeRow를 사용하도록 수정하세요.
| "filename" : "home_lv.1.png", | ||
| "idiom" : "universal", | ||
| "scale" : "1x" | ||
| }, | ||
| { | ||
| "filename" : "home_lv.2@2x.png", | ||
| "filename" : "home_lv.1@2x.png", | ||
| "idiom" : "universal", | ||
| "scale" : "2x" | ||
| }, | ||
| { | ||
| "filename" : "home_lv.2@3x.png", | ||
| "filename" : "home_lv.1@3x.png", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify home_lv.2.imageset folder contents and file naming
echo "=== Contents of home_lv.2.imageset folder ==="
find . -path "*home_lv.2.imageset*" -type f
echo ""
echo "=== All home_lv related PNG files ==="
find . -name "home_lv*.png" -type f | sort
echo ""
echo "=== home_lv.2.imageset/Contents.json ==="
find . -path "*home_lv.2.imageset/Contents.json" -type f -exec cat {} \;Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 2163
🏁 Script executed:
#!/bin/bash
# Verify Contents.json matches actual files in home_lv.2.imageset
echo "=== Checking if all referenced files in Contents.json exist ==="
IMAGESET_PATH="./Cherrish-iOS/Cherrish-iOS/Resources/Assets.xcassets/home_icon/home_lv.2.imageset"
cd "$IMAGESET_PATH" 2>/dev/null && {
echo "Referenced files in Contents.json:"
grep '"filename"' Contents.json | grep -o '"[^"]*\.png"'
echo ""
echo "Actual PNG files in folder:"
ls -1 *.png 2>/dev/null
echo ""
echo "Verification: All referenced files exist?"
for file in home_lv.1.png home_lv.1@2x.png home_lv.1@3x.png; do
if [ -f "$file" ]; then
echo "✓ $file exists"
else
echo "✗ $file missing"
fi
done
} || echo "Could not access imageset folder"Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 417
명명 규칙 일관성 개선 권장
home_lv.2.imageset 폴더의 파일들이 실제로 home_lv.1.*로 명명되어 있으며, Contents.json에서 정확히 이 파일들을 참조하고 있습니다. 파일이 올바르게 존재하고 참조되므로 이미지 로드에는 문제가 없습니다.
다만 imageset 폴더명과 파일명의 불일치는 유지보수성을 떨어뜨리므로, 일관된 명명 규칙으로 통일하는 것이 권장됩니다. (예: 폴더 내 파일을 home_lv.2.*로 명명하거나 폴더명을 home_lv.1.imageset으로 조정)
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Resources/Assets.xcassets/home_icon/home_lv.2.imageset/Contents.json`
around lines 4 - 14, The imageset folder is named home_lv.2.imageset but the
image files it contains are named home_lv.1.png, home_lv.1@2x.png, and
home_lv.1@3x.png, creating a naming mismatch. To resolve this, choose one
approach: either rename all three image files to home_lv.2.png,
home_lv.2@2x.png, and home_lv.2@3x.png to match the folder name and update their
references in Contents.json accordingly, or rename the imageset folder to
home_lv.1.imageset to match the actual file names.
| private struct Line1WidthKey: PreferenceKey { | ||
| static var defaultValue: CGFloat = 0 | ||
| static func reduce(value: inout CGFloat, nextValue: () -> CGFloat) { | ||
| value = nextValue() | ||
| } |
There was a problem hiding this comment.
이거 제가 PreferenceKey 만들어둔 파일 있는데 거기다가 옮겨주세여
4 participants
🔗 연결된 이슈
📄 작업 내용