Kingfisher icon indicating copy to clipboard operation
Kingfisher copied to clipboard

Fix .onFailureImage applying configurations to failure images

Open sagarrai21802 opened this issue 5 months ago • 2 comments

Fix .onFailureImage Configuration Application Issue

Description

The .onFailureImage modifier in SwiftUI's KFImage was applying all image configurations (resizable, aspectRatio, etc.) to the failure image, which could lead to unexpected styling. This fix ensures failure images are displayed without unwanted transformations while maintaining backward compatibility.

Problem

When using .onFailureImage with configurations like .resizable(), .aspectRatio(), etc., the failure image would inherit all these transformations, potentially causing layout issues or unexpected appearance.

Problematic Code:

KFImage.url(URL(string: imageUrl))
    .placeholder { ProgressView() }
    .resizable()           // Applied to failure image too
    .aspectRatio(contentMode: .fit)  // Applied to failure image too
    .clipShape(RoundedRectangle(cornerRadius: 20, style: .continuous))
    .frame(width: 60, height: 60)
    .cornerRadius(10)
    .onFailureImage(someImage)  // Configurations applied here unexpectedly

Solution

Modified the failure handling to display failure images without applying KFImage configurations, providing clean separation between success and failure image rendering.

Solution Code:

KFImage.url(URL(string: imageUrl))
    .placeholder { ProgressView() }
    .resizable()
    .aspectRatio(contentMode: .fit)
    .clipShape(RoundedRectangle(cornerRadius: 20, style: .continuous))
    .frame(width: 60, height: 60)
    .cornerRadius(10)
    .onFailureImage(someImage)  // Now displays without configurations

For styled failure content, use .onFailureView:

.onFailureView {
    Image(systemName: "photo")
        .resizable()
        .aspectRatio(contentMode: .fit)
        .clipShape(RoundedRectangle(cornerRadius: 20, style: .continuous))
        .frame(width: 60, height: 60)
        .cornerRadius(10)
}

Approach

  1. Added separate failure image handling in ImageBinder to avoid configuration application
  2. Modified renderer logic to skip configurations for failure images
  3. Maintained priority order: failureView > failureImage > placeholder
  4. Ensured backward compatibility with existing code

Files Changed

  • Sources/SwiftUI/ImageBinder.swift: Added failureImage and isFailureImage properties, updated failure handling logic
  • Sources/SwiftUI/KFImageRenderer.swift: Modified configuredImage to skip configurations for failure images
  • Demo/Demo/Kingfisher-Demo/SwiftUIViews/LoadingFailureDemo.swift: Updated demo to remove conflicting usage

Testing

  • Verified that .onFailureImage displays failure images without configurations
  • Confirmed .onFailureView still takes precedence when both are set
  • Ensured existing functionality remains intact
  • Updated demo to reflect correct usage patterns

Related Issue

Closes #2321: "How to add an image in case the loading of the url fails? SwiftUI"

Issue Link: https://github.com/onevcat/Kingfisher/issues/2321#issue-2672702086

This addresses the issue where .onFailureImage was applying unwanted configurations to failure images, improving the developer experience for failure state handling in SwiftUI.

sagarrai21802 avatar Nov 26 '25 16:11 sagarrai21802

Hi, @sagarrai21802

Thank you for taking the time to work on this—it’s greatly appreciated.

There are a few blocking issues and let me explain them:

  • The change makes .onFailureImage bypass all existing .configure {} modifiers, so failure fallbacks no longer respect sizing, rendering mode, or other decorators. This breaks a fundamental assumption in current SwiftUI usage and would regress many existing setups that rely on those modifiers for layout.
  • The placeholder layer only checks loadedImage == nil, so when a failure occurs the placeholder view now permanently sits on top of the failure image. That regresses every combination of .onFailureImage with a placeholder or progress view.

Given these points, the change as written would seriously disturb existing apps. In fact, we have a plan to deprecate .onFailureImage altogether (it’s a legacy API from an early, less mature phase), and we encourage migrating to the more flexible onFailureView instead. I will try to work on this soon.

Thank you again for your effort. I will try to close this PR when the deprecation annotation can be added.

onevcat avatar Dec 02 '25 14:12 onevcat

Hi, @sagarrai21802

Thank you for taking the time to work on this—it’s greatly appreciated.

There are a few blocking issues and let me explain them:

  • The change makes .onFailureImage bypass all existing .configure {} modifiers, so failure fallbacks no longer respect sizing, rendering mode, or other decorators. This breaks a fundamental assumption in current SwiftUI usage and would regress many existing setups that rely on those modifiers for layout.
  • The placeholder layer only checks loadedImage == nil, so when a failure occurs the placeholder view now permanently sits on top of the failure image. That regresses every combination of .onFailureImage with a placeholder or progress view.

Given these points, the change as written would seriously disturb existing apps. In fact, we have a plan to deprecate .onFailureImage altogether (it’s a legacy API from an early, less mature phase), and we encourage migrating to the more flexible onFailureView instead. I will try to work on this soon.

Thank you again for your effort. I will try to close this PR when the deprecation annotation can be added.

Hi @onevcat ,

Thank you for the detailed feedback. I understand the issues with .onFailureImage and the placeholder behavior. I’d be happy to help work on the migration to onFailureView or any other related improvements to ensure compatibility with existing setups.

Looking forward to contributing further.

Best, @sagarrai21802

sagarrai21802 avatar Dec 02 '25 20:12 sagarrai21802