Add shadow to the output diffImage #25
Conversation
|
I don't understand this PR. Can you please explain it? |
|
Sorry! forgot to add a description. I've updated it. |
| @@ -54,12 +54,13 @@ ImageDiff.createDiff = function (options, cb) { | |||
| // http://www.imagemagick.org/script/compare.php | |||
| var diffCmd = 'compare'; | |||
| var diffArgs = [ | |||
| 'compare', | |||
| // 'compare', | |||
There was a problem hiding this comment.
I think we didn't mean to comment out this line =/
There was a problem hiding this comment.
Ah, it looks like that was a duplicate altogether. Can we remove the line instead of commenting it out?
|
Left some comments on how to better structure the code. As far as naming goes with "shadow", I don't feel like that gives me the right idea. How about something more focused at opacity or averaging the images? (e.g. |
|
Thanks for the comments! I'll take them into account. |
|
Ah, k. I guess we should keep |
| '-compose', 'Src', | ||
| '-dissimilarity-threshold', '1', | ||
| '-highlight-color', 'RED'] | ||
| //shadow options if options.shadow is set |
There was a problem hiding this comment.
Please add a space after // and use Sentence casing for options to be consistent with other comments:
// Shadow options if options.shadow is set
|
Our dissimilarity threshold comments are getting buried due to a shifting diff. Previous to this PR, both locally and Travis CI have been passing without compare --version |
|
One more thing to note is that typically PRs need to be squashed to 1 commit to be landed so we should start squashing on each update to this PR =/ |
|
the version of imagemagick im running is: |
|
whoops wrong button |
|
I am running |
|
I shouldve squashed everything. I upgraded to 6.9, and found the dissimilarity threshold unnecessary, so thats good. |
|
Sweet, everything looks good. I am going to open up a follow up PR with an updated README/CHANGELOG to get this landed (weird process due to leaving the organization). I will post here with the PR number to subscribe but this should auto-close when it's landed. |
|
Alright, #26 has been opened and hopefully this will be published soon =) |
Landed #25 with updated CHANGELOG/README
|
@064678147 You should be good to go. This has been landed/released in |
This allows the user to set a shadow:true tag in options, such that a faint shadow of the original image can be seen in the diff image. This is helpful when screenshots are image-diffed so that reviewers can know exactly which part of the screenshot changed by looking at the diffed image instead of having the find their bearings in the original image.