[CIS-789,CIS-790] Use custom keys for image caching and Load image thumbnails correctly #1111
Conversation
skorepak
requested review from
b-onc,
VojtaStavik,
dmigach,
nuno-vieira,
evsaev and
DominikBucher12
Codecov Report
@@ Coverage Diff @@
## main #1111 +/- ##
=======================================
Coverage 91.36% 91.36%
=======================================
Files 216 216
Lines 9215 9215
=======================================
Hits 8419 8419
Misses 796 796
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
skorepak
force-pushed
the
CIS-789-Use-custom-keys-for-image-caching
branch
2 times, most recently
from
64c9755
to
8fc7eec
Compare
skorepak
force-pushed
the
CIS-789-Use-custom-keys-for-image-caching
branch
2 times, most recently
from
7a6ba0a
to
cb12be3
Compare
skorepak
changed the title
|
Added function to load thumbnail of preferred size and renamed protocol to ImageCDN. |
skorepak
force-pushed
the
CIS-789-Use-custom-keys-for-image-caching
branch
from
cb12be3
to
f6ea2a4
Compare
| from url: URL?, | ||
| placeholder: UIImage? = nil, | ||
| resizeAutomatically: Bool = true, | ||
| requestThumbnail: Bool = false, |
There was a problem hiding this comment.
TBH this API now feels a bit complex. In the end, it all comes to the question - do you need the image in a specific size? If yes, you should try to get the correct URL and get it.
Maybe there's a way to simplify this?
There was a problem hiding this comment.
I was thinking about some enum with associated value like:
enum Resize {
.resizeAfterDownload
.requestThumbnail(size: CGSize?)
.none
}
But you didn't like adding an extra option for ImageCrop and this would be the same. Adding extra public enum just for this. I would say it's a bit extensive, but not that complex.
I'm open to suggestions on how to simplify this
skorepak
force-pushed
the
CIS-789-Use-custom-keys-for-image-caching
branch
2 times, most recently
from
e88d0db
to
1a0fbd5
Compare
skorepak
force-pushed
the
CIS-789-Use-custom-keys-for-image-caching
branch
from
1a0fbd5
to
5c4f769
Compare
skorepak
force-pushed
the
CIS-789-Use-custom-keys-for-image-caching
branch
from
4788ce0
to
0518c54
Compare
| return size | ||
| } | ||
|
|
||
| private let sizeProvider: () -> CGSize |
There was a problem hiding this comment.
Maybe this should return CGSize? rather than using .zero as the fallback?
| } | ||
|
|
||
| public var identifier: String { | ||
| "com.github.kean/nuke/lateResize" |
There was a problem hiding this comment.
Shouldn't we make this unique every time we create a processor?
There was a problem hiding this comment.
I was trying to figure out if it matters or not. Nuke docs don't say anything and the original Resize is adding just the parameters of the processor. If you create two Resize processors with the same size parameter, they will have the same identifier. Since we have size closure and getting the size is waiting synchronously for the main queue (may be expensive), I decided to remove it.
skorepak
force-pushed
the
CIS-789-Use-custom-keys-for-image-caching
branch
from
0518c54
to
eba4886
Compare
In this PR
Links
https://stream-io.atlassian.net/browse/CIS-789
https://stream-io.atlassian.net/browse/CIS-790