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 resize module #451

Merged
merged 8 commits into from
Nov 3, 2018
Merged

Add resize module #451

merged 8 commits into from
Nov 3, 2018

Conversation

JKusio
Copy link

@JKusio JKusio commented Oct 28, 2018

Added resize module.
If you set resize value equal to 100%, the image will get twice as big, and when you set -100%, the image will be two times smaller.

It is my first module, so if something is wrong, tell me, and I'll fix it 😄

@JKusio
Copy link
Author

JKusio commented Oct 29, 2018

#435

Can you review it @publiclab/reviewers? 😃

Copy link
Member

@tech4GT tech4GT left a comment

Choose a reason for hiding this comment

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

This looks quite good to me, a couple of things though

  • Can you please add a screenshot of your work, it would make the review process easier 😄
  • I see you have modified the test file, please explain that?

@tech4GT
Copy link
Member

tech4GT commented Oct 29, 2018

Thanks for your pull request! 😊

@JKusio
Copy link
Author

JKusio commented Oct 29, 2018

It's hard to review it based on screenshots because it's hard to see a difference. But I'll add them when I get home.
I've changed the test file because I did some previous pull request with blend module, and now I've checked that there was an unused variable, and the test was pasted twice.

@JKusio
Copy link
Author

JKusio commented Oct 29, 2018

In browser:
When resize is set to 100%
https://prnt.sc/lc0sg8

When resize is set to -25%
https://prnt.sc/lc0st9

Original image
https://prnt.sc/lc0syl

+25% resize
https://prnt.sc/lc0t34

-25% resize
https://prnt.sc/lc0t6x

Copy link
Member

@tech4GT tech4GT left a comment

Choose a reason for hiding this comment

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

Good work here!! 🎉

@jywarren
Copy link
Member

This does look great! I wanted to ask, what if each percentage is taken as a proportion of the original size, where 125% means 25% bigger, and 75% means 25% smaller? In that model, a negative value would be meaningless. What do you think?

@jywarren
Copy link
Member

Also, as a follow-up issue, we might add a resize by pixel height and width, which we can make compatible with the percentage based interface. But we can open an issue later!

This is really cool! Thanks for your hard work here!!!

@JKusio
Copy link
Author

JKusio commented Oct 30, 2018

Ok, I've changed the resize calculation, and now it's taken as proportion.

However, I've got a questions that would help me understand this code fully, because I've copied some parts from other modules.

What this code does?
progressObj.stop(true); progressObj.overrideFlag = true;

@jywarren
Copy link
Member

jywarren commented Oct 30, 2018 via email

@jywarren
Copy link
Member

jywarren commented Nov 2, 2018

Is this otherwise ready? Thank you!!!

@JKusio
Copy link
Author

JKusio commented Nov 2, 2018

I think it is 😄

@jywarren jywarren merged commit 5f57123 into publiclab:main Nov 3, 2018
@jywarren
Copy link
Member

jywarren commented Nov 3, 2018

🙌🏽⚡⚡🚀

@JKusio JKusio deleted the add-resize-module branch November 4, 2018 22:53
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.

3 participants