Design/#14 - DVRadioButton/Group 컴포넌트 구현#15
Open
doyeonk429 wants to merge 3 commits into
Open
Conversation
- DVRadioButton: 16pt 원형 인디케이터, 기본/선택 상태, 텍스트 라벨 편의 init - DVRadioButtonGroup: XS/SM/MD 사이즈 변형(spacing 8/20/56, minWidth 180/330/380) - macOS 키보드 네비게이션: Tab 포커스, ←/→ 그룹 내 이동, Space 활성화 - Hit target 24pt 확대(시각은 16pt 유지), 시스템 포커스 링 비활성화 - DVColor 토큰만 사용, Swift DocC 상세 주석 작성
DVRadioButton(Default/Selected/Interactive) 및 DVRadioButtonGroup(XS/SM/MD) 케이스를 ContentView 라우팅에 연결
2 tasks
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
WalkthroughDVRadioButton과 DVRadioButtonGroup 컴포넌트를 신규 구현하고 샘플 앱에 통합했습니다. 라디오 버튼은 16pt 인디케이터, 탭/키보드 상호작용, 포커싱을 지원하며, 그룹은 화살표 키 네비게이션과 크기 제어를 제공합니다. Changes라디오 버튼 컴포넌트 개발 및 통합
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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
🧹 Nitpick comments (3)
Projects/DVDesign/SampleApp/Sources/RadioButtonPreviewView.swift (1)
12-14: 💤 Low value중첩 타입 이름
Environment— SwiftUI 표준 심볼과 혼동 우려이 파일에선
@Environment프로퍼티 래퍼를 쓰지 않아 컴파일은 무방하지만, SwiftUI의 표준Environment와 동명이라 추후 같은 파일에@Environment(\.foo) var foo같은 코드가 들어오면 의도치 않은 모호성이 생길 수 있어요.Env정도로 짧게 두는 편이 의도가 더 분명합니다.♻️ 제안 diff
- `@State` private var xsSelection: Environment = .dev - `@State` private var smSelection: Environment = .staging - `@State` private var mdSelection: Environment = .prod + `@State` private var xsSelection: Env = .dev + `@State` private var smSelection: Env = .staging + `@State` private var mdSelection: Env = .prod - private enum Environment: Hashable { + private enum Env: Hashable { case dev, staging, prod } - private var items: [DVRadioButtonGroup<Environment>.Item] { + private var items: [DVRadioButtonGroup<Env>.Item] {🤖 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 `@Projects/DVDesign/SampleApp/Sources/RadioButtonPreviewView.swift` around lines 12 - 14, The nested enum named Environment conflicts semantically with SwiftUI's Environment symbol; rename the enum Environment (in RadioButtonPreviewView.swift) to a less ambiguous name like Env (or RadioEnv) and update all references (e.g., usages in RadioButtonPreviewView and any associated initializers or switch/case) to the new identifier, keeping the same conformance (Hashable) and cases (dev, staging, prod) to avoid behavioral changes.Projects/DVDesign/SampleApp/Sources/ContentView.swift (1)
34-42: 💤 Low value문자열 기반 라우팅 — 타입 안전한 매핑 고려
component.name문자열 매칭은 오타 한 글자에 조용히 placeholder로 빠지는 구조라 장기적으로는 위험합니다. 컴포넌트 수가 늘어나기 전에 enum 라우트 또는Component에@ViewBuilder destination핸들을 들고 다니게 하면 더 안전합니다. 지금 단계에선 우선순위 낮은 개선 제안입니다.🤖 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 `@Projects/DVDesign/SampleApp/Sources/ContentView.swift` around lines 34 - 42, The switch in detailView(for:) relies on string matching of component.name which is fragile; refactor to a type-safe mapping by adding either an enum (e.g., ComponentType) used by Component or a computed `@ViewBuilder` property on Component (e.g., var destinationView: some View) and update detailView(for:) to use that instead of string literals; locate Component, the detailView(for:) function, RadioButtonPreviewView and ComponentPlaceholderView and implement the enum or computed view property, then replace the switch over component.name with a switch on the enum or simply return component.destinationView so new components won’t silently fall back to placeholders due to typos.Projects/DVDesign/Sources/Components/RadioButton/DVRadioButton.swift (1)
93-109: 💤 Low value
focusable(true)→focusable()정리기본 인자가
true라 명시는 군더더기에요. 작은 정리 한 줄 제안드립니다.♻️ 제안 diff
- .focusable(true) + .focusable()🤖 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 `@Projects/DVDesign/Sources/Components/RadioButton/DVRadioButton.swift` around lines 93 - 109, The call to focusable(true) in DVRadioButton.body is redundant because the default parameter is true; update the view modifier to use focusable() instead to clean up the code (locate the focusable(true) inside the DVRadioButton struct's body where HStack modifiers are applied and replace it with focusable()).
🤖 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 `@Projects/DVDesign/SampleApp/Sources/RadioButtonPreviewView.swift`:
- Line 62: 캡션 텍스트가 잘못 표기되어 있습니다: in the labeled(...) call the caption "spacing
28" should read "spacing 56" to match DVRadioButtonGroup.Size.md.spacing (and
the case in DVRadioButtonGroup.swift). Update the labeled("MD (spacing 28)")
invocation to labeled("MD (spacing 56)") so the preview text accurately reflects
DVRadioButtonGroup.Size.md.spacing.
In `@Projects/DVDesign/Sources/Components/RadioButton/DVRadioButton.swift`:
- Around line 125-154: Replace the module‑level access modifier on the extension
with explicit access on the initializer: remove "public" from the "public
extension DVRadioButton where Label == Text" declaration and mark the
init(_:isSelected:action:) initializer itself as public (i.e., add the public
modifier to that init in DVRadioButton where Label == Text) so the access
control follows the guideline of declaring visibility on each member rather than
on the extension.
In `@Projects/DVDesign/Sources/Components/RadioButton/DVRadioButtonGroup.swift`:
- Around line 120-186: Remove the `public` modifier from the extension
declaration and instead explicitly mark each exposed type/member as public: add
`public` to `struct Item` and its members (`id`, `title`, and `init`), and add
`public` to `enum Size` and its public computed properties (`spacing`,
`minWidth`) so the visibility is explicit on each declaration within
`DVRadioButtonGroup`.
---
Nitpick comments:
In `@Projects/DVDesign/SampleApp/Sources/ContentView.swift`:
- Around line 34-42: The switch in detailView(for:) relies on string matching of
component.name which is fragile; refactor to a type-safe mapping by adding
either an enum (e.g., ComponentType) used by Component or a computed
`@ViewBuilder` property on Component (e.g., var destinationView: some View) and
update detailView(for:) to use that instead of string literals; locate
Component, the detailView(for:) function, RadioButtonPreviewView and
ComponentPlaceholderView and implement the enum or computed view property, then
replace the switch over component.name with a switch on the enum or simply
return component.destinationView so new components won’t silently fall back to
placeholders due to typos.
In `@Projects/DVDesign/SampleApp/Sources/RadioButtonPreviewView.swift`:
- Around line 12-14: The nested enum named Environment conflicts semantically
with SwiftUI's Environment symbol; rename the enum Environment (in
RadioButtonPreviewView.swift) to a less ambiguous name like Env (or RadioEnv)
and update all references (e.g., usages in RadioButtonPreviewView and any
associated initializers or switch/case) to the new identifier, keeping the same
conformance (Hashable) and cases (dev, staging, prod) to avoid behavioral
changes.
In `@Projects/DVDesign/Sources/Components/RadioButton/DVRadioButton.swift`:
- Around line 93-109: The call to focusable(true) in DVRadioButton.body is
redundant because the default parameter is true; update the view modifier to use
focusable() instead to clean up the code (locate the focusable(true) inside the
DVRadioButton struct's body where HStack modifiers are applied and replace it
with focusable()).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cdf90ea8-04af-45c3-98d5-faad73c4113f
📒 Files selected for processing (4)
Projects/DVDesign/SampleApp/Sources/ContentView.swiftProjects/DVDesign/SampleApp/Sources/RadioButtonPreviewView.swiftProjects/DVDesign/Sources/Components/RadioButton/DVRadioButton.swiftProjects/DVDesign/Sources/Components/RadioButton/DVRadioButtonGroup.swift
- public extension 블록 대신 멤버별 접근 제어 명시 - SampleApp MD spacing 캡션 28 → 56 정정
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.
✨ What’s this PR?
📌 관련 이슈 (Related Issue)
🧶 주요 변경 내용 (Summary)
DVFont.swift의 토큰 네이밍 컨벤션 ({category}{Size}/{category}{Size}{Weight})View+DVFont.swift의 line-height 적용 방식 (.lineSpacing+ 수직 padding 보정)Projects/DVDesign/Sources/Components/RadioButton/하위에 라디오 버튼 컴포넌트 2종과 SampleApp 프리뷰 추가.컴포넌트
DVRadioButton<Label>— 16pt 원형 인디케이터 + 라벨, Default/Selected 2상태DVRadioButton("Dev", isSelected:, action:)→DVFont.bodyMD+DVColor.gray900자동 적용@ViewBuilder label클로저로 아이콘/혼합 콘텐츠 지원DVRadioButtonGroup<Value: Hashable>— 가로 정렬, 단일 선택 바인딩Size변형:xs(spacing 8 / minWidth 180),sm(20 / 330),md(56 / 380)Item(value:title:)Identifiable nested struct로 옵션 정의macOS 인터랙션
NSMatrix규약, wrap 없음), Space 활성화.focusEffectDisabled()), hover 효과 미적용 — nativeNSButton라디오 외관과 일치.isButton+isSelected시.isSelected트레이트문서화
DVColor/vaultGreen,DVFont/bodyMD)로 크로스 레퍼런스SampleApp
RadioButtonPreviewView신설 — Default/Selected/Interactive + XS/SM/MD 6 케이스ContentView라우팅: 도연 섹션의DVRadioButton/DVRadioButtonGroup항목 → 실제 프리뷰 화면📸 스크린샷 (Optional)
2026-05-21.11.20.58.mov
🧪 테스트 / 검증 내역
tuist generate정상 완료xcodebuild -scheme DVDesign빌드 성공xcodebuild -scheme DVDesignSampleApp빌드 성공DVRadioButton/DVRadioButtonGroup진입 시 인터랙션(마우스 클릭/Tab/화살표 키/Space) 정상 동작💬 기타 공유 사항 & 🙇🏻♀️ 리뷰 가이드
DVRadioButton.swiftButton없이HStack+.onTapGesture+.onKeyPress(.space)조합으로 hit target을 텍스트 영역까지 일관되게 확장한 부분DVRadioButtonGroup.swift@FocusState<Value?>+.onKeyPress(.leftArrow/.rightArrow)기반 화살표 키 네비게이션의 selection/focus 동기화 흐름(moveSelection(by:))Summary by CodeRabbit