Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move the imageLoopCount and isAnimated into UIImage+WebCache file, removed the outdated methods #2152

Merged
merged 10 commits into from Jan 23, 2018

Conversation

dreampiggy
Copy link
Contributor

@dreampiggy dreampiggy commented Jan 5, 2018

New Pull Request Checklist

  • I have read and understood the CONTRIBUTING guide

  • I have read the Documentation

  • I have searched for a similar pull request in the project and found none

  • I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)

  • I have added the required tests to prove the fix/feature I am adding

  • I have updated the documentation (if necessary)

  • I have run the tests and they pass

  • I have run the lint and it passes (pod lib lint)

This merge request fixes / reffers to the following issues: ...

Pull Request Description

Our current UIImage+GIF seems out of date. That isGIF is not related to GIF image format but whether it's animated. So, instead, I think we should use a more common property to let user know current image is animated.

We introduce sd_isAnimated to indicate whether the image instance is animated image or not. And also, we previously put those category property anywhere. For example, sd_imageLoopCount is in UIImage+MultiFormat, which should be format related but not image loop count or animated related. So we make it more clear.

Attention, that previous NSImage+WebCache will resolve a dumplicate symbol because UIImage is defined as NSImage on macOS. So UIImage+WebCache should also be use for NSImage. Therefore, the NSImage+WebCache.h file should remove that declaration, we seperate the helper method to NSImage+Addtions which provide convenience method for cross-platform code.

And also, this PR changes some not really good API for Swift with the NS_SWIFT_NAME and change some method declaration to property, this can make that more easy to use on Swift.

@dreampiggy dreampiggy force-pushed the little_refactor_and_deprecate branch from 21ee138 to 41cd76b Compare January 5, 2018 05:58
@dreampiggy dreampiggy added this to the 4.3.0 milestone Jan 5, 2018
@dreampiggy dreampiggy force-pushed the little_refactor_and_deprecate branch from 41cd76b to daecb24 Compare January 5, 2018 07:19
@codecov-io
Copy link

codecov-io commented Jan 5, 2018

Codecov Report

Merging #2152 into 5.x will increase coverage by 0.03%.
The diff coverage is 75.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##              5.x    #2152      +/-   ##
==========================================
+ Coverage   80.53%   80.57%   +0.03%     
==========================================
  Files          36       37       +1     
  Lines        3586     3588       +2     
  Branches      320      321       +1     
==========================================
+ Hits         2888     2891       +3     
+ Misses        675      674       -1     
  Partials       23       23
Impacted Files Coverage Δ
SDWebImage/SDWebImageGIFCoder.m 79.43% <ø> (ø) ⬆️
SDWebImage/SDWebImageCompat.h 100% <ø> (ø) ⬆️
SDWebImage/UIImage+WebP.m 0% <ø> (ø) ⬆️
SDWebImage/SDWebImageWebPCoder.m 83.33% <ø> (ø) ⬆️
SDWebImage/UIImageView+WebCache.m 15.38% <ø> (+0.89%) ⬆️
SDWebImage/UIImage+GIF.m 0% <ø> (ø) ⬆️
SDWebImage/UIImage+MultiFormat.m 0% <ø> (-50%) ⬇️
SDWebImage/UIView+WebCache.m 68.26% <100%> (+0.38%) ⬆️
Tests/Tests/SDWebImageDecoderTests.m 100% <100%> (ø) ⬆️
SDWebImage/UIImage+ForceDecode.m 90% <100%> (ø) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55d0822...cde5d5f. Read the comment docs.

@dreampiggy dreampiggy modified the milestones: 4.3.0, 5.0.0 Jan 12, 2018
@dreampiggy
Copy link
Contributor Author

dreampiggy commented Jan 12, 2018

This seems not really useful if we want to make it easier to migrate from 4.x to 5.x. So I decide to directly remove that API instead of a deprecation in 4.3.x and removal in 5.x.

@dreampiggy dreampiggy changed the base branch from master to 5.x January 13, 2018 08:57
@dreampiggy dreampiggy changed the title Move the imageLoopCount and isAnimated into UIImage+WebCache file, marked the outdated methods deprecated Move the imageLoopCount and isAnimated into UIImage+WebCache file, removed the outdated methods Jan 13, 2018
@dreampiggy
Copy link
Contributor Author

dreampiggy commented Jan 13, 2018

Current break change (Need change code)

UIImage+MultiFormat.h

  • sd_imageLoopCount, MOVED to UIImage(WebCache) sd_imageLoopCount

UIImage+ForceDecode.h

  • decodedImageWithImage:, REPLACED with +[UIImage(ForceDecode) sd_decodedImageWithImage:]
  • decodedAndScaledDownImageWithImage: REPLACED with +[UIImage(ForceDecode) sd_decodedAndScaledDownImageWithImage:]

UIImage+GIF.h

  • -[UIImage(GIF) isGIF], REPLACED with -[UIImage(WebCache) sd_isAnimated]

NSImage+WebCache.h

  • RENAMED to NSImage+Addtions.h
  • -[NSImage(WebCache) isGIF], REPLACED with -[UIImage(WebCache) sd_isAnimated]

Already Deprecated and Remove

UIImage+WebP.h

  • -[UIImage(WebP) sd_webpLoopCount], REPLACED with -[UIImage(WebCache) sd_imageLoopCount]

SDWebImageDownloaderOperation.h

  • shouldUseCredentialStorage REMOVED

UIImageView+WebCache.h

  • sd_setImageWithPreviousCachedImageWithURL:placeholderImage:options:progress:completed REMOVED

@@ -37,6 +37,6 @@ typedef NS_ENUM(NSInteger, SDImageFormat) {
@param format Format as SDImageFormat
@return The UTType as CFStringRef
*/
+ (nonnull CFStringRef)sd_UTTypeFromSDImageFormat:(SDImageFormat)format;
+ (nonnull CFStringRef)sd_UTTypeFromSDImageFormat:(SDImageFormat)format CF_RETURNS_NOT_RETAINED;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually follow the GET rules on Core Foundation, mark this to avoid Swift user use a Unmanaged check

@param image The image to be decompressed
@return The decompressed image
*/
+ (nullable UIImage *)sd_decodedImageWithImage:(nullable UIImage *)image;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use category prefix here to avoid name conflict

*/
- (void)sd_setIndicatorStyle:(UIActivityIndicatorViewStyle)style;
@property (nonatomic, assign) UIActivityIndicatorViewStyle sd_activityIndicatorStyle;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These change to make the API more suitable for Swift and Objective-C user. Use method is little wired :)
I'll try to create a new class for indicator. Current design is bad for extension and more flexible

@skyline75489
Copy link
Member

Looks good to me. Cheers for better APIs. 🍻

…use property based API, which more suitable for Swift
@dreampiggy dreampiggy merged commit f4b61b9 into SDWebImage:5.x Jan 23, 2018
@dreampiggy dreampiggy deleted the little_refactor_and_deprecate branch January 23, 2018 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants