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

Added ability to playing Gif only once, and set it as default #25

Merged
merged 1 commit into from Mar 17, 2022

Conversation

AlcorSalvador
Copy link
Contributor

Playing the Gif only once was a feature that is disabled in gifwrap due to spec.loops defaulting to 0 even when explicitly defined as null. omggif when it has a null value assigned to loop_count plays the gif once, 1 = twice and so forth.

Playing the Gif only once was a feature that is disabled in gifwrap due to spec.loops defaulting to 0 even when explicitly defined as null. omggif when it has a null value assigned to loop_count plays the gif once, 1 = twice and so forth.
@AlcorSalvador
Copy link
Contributor Author

Understandable, Thank you for reviewing it though.

@jtlapp
Copy link
Collaborator

jtlapp commented Mar 17, 2022

Okay, it looks good to me. Would you mind adding something to templates/README.hbs on the change in behavior, before I publish it? I'm thinking it should go in a new section near the front, maybe titled "Revisions". I'll generate the README.md from this, and it'll pull in the JSdocs.

@jtlapp
Copy link
Collaborator

jtlapp commented Mar 17, 2022

I'm going to bump the version to 0.10.0 for your change.

@jtlapp
Copy link
Collaborator

jtlapp commented Mar 17, 2022

Would the following do the job without breaking backward compatibility?

if (spec.loops === undefined) {
    spec.loops = 0;
}

@jtlapp
Copy link
Collaborator

jtlapp commented Mar 17, 2022

I just noticed that there are ~67,000 repos that depend on gifwrap, so I'm really hesitant to change default behavior. I changed the solution to the one I posted above. You can pass in null to get the behavior you want.

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