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 the resize quick button #782

Merged
merged 7 commits into from
Feb 18, 2019
Merged

Added the resize quick button #782

merged 7 commits into from
Feb 18, 2019

Conversation

harshithpabbati
Copy link

Fixes #499

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 test
  • code is in uniquely-named feature branch and has no merge conflicts
  • PR is descriptively titled
  • ask @publiclab/reviewers and @publiclab/is-reviewers for help, in a comment below

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

Thanks!

@harshithpabbati
Copy link
Author

@publiclab/is-reviewers

@harshithpabbati
Copy link
Author

@harshkhandeparkar checkout this..

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.

Please tell me the name of the fontawesome icon you are using.

<footer>
<hr style="margin:20px;"><center><a class="color:grey;" id="clear-cache">Clear offline cache</a></center>
<div class="row">
Copy link
Member

Choose a reason for hiding this comment

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

I think you have made this branch from your footer branch and not from main. Please remove these changes(footer)

@@ -86,6 +86,12 @@ <h1><a href="/" target='_blank' class="name-header">Image Sequencer</a></h1>
</div>
<p class="info">Select a new module to add to your sequence.</p>
<div class="row center-align radio-group">
<div>
<div class="radio" data-value="resize">
<i class="icon-resize-full fa-4x i-over"></i>
Copy link
Member

Choose a reason for hiding this comment

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

You have to add the classes fa fa-4x i-over and fa-name-of-icon. Do you have any other difficulty?

@@ -10,6 +10,12 @@ function IntermediateHtmlStepUi(_sequencer, step, options) {
<div class="panel-body">\
<p class="info">Select a new module to add to your sequence.</p>\
<div class="row center-align radio-group">\
<div>\
<div class="radio" data-value="resize">\
<i class="fa icon-resize-full fa-4x"></i>\
Copy link
Member

Choose a reason for hiding this comment

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

Also here.

@harshkhandeparkar
Copy link
Member

Also you have to build the dist riles by first running grunt browserify and then grunt uglify and please commit those files.

@harshkhandeparkar harshkhandeparkar changed the title Added the resize module Added the resize quick button Feb 16, 2019
@Divy123
Copy link
Member

Divy123 commented Feb 16, 2019

@harshithpabbati please provide a screenshot or gif .
Also as said by HarshKhandeparkar please checkout main branch then make a new branch suppose add-resize and make the said changes there.
Please keep this rule with you new PR by new branch always.
Thanks!

@harshkhandeparkar
Copy link
Member

@Divy123 @harshithpabbati I don't think there is a need to checkout main, just remove the footer.

@Divy123
Copy link
Member

Divy123 commented Feb 16, 2019

@harshkhandeparkar this a rule which I said that new PR by new branch.

@harshkhandeparkar
Copy link
Member

Divy123 this is a new branch, it is just branched from the footer branch so it has those changes.

@harshithpabbati
Copy link
Author

harshithpabbati commented Feb 16, 2019

I will remove the footer and send it to you

@Divy123
Copy link
Member

Divy123 commented Feb 16, 2019

Also @harshithpabbati something I want to know from you that if you have created a new branch resize by remaining on footer branch.
I think that may be the case because of that only your footer changes are visible.
So the order is always checkout main from your current branch before making changes that will result into a new PR

@harshithpabbati
Copy link
Author

Yeah i did it by mistake i didn't see it

@Divy123
Copy link
Member

Divy123 commented Feb 16, 2019

@harshithpabbati donl't do this as this is not a good practice.
Please checkout to the main and then do checkout a new branch so that no any unexpected changes may happen.

@harshkhandeparkar
Copy link
Member

@harshithpabbati when you create a new branch, it is branched from the branch that is currently checked out. Always checkout main before branching. Also keep main up to date. If you are using the git tab of a code editor like atom or vscode then it normally asks which branch to branch from. Always choose HEAD branch. Thanks.

@jywarren
Copy link
Member

jywarren commented Feb 16, 2019 via email

@harshithpabbati
Copy link
Author

How do I edit it??Do I close the request and send a new pr??

@harshkhandeparkar
Copy link
Member

You can commit to the branch and the changes will be visible here.

@harshithpabbati
Copy link
Author

even though i added the font awesome icons i can't get the icon

@harshkhandeparkar
Copy link
Member

Can you please commit the changes?

@harshithpabbati
Copy link
Author

@harshkhandeparkar made the commit

@harshithpabbati
Copy link
Author

screenshot from 2019-02-16 20-47-20
This was the changes which I made.But can't get the image in it's place

@harshkhandeparkar
Copy link
Member

I don't think this is your problem. @Divy123 the preview is not loading. Can you please look into it?

@harshithpabbati
Copy link
Author

I don't think this is your problem. @Divy123 the preview is not loading. Can you please look into it?

screenshot from 2019-02-16 20-59-45
Checkout this @harshkhandeparkar

@harshkhandeparkar
Copy link
Member

Yes, exactly the preview image behind the icon is not loading. @Divy123 please look into this.

@Divy123
Copy link
Member

Divy123 commented Feb 16, 2019

@harshithpabbati you need to do just one thing.
Navigate to the image-sequencer/examples/lib/insertPreview file and at the line number 33 inside previewSequencerSteps
you need to add resize:"125%" .
I think that should do.
Thanks !
Please feel free to ask in case you need any help.

@harshithpabbati
Copy link
Author

@harshithpabbati you need to do just one thing.
Navigate to the image-sequencer/examples/lib/insertPreview file and at the line number 33 inside previewSequencerSteps
you need to add resize:"125%" .
I think that should do.
Thanks !
Please feel free to ask in case you need any help.

thanks for the help @Divy123

@harshithpabbati
Copy link
Author

Done with all the changes.

@Divy123
Copy link
Member

Divy123 commented Feb 16, 2019

@harshithpabbati screenshot please.

@harshithpabbati
Copy link
Author

screenshot from 2019-02-16 23-00-46

@Divy123
Copy link
Member

Divy123 commented Feb 16, 2019

Please do grunt build

@harshkhandeparkar
Copy link
Member

Please run grunt browserify and grunt uglify and commit the changes.

@harshkhandeparkar
Copy link
Member

@Divy123 currently grunt build does not build the ui files.

@Divy123
Copy link
Member

Divy123 commented Feb 16, 2019

Oh my bad!
Thanks @harshkhandeparkar .

@harshithpabbati
Copy link
Author

harshithpabbati commented Feb 16, 2019

Please run grunt browserify and grunt uglify and commit the changes.

Yeah i did it before using npm start and even did it now.

@harshkhandeparkar
Copy link
Member

You have yo commit the files inside dist/ folder after running the grunt commands. You have to run the grunt commands each time you touch a js file. Thanks.

@harshithpabbati
Copy link
Author

@publiclab/is-reviewers done with the changes.

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.

Awesome!

@harshithpabbati
Copy link
Author

@publiclab/is-reviewers

@harshkhandeparkar
Copy link
Member

Your PR has been reviewed already and is ready to merge. When jywarren sees your PR next time , he will merge it(after a final review). According to his time, now will be the beginning of a sunday so you will have to wait for another 24hrs before it gets merged. Thanks!

@harshithpabbati
Copy link
Author

Oh ok fine

@harshithpabbati
Copy link
Author

I have a doubt how can i join into the reviewers team??

@harshkhandeparkar
Copy link
Member

Jywarren is really busy since he has to look after all the projects of publiclab. The biggest project i.e. plots2 has more than 100 PRs open so its really difficult for him to reply to all the questions. I hope you understand.

@harshkhandeparkar
Copy link
Member

I have a doubt how can i join into the reviewers team??

There are some prerequisites mentioned in #656 and you have to be well acquainted with the codebase and have to have some experience working with the publiclab community.

@jywarren jywarren merged commit fdb38f7 into publiclab:main Feb 18, 2019
@welcome
Copy link

welcome bot commented Feb 18, 2019

Congrats on merging your first pull request! 🙌🎉⚡️
Your code will likely be published to https://sequencer.publiclab.org in the next few days.
In the meantime, can you tell us your Twitter handle so we can thank you properly?
Now that you've completed this, you can help someone else take their first step!
See: Public Lab's coding community!

@jywarren
Copy link
Member

Great work here!!! Thanks so much!

@jywarren
Copy link
Member

And we'd love to have more contributions from you! 🎉🙌🏽

@harshithpabbati
Copy link
Author

harshithpabbati commented Feb 18, 2019

And we'd love to have more contributions from you! 🙌🏽

Sure @jywarren

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants