From 66a286487277889f8e7ab6597ef605e39c50fa4c Mon Sep 17 00:00:00 2001 From: PJ Fechner Date: Thu, 17 Mar 2022 16:56:15 -0500 Subject: [PATCH 1/7] Fix for UIImage placeholder always showing on error --- Sources/Extensions/ImageView+Kingfisher.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/Extensions/ImageView+Kingfisher.swift b/Sources/Extensions/ImageView+Kingfisher.swift index 79c33c0b9..a45c1538d 100644 --- a/Sources/Extensions/ImageView+Kingfisher.swift +++ b/Sources/Extensions/ImageView+Kingfisher.swift @@ -362,6 +362,7 @@ extension KingfisherWrapper where Base: KFCrossPlatformImageView { if let image = options.onFailureImage { self.base.image = image } + mutatingSelf.placeholder = nil completionHandler?(result) } } From 3e7ff48a7419afec15ff916fe2e9d0b185a0dfab Mon Sep 17 00:00:00 2001 From: PJ Fechner Date: Thu, 17 Mar 2022 17:21:40 -0500 Subject: [PATCH 2/7] Order tweek --- Sources/Extensions/ImageView+Kingfisher.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Extensions/ImageView+Kingfisher.swift b/Sources/Extensions/ImageView+Kingfisher.swift index a45c1538d..5dbaa2ce0 100644 --- a/Sources/Extensions/ImageView+Kingfisher.swift +++ b/Sources/Extensions/ImageView+Kingfisher.swift @@ -359,10 +359,10 @@ extension KingfisherWrapper where Base: KFCrossPlatformImageView { } case .failure: + mutatingSelf.placeholder = nil if let image = options.onFailureImage { self.base.image = image } - mutatingSelf.placeholder = nil completionHandler?(result) } } From f73bbf8615a2d2008ce9e886a1694a814a1f19fc Mon Sep 17 00:00:00 2001 From: PJ Fechner Date: Tue, 22 Mar 2022 09:45:24 -0500 Subject: [PATCH 3/7] Potential Fix for SwiftUI placeholder issue --- Sources/SwiftUI/KFImageRenderer.swift | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Sources/SwiftUI/KFImageRenderer.swift b/Sources/SwiftUI/KFImageRenderer.swift index 90e684623..3064b241a 100644 --- a/Sources/SwiftUI/KFImageRenderer.swift +++ b/Sources/SwiftUI/KFImageRenderer.swift @@ -38,11 +38,6 @@ struct KFImageRenderer : View where HoldingView: KFImageHoldingView var body: some View { ZStack { - context.configurations - .reduce(HoldingView.created(from: binder.loadedImage, context: context)) { - current, config in config(current) - } - .opacity(binder.loaded ? 1.0 : 0.0) if binder.loadedImage == nil { Group { if let placeholder = context.placeholder, let view = placeholder(binder.progress) { @@ -68,6 +63,11 @@ struct KFImageRenderer : View where HoldingView: KFImageHoldingView } } } + context.configurations + .reduce(HoldingView.created(from: binder.loadedImage, context: context)) { + current, config in config(current) + } + .opacity(binder.loaded ? 1.0 : 0.0) } } } From 37255f333cb92b3e09cd5c38ab99a65319dbe614 Mon Sep 17 00:00:00 2001 From: PJ Fechner Date: Tue, 22 Mar 2022 09:58:45 -0500 Subject: [PATCH 4/7] SwiftUI fix attempt via opacity --- Sources/SwiftUI/KFImageRenderer.swift | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Sources/SwiftUI/KFImageRenderer.swift b/Sources/SwiftUI/KFImageRenderer.swift index 3064b241a..d614db66a 100644 --- a/Sources/SwiftUI/KFImageRenderer.swift +++ b/Sources/SwiftUI/KFImageRenderer.swift @@ -38,6 +38,11 @@ struct KFImageRenderer : View where HoldingView: KFImageHoldingView var body: some View { ZStack { + context.configurations + .reduce(HoldingView.created(from: binder.loadedImage, context: context)) { + current, config in config(current) + } + .opacity(binder.loaded ? 1.0 : 0.0) if binder.loadedImage == nil { Group { if let placeholder = context.placeholder, let view = placeholder(binder.progress) { @@ -46,6 +51,7 @@ struct KFImageRenderer : View where HoldingView: KFImageHoldingView Color.clear } } + .opacity(binder.loaded ? 0.0 : 1.0) .onAppear { [weak binder = self.binder] in guard let binder = binder else { return @@ -63,11 +69,6 @@ struct KFImageRenderer : View where HoldingView: KFImageHoldingView } } } - context.configurations - .reduce(HoldingView.created(from: binder.loadedImage, context: context)) { - current, config in config(current) - } - .opacity(binder.loaded ? 1.0 : 0.0) } } } From 0676f024b3076f9ca1205f94b845ae21df129c88 Mon Sep 17 00:00:00 2001 From: PJ Fechner Date: Tue, 22 Mar 2022 10:12:08 -0500 Subject: [PATCH 5/7] Potential fix via loading tweak --- Sources/SwiftUI/ImageBinder.swift | 4 ++++ Sources/SwiftUI/KFImageRenderer.swift | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Sources/SwiftUI/ImageBinder.swift b/Sources/SwiftUI/ImageBinder.swift index 7dd9730b5..4fe20cfc4 100644 --- a/Sources/SwiftUI/ImageBinder.swift +++ b/Sources/SwiftUI/ImageBinder.swift @@ -54,6 +54,10 @@ extension KFImage { guard let source = context.source else { CallbackQueue.mainCurrentOrAsync.execute { context.onFailureDelegate.call(KingfisherError.imageSettingError(reason: .emptySource)) + if let image = context.options.onFailureImage { + self.loadedImage = image + } + self.loaded = true } return } diff --git a/Sources/SwiftUI/KFImageRenderer.swift b/Sources/SwiftUI/KFImageRenderer.swift index d614db66a..90e684623 100644 --- a/Sources/SwiftUI/KFImageRenderer.swift +++ b/Sources/SwiftUI/KFImageRenderer.swift @@ -51,7 +51,6 @@ struct KFImageRenderer : View where HoldingView: KFImageHoldingView Color.clear } } - .opacity(binder.loaded ? 0.0 : 1.0) .onAppear { [weak binder = self.binder] in guard let binder = binder else { return From e79cd54ca0336f0220541f8edb9903b23dc77ffa Mon Sep 17 00:00:00 2001 From: PJ Fechner Date: Tue, 22 Mar 2022 10:22:03 -0500 Subject: [PATCH 6/7] State update tweak --- Sources/SwiftUI/ImageBinder.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/SwiftUI/ImageBinder.swift b/Sources/SwiftUI/ImageBinder.swift index 4fe20cfc4..b0eec2d9e 100644 --- a/Sources/SwiftUI/ImageBinder.swift +++ b/Sources/SwiftUI/ImageBinder.swift @@ -57,6 +57,7 @@ extension KFImage { if let image = context.options.onFailureImage { self.loadedImage = image } + self.loading = false self.loaded = true } return From a4ea6737df7d2f45692c455b9a94816f67685eeb Mon Sep 17 00:00:00 2001 From: PJ Fechner Date: Tue, 22 Mar 2022 10:51:30 -0500 Subject: [PATCH 7/7] Fixing and adding Unit Tests --- Sources/Extensions/ImageView+Kingfisher.swift | 2 +- .../ImageViewExtensionTests.swift | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/Sources/Extensions/ImageView+Kingfisher.swift b/Sources/Extensions/ImageView+Kingfisher.swift index 5dbaa2ce0..6ead7d534 100644 --- a/Sources/Extensions/ImageView+Kingfisher.swift +++ b/Sources/Extensions/ImageView+Kingfisher.swift @@ -359,8 +359,8 @@ extension KingfisherWrapper where Base: KFCrossPlatformImageView { } case .failure: - mutatingSelf.placeholder = nil if let image = options.onFailureImage { + mutatingSelf.placeholder = nil self.base.image = image } completionHandler?(result) diff --git a/Tests/KingfisherTests/ImageViewExtensionTests.swift b/Tests/KingfisherTests/ImageViewExtensionTests.swift index 8921addbe..313f8d713 100644 --- a/Tests/KingfisherTests/ImageViewExtensionTests.swift +++ b/Tests/KingfisherTests/ImageViewExtensionTests.swift @@ -569,6 +569,28 @@ class ImageViewExtensionTests: XCTestCase { waitForExpectations(timeout: 3, handler: nil) } + + func testSettingNonWorkingImageWithCustomizePlaceholderAndFailureImage() { + let exp = expectation(description: #function) + let url = testURLs[0] + + stub(url, errorCode: 404) + + let view = KFCrossPlatformView() + + imageView.kf.setImage( + with: url, + placeholder: view, + options: [.onFailureImage(testImage)]) + { + result in + XCTAssertEqual(self.imageView.image, testImage) + XCTAssertFalse(self.imageView.subviews.contains(view)) + exp.fulfill() + } + + waitForExpectations(timeout: 3, handler: nil) + } func testSettingNonWorkingImageWithFailureImage() { let exp = expectation(description: #function)