Feature overlay top and left offset support #473
Conversation
Merge from lovell/sharp
|
Thank you very much for this, looks very useful. I'll make time over the weekend to review in more detail. |
| @@ -364,7 +365,10 @@ Sharp.prototype.overlayWith = function(overlay, options) { | |||
| throw new Error(' Invalid Value for tile ' + options.tile + 'Only Boolean Values allowed for overlay.tile.'); | |||
| } | |||
|
|
|||
| if (isInteger(options.gravity) && inRange(options.gravity, 0, 8)) { | |||
| if(isInteger(options.left) && options.left >= 0 && isInteger(options.top) && options.top >= 0) { | |||
There was a problem hiding this comment.
Given the strictness in the rest of this function, I think if people try to define either left or top and either are invalid, it should also throw an Error.
|
It's always a pleasure to see a PR with tests, thank you for adding these, it makes future maintenance of this feature so much easier. I've added a couple of minor comments/nitpicks inline. Are the |
| @@ -8,7 +8,6 @@ var events = require('events'); | |||
| var semver = require('semver'); | |||
| var color = require('color'); | |||
| var BluebirdPromise = require('bluebird'); | |||
|
|
|||
There was a problem hiding this comment.
Nitpick: I like the separation between third and first-party dependencies.
|
The Appveyor CI is failing and I am not sure why. Let me know if something has to be fixed. |
|
The latest version of npm should fix the AppVeyor problem - see npm/npm#13199 |
|
This is looking great now, thank you. Please can you remove the unused test expectations: then rebase against upstream/master and squash to a single commit ready for merging. |
|
I created a new branch with all the changes in a single commit. I still get a few merge commits from the master branch. Any ideas how do I squash them? |
|
Squashed as 278273b - thanks again Rahul for all you work on this handy addition. |
No description provided.