Conversation
webp support
matks
left a comment
matks
left a comment
There was a problem hiding this comment.
Hello @Patrocle !
This is nice, however the current version of ps_imageslider is compatible with PrestaShop 1.7.4.0 until PrestaShop 8.1.0 (here https://github.com/PrestaShop/ps_imageslider/blob/dev/ps_imageslider.php#L65)
WebP image support has been added in PrestaShop 8.0.0 (see PrestaShop/PrestaShop#10356).
So if someone uses this PR on PrestaShop 1.7.8 for example it will not work.
I suggest that, in this PR, you modify the minimum version of PrestaShop compatible with ps_imageslider to 8.0.0 😉
matks
left a comment
matks
left a comment
There was a problem hiding this comment.
Thanks for the changes 😉 I validate the PR. Let's now wait for QA team to test it.
matks
left a comment
matks
left a comment
There was a problem hiding this comment.
Woups I was too quick 😅 we also need to remove the CI jobs that verify the module is compatible with prestashop <= 8.0.0
This is why the CI is failing: it attempts to verify the module works with PrestaShop 1.7.7, 1.7.8 ... but it's not relevant now
| - name: Checkout | ||
| uses: actions/checkout@v2.0.0 | ||
|
|
||
| - name: PHP syntax checker 5.6 |
There was a problem hiding this comment.
@Patrocle I think it would be better to keep PHP checks for 7.2 to 8.1. As you probably know, PrestaShop 8.0 is compatible with PHP 7.2 to 8.1.
|
Hey @Patrocle Thanks for your contribution! A couple of suggestions:
|
… PrestaShop compatibility 8.0.0 and 8.1.0
| strategy: | ||
| matrix: | ||
| presta-versions: ['1.7.4.4', '1.7.5.1', '1.7.6', '1.7.7', '1.7.8', 'latest'] | ||
| presta-versions: ['8.0.0', '8.1.0', 'latest'] |
There was a problem hiding this comment.
I guess we should use '8.0', '8.1' here (they mean 8.0.x and 8.1.x :)
| uses: shivammathur/setup-php@v2 | ||
| with: | ||
| php-version: '7.4' | ||
| php-version: '8.1' |
There was a problem hiding this comment.
I'm confused about the impact of this php-version change 😕
| $this->displayName = $this->trans('Image slider', [], 'Modules.Imageslider.Admin'); | ||
| $this->description = $this->trans('Add sliding images to your homepage to welcome your visitors in a visual and friendly way.', [], 'Modules.Imageslider.Admin'); | ||
| $this->ps_versions_compliancy = ['min' => '1.7.4.0', 'max' => _PS_VERSION_]; | ||
| $this->ps_versions_compliancy = ['min' => '8.0.0', 'max' => _PS_VERSION_]; |
There was a problem hiding this comment.
module version need to be bumped to 4.0.0 accordingly
|
My ps_imageslider v3.1.4 doesn't seem to accept .webp source images even though other settings are configured correctly and products, categories, etc. have .webp source images that also render as such. PrestaShop v8.1.4
EDIT: works after manually applying the fix from @matks |
Did you just change the 'min' value on $this->ps_versions_compliancy to a value of '8.0.0'? I'm having the same problem on the latest Prestashop 8.1.7 and that sadly didn't work for me. |
|
Hi folks, do we know around when it will be compatible? With latest version 3.2.1 under Prestashop 8.2, it doesn't look to be compatible. |
|
Hello, what changes are required ? |
| $this->displayName = $this->trans('Image slider', [], 'Modules.Imageslider.Admin'); | ||
| $this->description = $this->trans('Add sliding images to your homepage to welcome your visitors in a visual and friendly way.', [], 'Modules.Imageslider.Admin'); | ||
| $this->ps_versions_compliancy = ['min' => '1.7.4.0', 'max' => _PS_VERSION_]; | ||
| $this->ps_versions_compliancy = ['min' => '8.0.0', 'max' => _PS_VERSION_]; |
There was a problem hiding this comment.
But why break compatibility with 1.7 if nothing in this PR actually needs version 8+?
9 participants
Uh oh!
There was an error while loading. Please reload this page.