Conversation
knine79
requested review from
jhseo and
szzang-kr
and removed request for
a team
|
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패키지 기본 로컬라이제이션을 한국어로 설정하고, 다수의 SwiftUI 컴포넌트에 접근성 레이블·값·트레이트 및 접근성 조정 로직을 추가하며 한국어 로컬라이제이션 리소스를 도입합니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Montage/1 Components/3 Selection And Input/Control.swift (1)
94-110:⚠️ Potential issue | 🟠 Major
.switchvariant에 접근성 레이블과 값이 누락되었습니다.
.labelsHidden()은 시각적 레이블만 숨기고 VoiceOver 레이블을 제공하지 않습니다. 다른 variant(checkbox, radio, checkmark)와 달리.switch에는accessibilityLabel,accessibilityValue등이 적용되지 않아 VoiceOver 사용자가 스위치의 용도와 상태를 알 수 없습니다.♿ 접근성 수정 제안
Toggle( "", isOn: Binding( get: { !state.isUnchecked }, set: { onSelect?($0 ? .checked : .unchecked) }) ) .labelsHidden() .disabled(disable) .tint(backgroundColor) .frame(width: switchSize.width, height: switchSize.height) // ... 기존 코드 ... .frame(width: boxSize.width, height: boxSize.height) +.accessibilityLabel(label) +.accessibilityValue(accessibilityStateDescription)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Montage/1` Components/3 Selection And Input/Control.swift around lines 94 - 110, The .switch variant currently only uses .labelsHidden() in the modifier chain (after .tint(...).frame(...).offset(...).transformEffect(...).frame(...).offset(...)) and lacks VoiceOver metadata; update the switch view’s modifier chain to add appropriate accessibility modifiers (e.g., .accessibilityLabel(...) using the switch’s descriptive label text and .accessibilityValue(...) or .accessibilityState(...) reflecting its on/off state, and .accessibilityHint(...) if needed) so screen readers announce both purpose and current state—apply these to the same view that calls .labelsHidden() to mirror how checkbox/radio variants expose accessibility information.
🧹 Nitpick comments (2)
Sources/Montage/1 Components/6 Navigations/Tab.swift (1)
118-120: 탭 접근성 구현이 올바릅니다.
.isButton과.isSelectedtrait 적용이 적절합니다. 다만, 선택 상태 처리 로직을 더 간결하게 작성할 수 있습니다.♻️ 선택적 리팩토링 제안
.accessibilityAddTraits(.isButton) -.accessibilityAddTraits(index == selectedIndex ? .isSelected : []) -.accessibilityRemoveTraits(index == selectedIndex ? [] : .isSelected) +.accessibilityAddTraits(index == selectedIndex ? [.isButton, .isSelected] : .isButton)또는 현재 방식을 유지해도 동작에는 문제가 없습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Montage/1` Components/6 Navigations/Tab.swift around lines 118 - 120, The accessibility trait handling for the tab is correct but verbose; simplify by computing the combined trait set from index and selectedIndex and applying it once instead of adding then removing traits. Replace the three calls (.accessibilityAddTraits(.isButton), .accessibilityAddTraits(index == selectedIndex ? .isSelected : []), .accessibilityRemoveTraits(index == selectedIndex ? [] : .isSelected)) with a single application that composes .isButton plus .isSelected when index == selectedIndex (use the local index and selectedIndex variables or build an AccessibilityTraits value) and call the appropriate accessibility modifier once.Sources/Montage/1 Components/3 Selection And Input/TextArea.swift (1)
432-433: heading이 없을 때 접근성 레이블이 비어있음
heading이nil인 경우 접근성 레이블이 빈 문자열이 됩니다.placeholder를 fallback으로 사용하면 VoiceOver 사용자에게 텍스트 영역의 목적을 전달할 수 있습니다.♿ Placeholder를 fallback 레이블로 사용
- .accessibilityLabel(heading ?? "") + .accessibilityLabel(heading ?? placeholder ?? "")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Montage/1` Components/3 Selection And Input/TextArea.swift around lines 432 - 433, The accessibilityLabel currently uses heading ?? "" which yields an empty label when heading is nil; update the TextArea's accessibilityLabel call to fallback to the placeholder (e.g., use heading ?? placeholder ?? "") so VoiceOver receives a meaningful label; locate the accessibilityLabel(...) invocation in TextArea.swift and replace the fallback logic so it uses placeholder when heading is nil.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/Montage/1` Components/2 Actions/Button.swift:
- Around line 368-371: The Button's accessibilityLabel currently uses
accessibilityLabel(text ?? "") which yields an empty string for icon-only
buttons; update the Button initializers and view modifiers (in Button.swift, the
Button component/initializer used for icon-only buttons and the view body
applying .accessibilityLabel) to accept an optional accessibilityLabel parameter
(or compute a fallback) and change the modifier to use
accessibilityLabel(customAccessibilityLabel ?? text ?? iconAccessibilityName ??
String(localized: "Button", bundle: .module)) so icon-only buttons expose a
descriptive label (use the icon's name or a provided custom label as fallback)
and ensure .accessibilityValue(loading ? ...) remains unchanged.
In `@Sources/Montage/1` Components/2 Actions/IconButton.swift:
- Around line 225-227: Add an optional accessibilityLabel parameter to the
IconButton initializer (e.g., public init(..., icon: Icon, accessibilityLabel:
String? = nil, handler: (() -> Void)? = nil)) and change the view’s modifier
from .accessibilityLabel(icon.rawValue) to
.accessibilityLabel(accessibilityLabel ?? icon.rawValue) so callers can supply
meaningful, localized labels (or continue to fall back to the enum rawValue);
alternatively implement a localized label property on the Icon enum and use that
when accessibilityLabel is nil.
In `@Sources/Montage/1` Components/3 Selection And Input/Slider.swift:
- Around line 235-236: The slider's accessibility currently only sets
accessibilityValue(headingLabel) and ignores adjustable behavior; update the
Slider view to add .accessibilityAddTraits(.adjustable), provide a descriptive
.accessibilityLabel (e.g., use headingLabel or a dedicated label string) and
implement .accessibilityAdjustableAction to call the same increment/decrement
logic used by the slider (use the Slider struct's value-binding/step handlers or
the existing increase/decrease methods) so VoiceOver users can perform
.increment and .decrement actions while preserving .accessibilityValue updates.
In `@Sources/Montage/1` Components/4 Contents/Avatar.swift:
- Around line 178-180: The Avatar view currently hides children with
.accessibilityElement(children: .ignore) and conditionally adds
.accessibilityAddTraits(.isButton) but lacks an accessibility label; update
Avatar to accept an optional accessibilityLabel parameter (or provide a
modifier) and in the view body apply .accessibilityLabel(accessibilityLabel ??
defaultLabel(for: variant)) so VoiceOver gets either the passed label or a
sensible default derived from the variant (use the existing variant enum and
onTap presence to build defaults like "User avatar", "Company logo", etc.);
ensure the .accessibilityAddTraits(.isButton) remains when onTap != nil and that
the new accessibilityLabel is applied alongside the existing
.accessibilityElement call.
In `@Sources/Montage/1` Components/4 Contents/AvatarGroup.swift:
- Line 143: The AvatarGroup view currently uses .accessibilityElement(children:
.ignore) without providing a label, so VoiceOver users can't know the group or
its count; update the AvatarGroup SwiftUI view to supply an accessibility
label/value (e.g., use .accessibilityLabel and/or .accessibilityValue on the
view that calls .accessibilityElement(children: .ignore)) that summarizes the
group (for example "3 avatars" or a descriptive string built from the view's
avatars/participants property), and ensure the label updates when the underlying
avatars/participants collection or count changes (reference the AvatarGroup
struct and the place where .accessibilityElement(children: .ignore) is applied).
---
Outside diff comments:
In `@Sources/Montage/1` Components/3 Selection And Input/Control.swift:
- Around line 94-110: The .switch variant currently only uses .labelsHidden() in
the modifier chain (after
.tint(...).frame(...).offset(...).transformEffect(...).frame(...).offset(...))
and lacks VoiceOver metadata; update the switch view’s modifier chain to add
appropriate accessibility modifiers (e.g., .accessibilityLabel(...) using the
switch’s descriptive label text and .accessibilityValue(...) or
.accessibilityState(...) reflecting its on/off state, and
.accessibilityHint(...) if needed) so screen readers announce both purpose and
current state—apply these to the same view that calls .labelsHidden() to mirror
how checkbox/radio variants expose accessibility information.
---
Nitpick comments:
In `@Sources/Montage/1` Components/3 Selection And Input/TextArea.swift:
- Around line 432-433: The accessibilityLabel currently uses heading ?? "" which
yields an empty label when heading is nil; update the TextArea's
accessibilityLabel call to fallback to the placeholder (e.g., use heading ??
placeholder ?? "") so VoiceOver receives a meaningful label; locate the
accessibilityLabel(...) invocation in TextArea.swift and replace the fallback
logic so it uses placeholder when heading is nil.
In `@Sources/Montage/1` Components/6 Navigations/Tab.swift:
- Around line 118-120: The accessibility trait handling for the tab is correct
but verbose; simplify by computing the combined trait set from index and
selectedIndex and applying it once instead of adding then removing traits.
Replace the three calls (.accessibilityAddTraits(.isButton),
.accessibilityAddTraits(index == selectedIndex ? .isSelected : []),
.accessibilityRemoveTraits(index == selectedIndex ? [] : .isSelected)) with a
single application that composes .isButton plus .isSelected when index ==
selectedIndex (use the local index and selectedIndex variables or build an
AccessibilityTraits value) and call the appropriate accessibility modifier once.
🪄 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: f257ce34-69e5-43b5-ad0a-ff529c91ec30
📒 Files selected for processing (23)
Package.swiftSources/Montage/1 Components/2 Actions/Button.swiftSources/Montage/1 Components/2 Actions/Chip.swiftSources/Montage/1 Components/2 Actions/IconButton.swiftSources/Montage/1 Components/3 Selection And Input/Control.swiftSources/Montage/1 Components/3 Selection And Input/FilterButton.swiftSources/Montage/1 Components/3 Selection And Input/SegmentedControl.swiftSources/Montage/1 Components/3 Selection And Input/Select.swiftSources/Montage/1 Components/3 Selection And Input/Slider.swiftSources/Montage/1 Components/3 Selection And Input/TextArea.swiftSources/Montage/1 Components/3 Selection And Input/TextField.swiftSources/Montage/1 Components/4 Contents/Avatar.swiftSources/Montage/1 Components/4 Contents/AvatarGroup.swiftSources/Montage/1 Components/4 Contents/Card.swiftSources/Montage/1 Components/4 Contents/ListCard.swiftSources/Montage/1 Components/4 Contents/ListCell.swiftSources/Montage/1 Components/5 Loading/Loading.swiftSources/Montage/1 Components/5 Loading/Skeleton.swiftSources/Montage/1 Components/6 Navigations/Tab.swiftSources/Montage/1 Components/6 Navigations/TopNavigation.swiftSources/Montage/1 Components/7 Feedback/Toast.swiftSources/Montage/1 Components/8 Presentation/Tooltip.swiftSources/Montage/Asset/Localizable.xcstrings
Sources/Montage/1 Components/3 Selection And Input/Slider.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Sources/Montage/1 Components/3 Selection And Input/Slider.swift (1)
235-251:⚠️ Potential issue | 🟠 Major
.accessibilityLabel누락으로 슬라이더 용도 설명 불가
accessibilityValue만 설정되어 있어 VoiceOver 사용자가 이 컨트롤이 무엇인지 알 수 없습니다. 슬라이더의 목적을 설명하는 레이블을 추가해야 합니다.또한
disable상태일 때 VoiceOver에 비활성화 상태가 전달되지 않습니다.♿ 접근성 개선 제안
.allowsHitTesting(!disable) .accessibilityElement(children: .ignore) +.accessibilityLabel(String(localized: "슬라이더", bundle: .module)) .accessibilityValue(headingLabel) +.accessibilityAddTraits(disable ? .isNotEnabled : []) .accessibilityAdjustableAction { direction in🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Montage/1` Components/3 Selection And Input/Slider.swift around lines 235 - 251, Accessibility is missing a label and disabled state reporting: add .accessibilityLabel(...) alongside the existing .accessibilityValue(...) (e.g., use the control's descriptive text instead of only the value) and when the slider is not interactive add the disabled trait via .accessibilityAddTraits(.isDisabled) (or .accessibilityAddTraits(.notEnabled) if your API variant uses that); update the chain around .accessibilityElement(children: .ignore), .accessibilityValue(headingLabel), and .accessibilityAdjustableAction to include .accessibilityLabel(headingLabel) and a conditional .accessibilityAddTraits(.isDisabled) tied to your slider’s enabled/disabled flag (e.g., isEnabled or isDisabled).
🧹 Nitpick comments (1)
Sources/Montage/1 Components/3 Selection And Input/Slider.swift (1)
237-250: 범위 슬라이더의 VoiceOver 조작이 제한적입니다현재 구현에서:
- Increment:
thumbRatio2(상한값)만 증가- Decrement:
thumbRatio1(하한값)만 감소이 로직은 범위를 확장만 할 수 있고 축소는 불가능합니다. VoiceOver 사용자가 양쪽 thumb을 자유롭게 조절할 수 없어 시각 사용자와 동등한 제어가 어렵습니다.
💡 대안 제안: 마지막으로 선택한 thumb 기준 조절
.accessibilityAdjustableAction { direction in let step = 0.05 switch direction { case .increment: - thumbRatio2 = min(1.0, thumbRatio2 + step) + if isRangeSlider { + if focusedThumb == 1 { + thumbRatio1 = min(thumbRatio2, thumbRatio1 + step) + } else { + thumbRatio2 = min(1.0, thumbRatio2 + step) + } + } else { + thumbRatio2 = min(1.0, thumbRatio2 + step) + } case .decrement: if isRangeSlider { - thumbRatio1 = max(0.0, thumbRatio1 - step) + if focusedThumb == 1 { + thumbRatio1 = max(0.0, thumbRatio1 - step) + } else { + thumbRatio2 = max(thumbRatio1, thumbRatio2 - step) + } } else { thumbRatio2 = max(0.0, thumbRatio2 - step) } `@unknown` default: break } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Montage/1` Components/3 Selection And Input/Slider.swift around lines 237 - 250, The VoiceOver adjustable action currently only moves thumbRatio2 on increment and moves thumbRatio1 (if range) or thumbRatio2 on decrement, preventing shrinking a range; update the accessibilityAdjustableAction handler to operate on whichever thumb was last selected (introduce or use a lastSelectedThumb or selectedThumb enum/flag alongside thumbRatio1 and thumbRatio2) so both increment and decrement apply to that active thumb, clamp values within 0.0...1.0 and preserve range ordering (swap or constrain the other thumb if needed) when isRangeSlider is true; ensure the same lastSelectedThumb is set when a thumb is tapped/driven so VoiceOver adjusts the intended thumb.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Sources/Montage/1` Components/3 Selection And Input/Slider.swift:
- Around line 235-251: Accessibility is missing a label and disabled state
reporting: add .accessibilityLabel(...) alongside the existing
.accessibilityValue(...) (e.g., use the control's descriptive text instead of
only the value) and when the slider is not interactive add the disabled trait
via .accessibilityAddTraits(.isDisabled) (or
.accessibilityAddTraits(.notEnabled) if your API variant uses that); update the
chain around .accessibilityElement(children: .ignore),
.accessibilityValue(headingLabel), and .accessibilityAdjustableAction to include
.accessibilityLabel(headingLabel) and a conditional
.accessibilityAddTraits(.isDisabled) tied to your slider’s enabled/disabled flag
(e.g., isEnabled or isDisabled).
---
Nitpick comments:
In `@Sources/Montage/1` Components/3 Selection And Input/Slider.swift:
- Around line 237-250: The VoiceOver adjustable action currently only moves
thumbRatio2 on increment and moves thumbRatio1 (if range) or thumbRatio2 on
decrement, preventing shrinking a range; update the
accessibilityAdjustableAction handler to operate on whichever thumb was last
selected (introduce or use a lastSelectedThumb or selectedThumb enum/flag
alongside thumbRatio1 and thumbRatio2) so both increment and decrement apply to
that active thumb, clamp values within 0.0...1.0 and preserve range ordering
(swap or constrain the other thumb if needed) when isRangeSlider is true; ensure
the same lastSelectedThumb is set when a thumb is tapped/driven so VoiceOver
adjusts the intended thumb.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 53a6521c-3e2e-4b73-a523-8c9a4f047b09
📒 Files selected for processing (6)
Sources/Montage/1 Components/2 Actions/Button.swiftSources/Montage/1 Components/2 Actions/IconButton.swiftSources/Montage/1 Components/3 Selection And Input/Slider.swiftSources/Montage/1 Components/4 Contents/Avatar.swiftSources/Montage/1 Components/4 Contents/AvatarGroup.swiftSources/Montage/Asset/Localizable.xcstrings
🚧 Files skipped from review as they are similar to previous changes (4)
- Sources/Montage/1 Components/4 Contents/AvatarGroup.swift
- Sources/Montage/1 Components/2 Actions/Button.swift
- Sources/Montage/1 Components/2 Actions/IconButton.swift
- Sources/Montage/Asset/Localizable.xcstrings
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/Montage/1` Components/3 Selection And Input/Slider.swift:
- Around line 279-299: handleAccessibilityAdjust currently lets VoiceOver adjust
the slider even when disable == true; add an early check at the start of
handleAccessibilityAdjust to return immediately if disable is true (or otherwise
prevents adjustments when disabled), so no changes are made to
thumbRatio1/thumbRatio2 when disabled; keep the existing logic for
isRangeSlider, focusedThumb, and the increment/decrement branches intact but
gated by the disable check (also remove reliance on .allowsHitTesting(!disable)
for accessibility).
🪄 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: 73dcc376-01f6-4a03-8dac-9e1d07e82af4
📒 Files selected for processing (2)
Sources/Montage/1 Components/3 Selection And Input/Slider.swiftSources/Montage/Asset/Localizable.xcstrings
🚧 Files skipped from review as they are similar to previous changes (1)
- Sources/Montage/Asset/Localizable.xcstrings
szzang-kr
approved these changes
Mar 20, 2026
szzang-kr
added
accepted
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.
개요
수정사항
.accessibilityElement,.accessibilityLabel,.accessibilityAddTraits(.isButton)적용.accessibilityValue로 선택 상태 전달.isSelectedtrait 적용.accessibilityElement(children: .combine)적용.accessibilityElement(children: .combine)적용String(localized:bundle:.module)로 현지화 준비Localizable.xcstrings파일 추가 및Package.swift에defaultLocalization: "ko"설정미리보기
접근성 변경은 시각적 차이 없음 — Accessibility Inspector 또는 VoiceOver로 검증
Summary by CodeRabbit
새로운 기능
지역화