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 gif feature #360

Merged
merged 21 commits into from
Oct 10, 2018
Merged

added gif feature #360

merged 21 commits into from
Oct 10, 2018

Conversation

adisapphire
Copy link
Member

Fixed #344

@adisapphire
Copy link
Member Author

gif_step

@tech4GT
Copy link
Member

tech4GT commented Oct 1, 2018

@rockstar777 2 things

  • Please make the button say Download as GIF
  • Please make the gif a little slower in the sense that give more time to each image, currently it's very difficult to make sense of the gif because it changes too quickly

@tech4GT
Copy link
Member

tech4GT commented Oct 1, 2018

But great work! Thanks for your pr!😄

@adisapphire
Copy link
Member Author

adisapphire commented Oct 1, 2018

Updates made that are asked for...
please review
thanks

@avsingh999
Copy link
Member

@rockstar777 improve gif button

@adisapphire
Copy link
Member Author

@avsingh999 Can you please explain did'nt get what to improve more. Is that color or shape or margin or something else?

@avsingh999
Copy link
Member

margin

@avsingh999
Copy link
Member

@rockstar777 can you post gif of this pr where I can understand what have you done

@adisapphire
Copy link
Member Author

exm

@avsingh999
Copy link
Member

@rockstar777 Place Download as a gif button along with download and add step It would be better

@adisapphire
Copy link
Member Author

it would look better but we are considering gif for all images of each step.

@jywarren
Copy link
Member

jywarren commented Oct 1, 2018 via email

@adisapphire
Copy link
Member Author

@jywarren agree with that one area for step section and one for download option could look better.

@adisapphire
Copy link
Member Author

adisapphire commented Oct 1, 2018

screenshot 27

@jywarren So what about this?
May I send PR?

@jywarren
Copy link
Member

jywarren commented Oct 1, 2018 via email

@adisapphire
Copy link
Member Author

No not at all, we just has to create some extra attributes like id for gif element.
and for dimension i think 400px wide and 350px could be perfect.
What say @jywarren ?

@adisapphire
Copy link
Member Author

screenshot from 2018-10-01 14-33-16
@jywarren how about this?

@jywarren
Copy link
Member

jywarren commented Oct 1, 2018 via email

@jywarren
Copy link
Member

jywarren commented Oct 1, 2018 via email

@adisapphire
Copy link
Member Author

let me try

@tech4GT
Copy link
Member

tech4GT commented Oct 2, 2018

@jywarren @rockstar777 I was thinking of downloading the gif and not displaying it in the page, do you think this will be better? Also @rockstar777 looks like we have to rebase here.

@adisapphire
Copy link
Member Author

That is a good idea, but we should provide a demo to our users so that he could decide whether he should download or not? just my opinion what do you think @jywarren @tech4GT
@tech4GT , just rebasing my branch at my end.

@avsingh999
Copy link
Member

avsingh999 commented Oct 2, 2018

@rockstar777 please solve conflicts, Thank you

@tech4GT
Copy link
Member

tech4GT commented Oct 2, 2018

Hmm, @rockstar777 maybe we can do like a small eye logo which displays the preview but I don't think that's required since the preview is the sequence itself, we are just exporting it as gif. Nothing new is being generated.

@avsingh999
Copy link
Member

@tech4GT I think @rockstar777 is right if we want to download as gif user will preview for that gif before download if wants any changes in that gif he/she can change

@tech4GT
Copy link
Member

tech4GT commented Oct 6, 2018

@rockstar777 I think you misunderstood me there, the button name was fine, I meant clicking on the gif should download it, like we do for the step images in the sequence. Thanks!

@adisapphire
Copy link
Member Author

adisapphire commented Oct 6, 2018

ok got it @tech4GT but that funtion already works in this PR.:smile:
So now again changing click to download to download as gif
Thanks

@tech4GT
Copy link
Member

tech4GT commented Oct 6, 2018

Great work @rockstar777 I think we are ready to merge here, but first a couple of pointers for you

  • You can always remove your commits(not that it matters in this case), so ideally for rolling back on something in a PR we just undo the last commit.
  • To solve the conflicts in dist files, use git pull in your local branch, the automatic merge will fail and it will prompt you to manually resolve the conflicts. If there are conflicts in any file other than dist/ resolve the issues and save the files. Then use the grunt build command to generate new dist files. commit the changes and push to this pr.
    Please settle the conflicts and then we can merge. Again much thanks for your patience!

@adisapphire
Copy link
Member Author

thanks @tech4GT for your guidance, I was actually facing problem in resolving conflicts.
Thank you.:smile:

…clab#393)

*  Fixes alignment of message in Add step box

* fixes css info id

* updates class selector in demo

* Spacing changes
@jywarren jywarren closed this Oct 8, 2018
@jywarren jywarren reopened this Oct 8, 2018
@ghost ghost assigned jywarren Oct 8, 2018
@ghost ghost added the in-progress label Oct 8, 2018
@jywarren
Copy link
Member

jywarren commented Oct 8, 2018

Almost done here, the checks were not appearing properly so I restarted them. Great teamwork folks. Thanks to everyone!!

@jywarren
Copy link
Member

jywarren commented Oct 8, 2018

Ah, I see! This needs a quick rebasing... Are you able to give it a try? If so I can merge as soon as you're done, or I can help out.

Sorry to introduce the conflicts by merging another PR; sometimes this happens!!!

@adisapphire
Copy link
Member Author

@tech4GT @jywarren finding difficulty in rebasing, please provide some help
Thanks

@tech4GT
Copy link
Member

tech4GT commented Oct 9, 2018

Okay no worries I'll rebase this!

Signed-off-by: Varun Gupta <[email protected]>
@ghost ghost assigned tech4GT Oct 9, 2018
@adisapphire
Copy link
Member Author

Thanks @tech4GT but also please guide me through some tutorial about rebasing
Thank you

@jywarren
Copy link
Member

Awesome, ready to merge then?

Here's some resources on rebasing!!!

https://publiclab.org/wiki/developers#Resources

@jywarren jywarren merged commit 9168c15 into publiclab:main Oct 10, 2018
@ghost ghost removed the in-progress label Oct 10, 2018
@jywarren
Copy link
Member

Fantastic!!! I will publish this soon but would you like to open a pull request to increment or "bump" the version number in accordance w http://semver.org?

This is essentially a new feature so I believe itd be a 0.x.0 type change. Does this make sense?

@adisapphire adisapphire deleted the branch-p1 branch January 23, 2019 04:01
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.

10 participants