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

Expose the read write to FLAnimatedImage associate to the UIImage to allow advanced feature like placeholder #2220

Merged

Conversation

dreampiggy
Copy link
Contributor

@dreampiggy dreampiggy commented Feb 13, 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: #2217

Pull Request Description

In SD 4.3.0. We introduce a way to use associate object to bind FLAnimatedImage to UIImage. This allow user to "store" the FLAnimatedImage into our memory cache and fix some potential issue. At that time I mark this property readonly because I think user should avoid touching this in most cases.

However, some user also argue that they need to set placeholder of a FLAnimatedImage instance. Since FLAnimatedImage is not a UIImage subclass, they can not do this. So maybe we should just open this ability to allow write to that associate type. User now just set a FLAnimatedImage to UIImage placeholder and all things done.

This will not cause API-breaking because readwrite is superset of readonly. This is used for advanced feature and user should have the knowledge of what they are doing.

@codecov-io
Copy link

Codecov Report

Merging #2220 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2220   +/-   ##
=======================================
  Coverage   78.27%   78.27%           
=======================================
  Files          36       36           
  Lines        3664     3664           
  Branches      338      339    +1     
=======================================
  Hits         2868     2868           
  Misses        773      773           
  Partials       23       23

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 a54d1d7...26e77d0. Read the comment docs.

@dreampiggy dreampiggy merged commit 3054a38 into SDWebImage:master Feb 18, 2018
@dreampiggy dreampiggy deleted the improvement_FLAnimatedImage_expose branch February 18, 2018 07:41
@bpoplauschi
Copy link
Member

I agree with this opening of the API

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

5 participants