-
Notifications
You must be signed in to change notification settings - Fork 1
Add configuration for and handling of other image types #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Hello @pykettk, I have thoroughly tested it and all my tests were successful ✅ |
@FinnReinhardtBsc Looks good to me. I think the only things I'd say aren't massively important:
|
Thanks for your review, @pykettk!
Good eye, I added the missing types there! 👍
I removed CDATA for the comment field.
I see your point. When configuring this extension, I personally found it rather annoying to always uncheck the "use system value" box. As I see it, the default for the selected image types and resize mode provide less value than the other defaults do, because it depends a lot on your system. But I understand that it is a good quality of life feature to have, so I changed them again :) I also removed the "Only works on Hyvä Storefronts!" text (as mentioned here). My testing showed, that this functionality is not limited to Hyvä-Themes (although I would always recommend people to use Hyvä, of course :D). I just wanted to hear your take on this, in case I missed something. Thanks for your review, @pykettk! |
I think that was my bad - I copied part of one of our Everything else looks good though! 💪 |
Based on changes from !2