Fix Image::fromString to support PNG and WebP files #31
Conversation
- Add missing fromString methods to PNG and WebP classes - Update Image::fromString to read image size and MIME type with getimagesizefromstring - Update and refactor related tests
|
@ilu I have time to review on Thursday but if @klaari needs this to be published already, feel free to merge/publish now and I can still review afterwards :) For publishing, all you need to do is push a git tag with the new version number after you have merged. See https://docs.frameright.io/php/contributing#releasing |
|
(and thanks for the contribution @klaari !) |
|
@lourot I think it's better you review & publish, but let me know if something comes up and I can take over so we get it out this week 🙌 |
Expected to fail because in the current implementation, Image::fromString() calculates the image size but methods like PNG::fromString() and JPEG::fromFile() don't.
There was a problem hiding this comment.
@klaari this looks great and I could have merged as is but I spotted an additional issue and maybe we could go the extra-mile and fix it as well. What do you think?
Right now Image::fromString() calculates the image size but PNG::fromString() and JPEG::fromFile() for example don't. I think all these methods should behave the same and calculate the image size. I have added now-failing test assertions to illustrate that. If you agree but have no time, that's perfectly fine, I can also do it. Just let me know.
And thanks a lot for having refactored the tests. It looks way better now!
|
Hi @lourot! Good point, I agree. |
|
@klaari Thanks a lot! I refactored a bit and will merge and release in a few hours. Please shout if you think I may have missed something. |
Fixes #30