[#12] WKWebView에 해당하는 Representable을 구현한다#13
Merged
indextrown merged 9 commits intomainfrom Apr 9, 2026
Merged
Conversation
Contributor
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough새로운 SwiftUI 래퍼 컴포넌트 Sequence Diagram(s)sequenceDiagram
participant SwiftUI as "SwiftUI View"
participant SKWebView as "SKWebView\n(UIViewRepresentable)"
participant WKWebView as "WKWebView"
participant Coordinator as "Coordinator\n(WKNavigationDelegate)"
participant Refresh as "UIRefreshControl"
SwiftUI->>SKWebView: makeUIView(context)
activate SKWebView
SKWebView->>WKWebView: init WKWebView()
SKWebView->>Coordinator: makeCoordinator()
SKWebView->>WKWebView: set navigationDelegate = Coordinator
alt isRefreshEnabled == true
SKWebView->>Refresh: create UIRefreshControl (title/colors/scale)
SKWebView->>Refresh: addTarget(Coordinator.handleRefresh)
WKWebView->>WKWebView: attach refresh control to scrollView
end
deactivate SKWebView
SwiftUI->>SKWebView: updateUIView(uiView, context)
activate SKWebView
SKWebView->>WKWebView: if URL changed -> load(URLRequest)
deactivate SKWebView
User->>Refresh: Pull gesture
activate Refresh
Refresh->>Coordinator: handleRefresh()
Coordinator->>WKWebView: reload()
WKWebView->>Coordinator: didFinish / didFail...
Coordinator->>Refresh: endRefreshing()
deactivate Refresh
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 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)
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
🧹 Nitpick comments (7)
Examples/SampleApp/SampleApp/View/Wrapper/0. WrapperListView/WrapperListView.swift (1)
14-18: 빈 문자열 섹션 헤더는 제거하는 편이 좋습니다.
Section("")대신 헤더 없는Section을 사용하면 불필요한 여백/의미 없는 헤더를 줄일 수 있습니다.제안 코드
- Section("") { + Section { NavigationLink("SKWebView") { SampleSKWebView() } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Examples/SampleApp/SampleApp/View/Wrapper/0`. WrapperListView/WrapperListView.swift around lines 14 - 18, The Section in WrapperListView.swift currently uses an explicit empty-string header (Section("")) which creates unnecessary header space; change it to a headerless Section by removing the "" so that the NavigationLink("SKWebView") { SampleSKWebView() } is wrapped in Section { ... } (i.e., use Section without a string header) to eliminate the empty header spacing.Examples/SampleApp/SampleApp/View/Wrapper/1. SKWebView/SampleSKWebView.swift (1)
13-13: 데모 URL은 상수/주입 형태로 분리해두는 것을 권장합니다.하드코딩 URL은 샘플 변경 시 수정 지점이 늘어납니다.
private let demoURL또는 init 주입으로 분리하면 데모 재사용성과 유지보수성이 좋아집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Examples/SampleApp/SampleApp/View/Wrapper/1`. SKWebView/SampleSKWebView.swift at line 13, The SKWebView instance is created with a hardcoded URL (SKWebView(url: "https://www.naver.com")); change this to use a configurable value by adding a private let demoURL in SampleSKWebView or accept the URL via the SampleSKWebView initializer and pass that into SKWebView(url:), so the demo URL is not hardcoded and can be reused/tested/changed easily.Examples/SampleApp/SampleApp/View/Native/0. NativeListView/NativeListView.swift (1)
12-13: 샘플 탭 목적이 드러나도록 플레이스홀더 텍스트는 교체를 권장합니다.현재 문자열은 Xcode 기본 템플릿 느낌이 강해서, 릴리즈 전에 탭 목적(예: Native 컴포넌트 목록)으로 바꾸면 데모 완성도가 더 좋아집니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Examples/SampleApp/SampleApp/View/Native/0`. NativeListView/NativeListView.swift around lines 12 - 13, The placeholder "Hello, World!" in NativeListView.swift should be replaced with a descriptive label that reflects the tab's purpose (e.g., "Native Components", "Native List", or "Native Component Examples"); locate the Text(...) inside the NativeListView body and update the string to a meaningful placeholder so the sample tab clearly indicates it shows native components or a component list.Tests/SwiftUI-KitTests/SKWebViewTests.swift (1)
53-67: 체이닝 테스트에 ‘원본 불변성’ 검증도 추가해 주세요.값 복사 기반 modifier라면 수정 후에도 원본 인스턴스가 변하지 않는지 함께 검증하는 것이 API 신뢰성에 도움이 됩니다.
As per coding guidelines "2. 구현이 아니라 '사용 결과'를 검증하는 테스트인지 검토해주세요."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Tests/SwiftUI-KitTests/SKWebViewTests.swift` around lines 53 - 67, Add an immutability check by capturing the original SKWebView instance and/or its property snapshot before applying modifiers, then assert those captured values remain unchanged after creating sut via chaining in modifiers_chain_appliesAllConfigurations; specifically, create let original = SKWebView(url: "https://www.apple.com") (or capture originalProps = (isRefreshEnabled: original.isRefreshEnabled, refreshTitle: original.refreshTitle, refreshTextColor: original.refreshTextColor, refreshIndicatorColor: original.refreshIndicatorColor, refreshIndicatorScale: original.refreshIndicatorScale)), then build sut using .refreshable(), .refreshTitle(...), etc., and add expectations that original (or originalProps) still equals the captured values while sut reflects the new values.Sources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView+Modifier.swift (1)
24-28:refreshable()은 SwiftUI의.refreshable(action:)비동기 핸들러와 혼동될 여지가 있습니다.SwiftUI의
.refreshable(action:)은 async 클로저 파라미터를 받아 비동기 새로고침 작업을 처리하는 반면, 현재 메서드는 단순히 Pull to Refresh UI 활성화 플래그를 설정합니다.라이브러리의 다른 configuration 메서드들(
.refreshTitle(),.refreshIndicatorColor())과 일관성을 맞추고, API 사용자의 의도를 더 명확하게 전달하기 위해 다음과 같은 이름 검토를 추천합니다:
enablePullToRefresh()— 기능 활성화를 직관적으로 표현pullToRefreshEnabled()— 프로퍼티명을 직접 반영하여 설정 의도를 분명하게🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView`+Modifier.swift around lines 24 - 28, The method refreshable() is confusingly named because it conflicts with SwiftUI's async .refreshable(action:) — rename it to a clearer name (choose either enablePullToRefresh() or pullToRefreshEnabled()) and update the implementation to set isRefreshEnabled as before; also rename any related API references like refreshTitle(), refreshIndicatorColor() and all call sites, tests, and public docs to use the new name so the library API remains consistent and unambiguous.Sources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView.swift (2)
28-46: 깔끔한 초기화 설계입니다! 👍필수 파라미터를
url만으로 최소화하고 나머지는 modifier로 설정하도록 한 점이 SwiftUI 스타일에 잘 맞습니다.다만
refreshIndicatorScale의 기본값0.7이 왜 선택되었는지 주석이나 상수명으로 의도를 명시하면 좋겠습니다.+ private static let defaultRefreshIndicatorScale: CGFloat = 0.7 + public init(url: String) { self.url = url self.refreshTitle = "" self.refreshTextColor = .label self.refreshIndicatorColor = .label - self.refreshIndicatorScale = 0.7 + self.refreshIndicatorScale = Self.defaultRefreshIndicatorScale self.isRefreshEnabled = false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView.swift` around lines 28 - 46, The default 0.7 for refreshIndicatorScale in SKWebView is a magic number—declare a descriptive static constant (e.g., SKWebView.defaultRefreshIndicatorScale) or add a one-line comment explaining the rationale, then replace the hardcoded 0.7 in the public init with that constant; update any docs/comments for SKWebView and the refreshIndicatorScale property so future readers understand why that scale was chosen.
112-114: URL 비교 로직에 엣지 케이스가 있습니다.현재
absoluteString비교 방식은 다음 상황에서 예상치 못한 동작을 유발할 수 있습니다:
첫 로드 시 문제:
makeUIView직후uiView.url은nil이므로 정상 동작하지만, 로드 중 상태에서 URL이 변경되면 중복 요청 가능성이 있습니다.URL 정규화 차이:
"https://example.com"vs"https://example.com/"같은 케이스에서 다른 URL로 인식될 수 있습니다.func updateUIView( _ uiView: WKWebView, context: Context ) { guard let requestURL = URL(string: url) else { return } - guard uiView.url?.absoluteString != requestURL.absoluteString else { return } + // host와 path를 비교하여 정규화 차이 무시 + guard uiView.url?.host != requestURL.host || + uiView.url?.path != requestURL.path || + uiView.url?.query != requestURL.query else { return } uiView.load(URLRequest(url: requestURL)) }또한, 잘못된 URL에 대해 silent fail하는 것보다 라이브러리 사용자에게 피드백을 제공하는 것이 디버깅에 도움이 됩니다.
os.Logger나 completion handler를 고려해 보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView.swift` around lines 112 - 114, The current URL-check in SKWebView (the update logic that uses URL(string: url) and compares uiView.url?.absoluteString) should be hardened: parse the incoming string into a URLComponents/standardized URL and compare normalized URLs (e.g., use URL.standardized or compare scheme+host+path+query via URLComponents) instead of raw absoluteString to avoid differences like trailing slashes; also handle the case where uiView.url is nil or uiView.isLoading by allowing a load when the current webView URL is nil or when the normalized new URL differs, and surface invalid URL parsing via an os.Logger or the provided completion handler so failures are not silent (refer to SKWebView, updateUIView and the URL loading gate in your diff).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Examples/SampleApp/SampleApp.xcodeproj/project.pbxproj`:
- Line 273: The sample app's deployment target (IPHONEOS_DEPLOYMENT_TARGET =
16.0) and the package manifest platform declaration are inconsistent; either
lower the app to iOS 15.0 or raise the package/readme to require iOS 16.0. Fix
by (A) changing IPHONEOS_DEPLOYMENT_TARGET to 15.0 in the Xcode project and
update the package manifest platforms entry (e.g., platforms: [.iOS(.v15)]) so
they match, or (B) update the package manifest platforms to .v16 and add a short
note in README explaining why iOS 16.0 is required; ensure you update the same
symbol names (IPHONEOS_DEPLOYMENT_TARGET and the Package.swift platforms
declaration) and run a build to verify.
In `@Sources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView.swift`:
- Around line 65-97: The refresh control is only configured in makeUIView so
SwiftUI state updates (refreshTitle, refreshTextColor, refreshIndicatorColor,
refreshIndicatorScale, isRefreshEnabled) are not applied; modify updateUIView to
(1) add or remove the UIRefreshControl from webView.scrollView when
isRefreshEnabled changes, (2) update the existing
context.coordinator.refreshControl's attributedTitle, tintColor and transform to
reflect refreshTitle, refreshTextColor, refreshIndicatorColor and
refreshIndicatorScale, and (3) ensure context.coordinator.webView is kept in
sync with the passed webView; use the existing Coordinator,
webView.scrollView.refreshControl and isRefreshEnabled symbols to locate and
update the logic.
In `@Sources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView`+Modifier.swift:
- Around line 105-108: The setter method refreshIndicatorScale(_ scale: CGFloat)
on SKWebView+Modifier should validate the incoming scale to prevent
zero/negative values; update the method that creates a copy (variable copy) and
sets copy.refreshIndicatorScale to either clamp the value (e.g., use
max(minimum, scale)) or assert the contract with precondition(scale > 0) before
assigning, so the chainable API remains predictable and the refresh indicator
cannot be set to invalid sizes.
- Around line 64-68: The public modifier refreshTextColor currently accepts
UIColor but should expose a SwiftUI-friendly Color API: change or add an
overload func refreshTextColor(_ color: Color) -> Self that converts Color to
UIColor for internal storage (assigning to copy.refreshTextColor =
UIColor(color)); keep or add a UIColor overload for UIKit-only use under `#if`
canImport(UIKit) if needed, and apply the same pattern to refreshIndicatorColor
so the public API surface uses Color while internals remain UIColor.
In `@Tests/SwiftUI-KitTests/SKWebViewTests.swift`:
- Around line 9-67: Tests currently assert internal properties
(isRefreshEnabled, refreshTitle, refreshTextColor, refreshIndicatorColor,
refreshIndicatorScale) via `@testable`; change them to observe public behavior by
instantiating SKWebView, calling the modifier methods (.refreshable(),
.refreshTitle(...), .refreshTextColor(...), .refreshIndicatorColor(...),
.refreshIndicatorScale(...)), then drive makeUIView() (or the
UIViewRepresentable lifecycle) and assert the resulting UIRefreshControl exists
on the created UIView and its public properties reflect the inputs (e.g.,
UIRefreshControl.tintColor, UIRefreshControl.attributedTitle,
UIRefreshControl.transform or layer.scale) rather than inspecting internal
stored properties on SKWebView; update SKWebViewTests methods to construct the
view, call makeUIView/makeUIView(context:) or snapshot the view hierarchy, find
the UIRefreshControl, and assert its tintColor/attributedTitle/transform match
the modifier values.
---
Nitpick comments:
In `@Examples/SampleApp/SampleApp/View/Native/0`.
NativeListView/NativeListView.swift:
- Around line 12-13: The placeholder "Hello, World!" in NativeListView.swift
should be replaced with a descriptive label that reflects the tab's purpose
(e.g., "Native Components", "Native List", or "Native Component Examples");
locate the Text(...) inside the NativeListView body and update the string to a
meaningful placeholder so the sample tab clearly indicates it shows native
components or a component list.
In `@Examples/SampleApp/SampleApp/View/Wrapper/0`.
WrapperListView/WrapperListView.swift:
- Around line 14-18: The Section in WrapperListView.swift currently uses an
explicit empty-string header (Section("")) which creates unnecessary header
space; change it to a headerless Section by removing the "" so that the
NavigationLink("SKWebView") { SampleSKWebView() } is wrapped in Section { ... }
(i.e., use Section without a string header) to eliminate the empty header
spacing.
In `@Examples/SampleApp/SampleApp/View/Wrapper/1`.
SKWebView/SampleSKWebView.swift:
- Line 13: The SKWebView instance is created with a hardcoded URL
(SKWebView(url: "https://www.naver.com")); change this to use a configurable
value by adding a private let demoURL in SampleSKWebView or accept the URL via
the SampleSKWebView initializer and pass that into SKWebView(url:), so the demo
URL is not hardcoded and can be reused/tested/changed easily.
In `@Sources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView.swift`:
- Around line 28-46: The default 0.7 for refreshIndicatorScale in SKWebView is a
magic number—declare a descriptive static constant (e.g.,
SKWebView.defaultRefreshIndicatorScale) or add a one-line comment explaining the
rationale, then replace the hardcoded 0.7 in the public init with that constant;
update any docs/comments for SKWebView and the refreshIndicatorScale property so
future readers understand why that scale was chosen.
- Around line 112-114: The current URL-check in SKWebView (the update logic that
uses URL(string: url) and compares uiView.url?.absoluteString) should be
hardened: parse the incoming string into a URLComponents/standardized URL and
compare normalized URLs (e.g., use URL.standardized or compare
scheme+host+path+query via URLComponents) instead of raw absoluteString to avoid
differences like trailing slashes; also handle the case where uiView.url is nil
or uiView.isLoading by allowing a load when the current webView URL is nil or
when the normalized new URL differs, and surface invalid URL parsing via an
os.Logger or the provided completion handler so failures are not silent (refer
to SKWebView, updateUIView and the URL loading gate in your diff).
In `@Sources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView`+Modifier.swift:
- Around line 24-28: The method refreshable() is confusingly named because it
conflicts with SwiftUI's async .refreshable(action:) — rename it to a clearer
name (choose either enablePullToRefresh() or pullToRefreshEnabled()) and update
the implementation to set isRefreshEnabled as before; also rename any related
API references like refreshTitle(), refreshIndicatorColor() and all call sites,
tests, and public docs to use the new name so the library API remains consistent
and unambiguous.
In `@Tests/SwiftUI-KitTests/SKWebViewTests.swift`:
- Around line 53-67: Add an immutability check by capturing the original
SKWebView instance and/or its property snapshot before applying modifiers, then
assert those captured values remain unchanged after creating sut via chaining in
modifiers_chain_appliesAllConfigurations; specifically, create let original =
SKWebView(url: "https://www.apple.com") (or capture originalProps =
(isRefreshEnabled: original.isRefreshEnabled, refreshTitle:
original.refreshTitle, refreshTextColor: original.refreshTextColor,
refreshIndicatorColor: original.refreshIndicatorColor, refreshIndicatorScale:
original.refreshIndicatorScale)), then build sut using .refreshable(),
.refreshTitle(...), etc., and add expectations that original (or originalProps)
still equals the captured values while sut reflects the new values.
🪄 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
Run ID: a106b019-a59a-4659-bb46-3b904d7b9b0b
📒 Files selected for processing (14)
Examples/SampleApp/SampleApp.xcodeproj/project.pbxprojExamples/SampleApp/SampleApp/App/Assets.xcassets/AccentColor.colorset/Contents.jsonExamples/SampleApp/SampleApp/App/Assets.xcassets/AppIcon.appiconset/Contents.jsonExamples/SampleApp/SampleApp/App/Assets.xcassets/Contents.jsonExamples/SampleApp/SampleApp/App/SampleApp.swiftExamples/SampleApp/SampleApp/App/TabBarView.swiftExamples/SampleApp/SampleApp/ContentView.swiftExamples/SampleApp/SampleApp/View/Native/0. NativeListView/NativeListView.swiftExamples/SampleApp/SampleApp/View/Wrapper/0. WrapperListView/WrapperListView.swiftExamples/SampleApp/SampleApp/View/Wrapper/1. SKWebView/SampleSKWebView.swiftSources/SwiftUI-Kit/Wrapper/SKWebView.swiftSources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView+Modifier.swiftSources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView.swiftTests/SwiftUI-KitTests/SKWebViewTests.swift
💤 Files with no reviewable changes (2)
- Sources/SwiftUI-Kit/Wrapper/SKWebView.swift
- Examples/SampleApp/SampleApp/ContentView.swift
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Sources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView+Modifier.swift (1)
105-108:⚠️ Potential issue | 🟡 Minor
refreshIndicatorScale는 비정상 입력(0/음수)을 방어해 주세요.
Line 105~Line 107은 값을 그대로 대입해, 호출자가 0/음수를 넣으면 인디케이터 표시가 예측 불가능해질 수 있습니다. 최소값 clamp 또는 계약(precondition)으로 입력 유효성을 보장하는 편이 안전합니다.제안 수정안
func refreshIndicatorScale(_ scale: CGFloat) -> Self { var copy = self - copy.refreshIndicatorScale = scale + copy.refreshIndicatorScale = max(0.1, scale) return copy }참고로 같은 맥락으로 경계값 테스트(예: 0, 음수) 1~2개를 추가하면 회귀 방지에 도움이 됩니다.
As per coding guidelines "10. chainable API가 예측 가능하게 동작하는지 검토해주세요."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView`+Modifier.swift around lines 105 - 108, The setter method refreshIndicatorScale(_ scale: CGFloat) currently assigns any value directly; guard against zero/negative inputs by validating and normalizing the value before assignment in the function refreshIndicatorScale: either enforce a precondition (precondition(scale > 0, "refreshIndicatorScale must be > 0")) or clamp the value (e.g., let safeScale = max(scale, CGFloat.leastNonzeroMagnitude) or a chosen positive MIN_SCALE and assign copy.refreshIndicatorScale = safeScale), and add 1–2 unit tests for boundary inputs (0 and a negative value) to ensure predictable behavior.
🤖 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/SwiftUI-Kit/Wrapper/SKWebView/SKWebView`+Modifier.swift:
- Around line 105-108: The setter method refreshIndicatorScale(_ scale: CGFloat)
currently assigns any value directly; guard against zero/negative inputs by
validating and normalizing the value before assignment in the function
refreshIndicatorScale: either enforce a precondition (precondition(scale > 0,
"refreshIndicatorScale must be > 0")) or clamp the value (e.g., let safeScale =
max(scale, CGFloat.leastNonzeroMagnitude) or a chosen positive MIN_SCALE and
assign copy.refreshIndicatorScale = safeScale), and add 1–2 unit tests for
boundary inputs (0 and a negative value) to ensure predictable behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7215323c-2d3a-4da7-9fae-fe0c29ec54ae
📒 Files selected for processing (3)
Examples/SampleApp/SampleApp/View/Wrapper/0. WrapperListView/WrapperListView.swiftExamples/SampleApp/SampleApp/View/Wrapper/1. SKWebView/SampleSKWebView.swiftSources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView+Modifier.swift
✅ Files skipped from review due to trivial changes (2)
- Examples/SampleApp/SampleApp/View/Wrapper/0. WrapperListView/WrapperListView.swift
- Examples/SampleApp/SampleApp/View/Wrapper/1. SKWebView/SampleSKWebView.swift
opficdev
reviewed
Apr 9, 2026
Comment on lines
+65
to
+97
| if isRefreshEnabled { | ||
|
|
||
| // Pull to Refresh 구성 | ||
| let refreshControl = UIRefreshControl() | ||
|
|
||
| // 새로고침 시 보여줄 텍스트 스타일 지정 | ||
| refreshControl.attributedTitle = NSAttributedString( | ||
| string: refreshTitle, | ||
| attributes: [.foregroundColor: refreshTextColor] | ||
| ) | ||
|
|
||
| // 인디케이터 색상 지정 | ||
| refreshControl.tintColor = refreshIndicatorColor | ||
|
|
||
| // 인디케이터 크기 조절 | ||
| refreshControl.transform = CGAffineTransform( | ||
| scaleX: refreshIndicatorScale, | ||
| y: refreshIndicatorScale | ||
| ) | ||
|
|
||
| // reload | ||
| refreshControl.addTarget( | ||
| context.coordinator, | ||
| action: #selector(Coordinator.handleRefresh), | ||
| for: .valueChanged | ||
| ) | ||
|
|
||
| context.coordinator.webView = webView | ||
| context.coordinator.refreshControl = refreshControl | ||
|
|
||
| webView.scrollView.refreshControl = refreshControl | ||
| webView.scrollView.bounces = true | ||
| } |
Contributor
There was a problem hiding this comment.
isRefreshEnabled 초기값이 false라서 없어도 될거같은 느낌?
Contributor
Author
There was a problem hiding this comment.
SKWebView(url: ...)
.refreshable() // isRefreshEnabled = true[2가지 고민]
- refreshable() 적용 시에만 UIRefreshControl() 기능을 설정하려는 의도여서 이렇게 적용했어.
if절 없으면 refreshable() 유무 관계 없이 항상 설정될듯? - 고민인건 refreshable()을 적용하지 않고 refresh관련 modifier를 설정하면 아무런 동작이 없기 때문에
관련 modifier마다if !isRefreshEnabled이면 강제로 런타임 에러를 발생시키는게 좋을까?
Contributor
There was a problem hiding this comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Sources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView.swift (1)
111-117:⚠️ Potential issue | 🟠 Major
updateUIView에서 refresh 상태 동기화가 누락되어 상태 변경이 반영되지 않습니다.현재는 URL만 갱신해서,
.refreshText(...),.refreshIndicatorColor(...),.refreshIndicatorScale(...),.refreshable()의 이후 상태 변경이 UIKit 뷰에 반영되지 않습니다.updateUIView에서 refresh control 생성/제거 및 속성 동기화를 함께 처리해 주세요. (UIViewRepresentable의updateUIView계약)♻️ 제안 코드
func updateUIView( _ uiView: WKWebView, context: Context ) { + context.coordinator.webView = uiView + + if isRefreshEnabled { + let refreshControl = uiView.scrollView.refreshControl ?? { + let control = UIRefreshControl() + control.addTarget( + context.coordinator, + action: `#selector`(Coordinator.handleRefresh), + for: .valueChanged + ) + uiView.scrollView.refreshControl = control + uiView.scrollView.bounces = true + return control + }() + + refreshControl.attributedTitle = NSAttributedString( + string: refreshText, + attributes: [.foregroundColor: refreshTextColor] + ) + refreshControl.tintColor = refreshIndicatorColor + refreshControl.transform = CGAffineTransform( + scaleX: refreshIndicatorScale, + y: refreshIndicatorScale + ) + context.coordinator.refreshControl = refreshControl + } else { + uiView.scrollView.refreshControl = nil + context.coordinator.refreshControl = nil + } + guard uiView.url != url else { return } uiView.load(URLRequest(url: url)) }참고: Apple
UIViewRepresentable문서와 WWDC19 Integrating SwiftUI 세션의 make/update 역할 분리를 같이 보면 의도 정렬이 빠릅니다.As per coding guidelines "6. update 단계에서 불필요한 작업이 반복되지 않는지, SwiftUI 상태와 UIKit 동기화가 올바른지 검토해주세요."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView.swift` around lines 111 - 117, The updateUIView implementation in SKWebView currently only reloads when the URL changes and therefore fails to synchronize refresh control state and properties from SwiftUI (e.g., .refreshText, .refreshIndicatorColor, .refreshIndicatorScale, .refreshable()). In updateUIView(_ uiView: WKWebView, context: Context) create or remove the UIRefreshControl as needed to match the current SwiftUI refreshable state, and update its attributedTitle/text, tintColor, and transform/scale to match the corresponding stored properties; ensure you do not recreate the control unnecessarily by checking an existing refreshControl on uiView.scrollView, and wire or remove the target/action used by SKWebView’s refresh handler so the UIKit view always reflects the SwiftUI state.Sources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView+Modifier.swift (1)
105-108:⚠️ Potential issue | 🟡 Minor
refreshIndicatorScale입력값 검증이 필요합니다.0 이하 값이 들어오면 인디케이터가 비정상적으로 보일 수 있습니다. 최소값 clamp 또는 계약(
precondition)을 추가해 API 동작을 안정화해 주세요.🛠️ 제안 코드 (clamp 방식)
func refreshIndicatorScale(_ scale: CGFloat) -> Self { var copy = self - copy.refreshIndicatorScale = scale + copy.refreshIndicatorScale = max(0.1, scale) return copy }As per coding guidelines "10. chainable API가 예측 가능하게 동작하는지 검토해주세요."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView`+Modifier.swift around lines 105 - 108, The setter method refreshIndicatorScale(_:) currently accepts any CGFloat and can produce invalid UI when given <= 0; update the method in SKWebView+Modifier (the refreshIndicatorScale(_:) function that mutates copy.refreshIndicatorScale) to validate the input by either clamping to a safe minimum (e.g., max(scale, someMinValue)) or asserting/preconditioning (precondition(scale > 0, ...)) before assigning to the refreshIndicatorScale property so the chainable API remains predictable and defensive.
🤖 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/SwiftUI-Kit/Wrapper/SKWebView/SKWebView`+Modifier.swift:
- Around line 39-40: 문서 예시에서 사용된 메서드명 `.refreshTitle(...)`이 현재 공개 API
`refreshText`와 불일치하므로 SKWebView 관련 문서 예시를 찾아 `refreshTitle`를 `refreshText`로 교체해
주세요(참조: SKWebView+Modifier.swift의 예시 주석 블록에 등장하는 `.refreshTitle(...)`). SwiftUI
네이밍 패턴을 따르도록 예시 호출과 설명 텍스트도 `refreshText` 시그니처에 맞게 함께 업데이트하고 빌드/문서 주석에서 오타나 오래된
참조가 남아있지 않은지 검토해 주세요.
---
Duplicate comments:
In `@Sources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView.swift`:
- Around line 111-117: The updateUIView implementation in SKWebView currently
only reloads when the URL changes and therefore fails to synchronize refresh
control state and properties from SwiftUI (e.g., .refreshText,
.refreshIndicatorColor, .refreshIndicatorScale, .refreshable()). In
updateUIView(_ uiView: WKWebView, context: Context) create or remove the
UIRefreshControl as needed to match the current SwiftUI refreshable state, and
update its attributedTitle/text, tintColor, and transform/scale to match the
corresponding stored properties; ensure you do not recreate the control
unnecessarily by checking an existing refreshControl on uiView.scrollView, and
wire or remove the target/action used by SKWebView’s refresh handler so the
UIKit view always reflects the SwiftUI state.
In `@Sources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView`+Modifier.swift:
- Around line 105-108: The setter method refreshIndicatorScale(_:) currently
accepts any CGFloat and can produce invalid UI when given <= 0; update the
method in SKWebView+Modifier (the refreshIndicatorScale(_:) function that
mutates copy.refreshIndicatorScale) to validate the input by either clamping to
a safe minimum (e.g., max(scale, someMinValue)) or asserting/preconditioning
(precondition(scale > 0, ...)) before assigning to the refreshIndicatorScale
property so the chainable API remains predictable and defensive.
🪄 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
Run ID: 71a78667-9b27-4100-8de4-d8e5087a5ac4
📒 Files selected for processing (4)
Examples/SampleApp/SampleApp/View/Wrapper/1. SKWebView/SampleSKWebView.swiftSources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView+Modifier.swiftSources/SwiftUI-Kit/Wrapper/SKWebView/SKWebView.swiftTests/SwiftUI-KitTests/SKWebViewTests.swift
✅ Files skipped from review due to trivial changes (2)
- Examples/SampleApp/SampleApp/View/Wrapper/1. SKWebView/SampleSKWebView.swift
- Tests/SwiftUI-KitTests/SKWebViewTests.swift
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.
💡 PR 유형
✏️ 변경 사항
🚨 관련 이슈
🎨 스크린샷
🔥 추가 설명
Fg같은 축약형도 고민했지만, 공개 API에서는 길더라도 의미가 바로 드러나는 명시적인 이름이 더 적합하다고 판단했습니다.refreshTextColor,refreshIndicatorColor,refreshIndicatorScale형태로 정리했습니다.Summary by CodeRabbit
새로운 기능
개선 사항
테스트