Add image rotate filter #588
Conversation
|
If it's OK, I can add a few usage examples for it |
|
Looks good to me, could you please add some tests for it and fix currently broken ones. Would be good if you can add some docs too. |
makasim
added
the
Level: New Feature 🆕
|
Private method and variable looks useless |
|
There is not much need of private method. The code is pretty simple to understand in one place. 👍 Let's do as @Koc suggested. Same for the property. |
bocharsky-bw
force-pushed
the
master
branch
2 times, most recently
from
2f810be to
520ff9b
Compare
|
@makasim I added some tests and usage example to docs. |
| */ | ||
| public function load(ImageInterface $image, array $options = array()) | ||
| { | ||
| $angle = isset($options['angle']) ? intval(round(floatval($options['angle']), 0)) : 0; |
There was a problem hiding this comment.
intval(round(floatval($x) is it really should be that complicated?
There was a problem hiding this comment.
$angle = isset($options['angle']) ? (int) $options['angle'] : 0;
There was a problem hiding this comment.
OK, I want to maintain a good precision in this place, but most probably it's unnecessary because angle in 0.5 degree will not be visible to the naked eye )
There was a problem hiding this comment.
I am thinking we should not do anything here. The angle comes from config? A developer set it? So it is ok if he wants 0.5 angle. Let's just pass it as is to rotate method...
wdyt?
There was a problem hiding this comment.
In my case, I use this filter at runtime in my twig templates, and my angle evaluate from several params and can be float. But Imagine\Image\ManipulatorInterface::rotate method, that I used in filter loader, expect angle as int, so you're right, we need to type cast to int and discard fractional part.
| ; | ||
|
|
||
| $result = $loader->load($image, array('angle' => 90)); | ||
| $this->assertInstanceOf('Imagine\Image\ManipulatorInterface', $result); |
There was a problem hiding this comment.
You have to create one more image mock and return it from rotate method. and here you have to check that it is that image, like you did in previous test
$this->assertSame($rotatedImage, $result);
There was a problem hiding this comment.
Do not check for instance of and keep only one load method with image and rotatedImage.
There was a problem hiding this comment.
Check again, please
|
Thanks |
PR for #587 issue.