Skip to content

Add pin animated image view - #453

Merged
garrettmoon merged 12 commits into
masterfrom
addPINAnimatedImageView
Apr 25, 2018
Merged

Add pin animated image view#453
garrettmoon merged 12 commits into
masterfrom
addPINAnimatedImageView

Conversation

@garrettmoon

Copy link
Copy Markdown
Collaborator
  • Add support for GIFs (natively) and animated WebP

@ghost

ghost commented Apr 24, 2018

Copy link
Copy Markdown

🚫 CI failed with log

@Adlai-Holler Adlai-Holler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just some general nits and qs, nothing blocking 👍

override func viewWillAppear(_ animated: Bool) {
super.viewWillAppear(animated)

imageView.pin_setImage(from: URL(string: "https://i.pinimg.com/originals/f5/23/f1/f523f141646b613f78566ba964208990.gif"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wonder why this example uses viewWillAppear: and the other uses viewDidLoad?

</subviews>
<animations/>
<color key="backgroundColor" white="1" alpha="1" colorSpace="calibratedWhite"/>
<color key="backgroundColor" red="1" green="1" blue="1" alpha="1" colorSpace="custom" customColorSpace="sRGB"/>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These storyboards seem nice 😍

- (void)viewDidAppear:(BOOL)animated
{
[super viewDidAppear:animated];
[self.animatedImageView pin_setImageFromURL:[NSURL URLWithString:@"https://i.pinimg.com/originals/f5/23/f1/f523f141646b613f78566ba964208990.gif"]];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Now we're doing viewDidAppear: wat

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

😁 I was just copying the other view controllers in each project. This is what happens when each example project is built by someone else…

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

But did appear seems decidedly wrong and so does load…

if (_animatedImage) {
return _frameImage;
} else if ([super image]) {
return (CGImageRef)CFAutorelease(CFRetain([[super image] CGImage]));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: worth eliminating the multiple [super image] calls?

{
PINAssertMain();
_frameInterval = frameInterval;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

keanuwhoa.jpg

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Gotta have that assert :)

@ghost

ghost commented Apr 25, 2018

Copy link
Copy Markdown
1 Warning
⚠️ This is a big PR, please consider splitting it up to ease code review.

Generated by 🚫 Danger

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants