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

Improve animation view #1114

Merged
merged 3 commits into from
Jan 27, 2020
Merged

Conversation

zenangst
Copy link
Contributor

  • Remove removeObserver call in AnimationView.deinit
  • Add guard to animationMovedToWindow() to ensure that state change don't happen to orphan AnimationView's

Reference issue: #1113

According to Apple's documentation, this method invocation should not be needed anymore.
```
If your app targets iOS 9.0 and later or macOS 10.11 and later, you don't need to unregister an observer in its dealloc method.
```
Don't invoke any state updates when both the `window` and the `superview` is `nil`
@zenangst zenangst requested a review from buba447 January 21, 2020 17:52
/// Don't update any state if both the `superview` and `window` is `nil`
guard window != nil && superview != nil else {
return
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use early-return instead of new lines.

guard window != nil && superview != nil else { return }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @emrcftci, pushed another commit fixing the syntax :)

@emrcftci
Copy link
Contributor

It looks good for me PTAL @buba447

@buba447
Copy link
Collaborator

buba447 commented Jan 27, 2020

LGTM! Nice catch.

@buba447 buba447 merged commit 9d6dae0 into airbnb:master Jan 27, 2020
@zenangst zenangst deleted the improve/animation-view branch January 27, 2020 15:48
@zenangst zenangst restored the improve/animation-view branch January 27, 2020 15:49
calda pushed a commit that referenced this pull request Nov 28, 2022
calda pushed a commit that referenced this pull request Dec 1, 2022
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.

3 participants