Closed SirArkimedes closed 3 years ago
Hey,
thank you for the suggestion and most important for being ready to contribute back to the project.
From a general purpose perspective, it might be that other information about an image needed for a different case, or access to an image itself (CGImage
object). Then adding more arguments to the closure won't be the best way.
And as you mention yourself, this will be a breaking change, so I prefer to keep a single argument as it is.
The solution, as I see it, is to provide access to the proxy object TransientImageType
, in URLImage
initializer. This will give an alternative of using a bit more complex API to get access to CGImage
object.
public init(url: URL,
options: URLImageOptions = URLImageService.shared.defaultOptions,
empty: @escaping () -> Empty,
inProgress: @escaping (_ progress: Float?) -> InProgress,
failure: @escaping (_ error: Error, _ retry: @escaping () -> Void) -> Failure,
content: @escaping (_ image: Image) -> Content)
public init(url: URL,
options: URLImageOptions = URLImageService.shared.defaultOptions,
empty: @escaping () -> Empty,
inProgress: @escaping (_ progress: Float?) -> InProgress,
failure: @escaping (_ error: Error, _ retry: @escaping () -> Void) -> Failure,
content: @escaping (_ transientImage: TransientImageType) -> Content)
This will also require to add similar changes to URLImage
extension to utilize default values for empty
, inProgress
, and failure
state views.
I will try this change a bit later today, or more likely tomorrow, and see if it's convenient to use.
I added a new ImageInfo
object to keep reference to CGImage
object and related information. And also there are now two initializers:
init(url: URL, options: URLImageOptions, content: @escaping (_ image: Image) -> Content)
init(url: URL, options: URLImageOptions, content: @escaping (_ image: Image, _ info: ImageInfo) -> Content)
Unfortunately approach with adding TransientImageType
initializer is not convenient, because it requires the client code to make the closure type explicit.
Also note that the size in ImageInfo
is the real size. Meaning, if you use maxPixelSize
to limit decoded image size (and it's limited by default) it's not affected by this setting.
The change is currently in master branch.
This is great! Thank you for adding this so quick!
This enhancement is now part of 2.2.0 release. I'm closing the issue. Feel free to reopen if you encounter any issues. Cheers.
Is your feature request related to a problem? Please describe. Not necessarily a problem, but was a use-case that came up with using the library!
On the application that I'm working with, we have a case where we want to know the size of the image and add different view modifiers to it depending upon a few conditions. Though, since the SwiftUI
Image
type doesn't have any size information we can't access it directly through the implementation.Describe the solution you'd like Maybe adding a size parameter to the
Content
callback when the image successfully loads? After adding this myself, it looks like it works as intended!Here's my solution: https://github.com/SirArkimedes/url-image/commit/3314f3b711e375b124e41c9d59ed7b1390eb5285
Describe alternatives you've considered Haven't considered much, but am thinking that it could be added somewhere else so it wouldn't need to be a breaking change to support the new parameter?
Additional context I'd be willing to create a pull request with my change if it seems like an okay solution!
Here's a code sample of the
Content
block: