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

Option for variable frame rates to download as GIF option #1611

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

rishabhshuklax
Copy link
Member

@rishabhshuklax rishabhshuklax commented Feb 3, 2020

Fixes #743

variable-frames

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with npm run test-all
  • code is in uniquely-named feature branch and has no merge conflicts
  • PR is descriptively titled
  • ask @publiclab/is-reviewers for help, in a comment below
  • Insert-step functionality is working correct as expected.

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Please make sure to get at least two reviews before asking for merging the PR as that would make the PR more reliable on our part
Thanks!

@codecov
Copy link

codecov bot commented Feb 3, 2020

Codecov Report

Merging #1611 into main will increase coverage by 10.29%.
The diff coverage is 63.90%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #1611       +/-   ##
===========================================
+ Coverage   55.11%   65.41%   +10.29%     
===========================================
  Files         117      132       +15     
  Lines        2344     2741      +397     
  Branches      360      438       +78     
===========================================
+ Hits         1292     1793      +501     
+ Misses       1052      948      -104     
Impacted Files Coverage Δ
examples/lib/scopeQuery.js 18.51% <ø> (ø)
src/Modules.js 100.00% <ø> (ø)
src/modules/WebglDistort/Module.js 2.29% <0.00%> (ø)
src/ui/SetInputStep.js 12.90% <0.00%> (-1.39%) ⬇️
src/modules/ColorHalftone/Module.js 3.63% <3.63%> (ø)
examples/lib/defaultHtmlStepUi.js 11.35% <4.08%> (-0.85%) ⬇️
examples/lib/intermediateHtmlStepUi.js 11.11% <5.55%> (+0.94%) ⬆️
examples/lib/insertPreview.js 13.15% <20.00%> (-0.36%) ⬇️
src/util/getImageDimensions.js 20.00% <20.00%> (ø)
src/util/isGif.js 20.00% <20.00%> (ø)
... and 100 more

@rishabhshuklax
Copy link
Member Author

@publiclab/is-reviewers please review this

examples/index.html Outdated Show resolved Hide resolved
examples/index.html Outdated Show resolved Hide resolved
examples/demo.js Outdated Show resolved Hide resolved
@harshkhandeparkar harshkhandeparkar added the almost-complete PRs that are almost done but little more work. label Jul 7, 2020
@gitpod-io
Copy link

gitpod-io bot commented Jul 12, 2020

@harshkhandeparkar harshkhandeparkar added ready-to-merge and removed almost-complete PRs that are almost done but little more work. labels Oct 17, 2020
@harshkhandeparkar harshkhandeparkar requested a review from a team October 17, 2020 14:04
Copy link
Member

@harshkhandeparkar harshkhandeparkar left a comment

Choose a reason for hiding this comment

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

image

  1. The frame duration unit is not shown
  2. We should no allow any frame rate (-ve or too low etc)

@jywarren jywarren added this to the v3.7.0 milestone Oct 28, 2020
@harshkhandeparkar harshkhandeparkar removed this from the v3.7.0 milestone Nov 2, 2020
@harshkhandeparkar
Copy link
Member

image

  1. The frame duration unit is not shown
  2. We should no allow any frame rate (-ve or too low etc)

This could be merged if this is not a problem.

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.

Add GIF speed selector
4 participants