Allow empty url - Fix PrestaShop/PrestaShop#18536 #74
Conversation
marsaldev
commented
Apr 29, 2022
•
marsaldev
commented
| Questions | Answers |
|---|---|
| Description? | Allow empty url for the slides |
| Type? | improvement |
| BC breaks? | no |
| Deprecations? | no |
| Fixed ticket? | Fixes PrestaShop/PrestaShop#18536. |
| How to test? | Apply fix and create a slide without URL (delete the override tpl file in the classic-theme or hummingbird) 😊 |
| @@ -28,15 +28,15 @@ | |||
| <ul class="rslides"> | |||
| {foreach from=$homeslider.slides item=slide} | |||
| <li class="slide"> | |||
| <a href="{$slide.url}"> | |||
| {if !empty($slide.url)}<a href="{$slide.url}">{/if} | |||
There was a problem hiding this comment.
Doesn't impact for testing, this file has been override in classic theme.
https://github.com/PrestaShop/classic-theme/blob/develop/modules/ps_imageslider/views/templates/hook/slider.tpl
So how QA must be done?!
There was a problem hiding this comment.
Nice question, I've created the PRs also for classic-theme and hummingbird
PrestaShop/classic-theme#27
PrestaShop/hummingbird#247
But I think that it's important that works at module level, than you can have the theme you want and override in it, but it's out of the module business, imho 😊
There was a problem hiding this comment.
I saw related PR's 👍
Which came first, the chicken or the egg? 😄
I think first of all need to merge PR on classic theme.
Then QA can be done here.
As I experienced fixing an issue on diffrent repos is really hard in PS 😉
There was a problem hiding this comment.
Hello @marsaldev ,
Thanks for the PR !
I tested this PR with PrestaShop/classic-theme#27.
Tested on branch 1.7.8.x and develop - PHP 7.4.
Tested the case on on existing slides form and on new slides form. We can add or not a Target URL.
On BO, the form is well saved. On FO, we can see the image slider properly.
It is QA ✅
|
Thanks @marsaldev & @florine2623 |
Related fix for PrestaShop/ps_imageslider#74
