New Shadow module

[niravasher]

Fixes #806 (<=== Replace 0000 with the Issue Number)

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/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!

[niravasher]

@publiclab/is-reviewers @harshkhandeparkar @jywarren @VibhorCodecianGupta please review this. Made it with the help of canvas resize module.

[codecov]

Codecov Report

Merging #1507 into main will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1507   +/-   ##
=======================================
  Coverage   64.28%   64.28%           
=======================================
  Files         129      129           
  Lines        2629     2629           
  Branches      421      421           
=======================================
  Hits         1690     1690           
  Misses        939      939

[harshkhandeparkar]

Did you copy the code of canvas resize or do you use canvas resize module inside this module?

[niravasher]

This portion is from canvas resize, the rest is similar to that of gradient module. But this cannot be a metamodule of the two because here gradient is present only till 20 pixels. @harshkhandeparkar

for (let i = 0; i < iMax && i < w; i++) {
        for (let j = 0; j < jMax && j < h; j++) {
          let x = i + 20, y = j + 20;
          pixelSetter(x, y, [pixels.get(i, j, 0), pixels.get(i, j, 1), pixels.get(i, j, 2), pixels.get(i, j, 3)], newPixels);
        }
      }

[harshkhandeparkar]

Don't you think it will be better to use canvas resize inside of this module because if a change is required (to the canvas resize code), ever in the future, we won't have to change it at two places.

Also, duplicate code is not good. Also, the code will be easier to keep track of. And easier to document.

[niravasher]

So you want me to create a metamodule with some extra lines of code that make up for the gradient? @harshkhandeparkar

[harshkhandeparkar]

Not exactly a meta module. Just import `require('../CanvasResize/...')`. Have a look at the code of EdgeDetect. It imports blur module in this manner.

…

On Wed, 15 Jan, 2020, 10:51 PM Nirav Asher, ***@***.***> wrote: So you want me to create a metamodule with some extra lines that make up for the gradient? @harshkhandeparkar <https://github.com/HarshKhandeparkar> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1507?email_source=notifications&email_token=AIJI5H56CM3SZNV4SRT46XDQ55AYTA5CNFSM4KGYG3D2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJBDHLA#issuecomment-574763948>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIJI5H2547NCWZ2HK3LDVA3Q55AYTANCNFSM4KGYG3DQ> .

[niravasher]

Ohh, looking into it.

[rishabhshuklax]

Not exactly a meta module. Just import require('../CanvasResize/...'). Have a look at the code of EdgeDetect. It imports blur module in this manner.

We use InternalSequencer in edge detect.

@niravasher you can import resize and any other module in the same way

[harshkhandeparkar]

We use InternalSequencer in edge detect.

We don't do that anymore. We just use the import the Module.js and use the draw function.

[codecov]

Codecov Report

Merging #1507 into main will decrease coverage by 1.79%.
The diff coverage is 46.48%.

@@            Coverage Diff             @@
##             main    #1507      +/-   ##
==========================================
- Coverage   66.67%   64.88%   -1.80%     
==========================================
  Files         130      135       +5     
  Lines        2686     2828     +142     
  Branches      433      458      +25     
==========================================
+ Hits         1791     1835      +44     
- Misses        895      993      +98     

Impacted Files	Coverage Δ
src/Modules.js	100.00% <ø> (ø)
src/modules/EdgeDetect/Module.js	100.00% <ø> (ø)
src/modules/WebglDistort/Module.js	2.29% <0.00%> (ø)
src/modules/ColorHalftone/Module.js	3.63% <3.63%> (ø)
src/modules/Shadow/Module.js	44.30% <44.30%> (ø)
src/modules/Overlay/Module.js	93.18% <94.11%> (-1.69%)	⬇️
src/modules/ColorHalftone/index.js	100.00% <100.00%> (ø)
src/modules/Colorbar/Module.js	100.00% <100.00%> (ø)
src/modules/Crop/Module.js	88.88% <100.00%> (ø)
src/modules/EdgeDetect/EdgeUtils.js	86.81% <100.00%> (-0.15%)	⬇️
... and 8 more

[rishabhshuklax]

We don't do that anymore. We just use the import the Module.js and use the draw function.

Yea just saw! I thought we were still using InternalSequencer 😅

[niravasher]

In edge detect, all pixels are made blur by using Blur module. Here I am taking a small part of canvasResize and inside pixelManipulation adding extra lines of code for making gradients. I am not sure that can be done by importing canvasResize module. @harshkhandeparkar . In other words I am modifying core pixeSettings inside canvasResize module. Can't be done by importing the module

[harshkhandeparkar]

Hmm. Does canvas resize return a datauri and not pixels? If that is the case then you can keep the code as it is.

…

On Thu, 16 Jan, 2020, 8:23 PM Nirav Asher, ***@***.***> wrote: In edge detect, all pixels are made blur by using Blur module. Here I am taking a small part of canvasResize and inside pixelManipulation adding extra lines of code for making gradients. I am not sure that can be done by importing canvasResize module. @harshkhandeparkar <https://github.com/HarshKhandeparkar> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1507?email_source=notifications&email_token=AIJI5H44RVJTMOUFXFNKMN3Q6BYGXA5CNFSM4KGYG3D2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJEKU3I#issuecomment-575187565>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIJI5HYFSO2WZ3CFC7GQJDDQ6BYGXANCNFSM4KGYG3DQ> .

[niravasher]

Datauri it is @harshkhandeparkar

[harshkhandeparkar]

Ohk. Then your code is good. 👍

[niravasher]

Please merge the PR @jywarren

[rishabhshuklax]

Hi! This looks good! Could you write a module test to protect this in future?

[harshkhandeparkar]

Could you please add a test for this module? Also, does it work with GIFs?

[niravasher]

Added the test. @harshkhandeparkar

[rishabhshuklax]

Does this works with Gifs?

[rishabhshuklax]

At the moment it just adds the shadow to the top and left can we do something to let the user choose where user wants the shadow like css box-shadow property, ref: W3Schools, that would be really cool!!!

[niravasher]

At the moment it just adds the shadow to the top and left can we do something to let the user choose where user wants the shadow like css box-shadow property, ref: W3Schools, that would be really cool!!!

Yes this can be done @blurry-x-face will do this tmrw.

[niravasher]

@blurry-x-face

[rishabhshuklax]

Nice!

[niravasher]

Yes this does work with GIF's too.

@harshkhandeparkar

[niravasher]

Anything else I need to do here? @harshkhandeparkar ? If not please merge this.

[niravasher]

Pls merge this @jywarren @harshkhandeparkar

[gitpod-io]

[harshkhandeparkar]

Everything works but the code has a lot of repetition and is readable. Can that be changed?

[gitpod-io]

[gitpod-io]

[jywarren]

Awesome!!!! Thanks so much!!!