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

Add ability to load dotLottie via LottieAnimationView #2410

Closed

Conversation

thevoiceless
Copy link

Relevant issue: #2409

Adds a setAnimationFromDotLottie() method to LottieAnimationView that accepts a ZipInputStream.

* <p>
* Auto-closes the stream.
*/
public void setAnimationFromDotLottie(ZipInputStream stream, @Nullable String cacheKey) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
public void setAnimationFromDotLottie(ZipInputStream stream, @Nullable String cacheKey) {
public void setAnimation(ZipInputStream stream, @Nullable String cacheKey) {

You can give this the same name. The method overloading specificity is enough here.

It's worth noting that dotLottie aren't the only zip files Lottie supports. You can zip any animations or animations + images so the naming/docs should not be dotLottie specific.

I was on a plane yesterday and actually drafted up a similar PR. I'm happy to land your contribution since you got it first, however.

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense! I don't have a strong opinion on which PR we land. Maybe yours, since I like your docs more 😄

Copy link
Collaborator

@gpeal gpeal Nov 3, 2023

Choose a reason for hiding this comment

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

Okay, up to you. Feel free to update it or close it and then I'll land mine.

CI usually runs fine from forks, not sure why the token is missing for this PR.

@thevoiceless
Copy link
Author

Closing in favor of #2411

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.

None yet

2 participants