Add RTL sample images

[mparvazi]

Questions	Answers
Description?	Add RTL version of sample images and improve image upload. (Redo for #70 )
Type?	improvement
BC breaks?	no
Deprecations?	no
Fixed ticket?	-
How to test?	-

[mparvazi]

@Progi1984 @matks

I forget what I did on #70
So I tried to create a new one from scratch to make sure it works fine.

[matks]

Thanks @mparvazi !

I think you do a very interesting thing but can you explain it?

I see you added RTL versions for the images and enabled the code to use RTL images when using RTL language. What happens for custom images? If I add my own item to the slider how can I provide a RTL and a not-RTL image?

We need to explain this to users 😁 else nobody will use it.

[mparvazi]

Thanks @mparvazi !

I think you do a very interesting thing but can you explain it?

I see you added RTL versions for the images and enabled the code to use RTL images when using RTL language. What happens for custom images? If I add my own item to the slider how can I provide a RTL and a not-RTL image?

We need to explain this to users 😁 else nobody will use it.

The PS is not responsible for end users who want to add RTL language to shop,
but doesn't know how to create an image for slides in RTL.

These sample images will be used for PS demo (RTL language users thinks PS have problem in RTL for slider in first experience).
It will be useful for theme developers to see how their theme looks like correctly in RTL.

I just tried to flip current images horizontally to be used in RTL (fast solution),
but it would be better to design new sample images for LTR and RTL (by UX Team).

[NeOMakinG]

@matks

Users are able to upload slider images for any language, this PR adds some demo images to RTL languages, to display it properly, that's it 👍

[matks]

@NeOMakinG @mparvazi Is the following scenario possible:

1. I use ps_imageslider
2. I add a a custom slider element, using image landscape.png
3. I setup a RTL language
4. A customer visits my shop and select the RTL language
5. The ps_imageslider module tries to show the custom slider element and looks for landscape_rtl.png image but does not find it

[mparvazi]

The _rtl suffix only used in installSamples() function, doesn't impact in other functions.
installSamples() only will use on installation or reseting module.

back to scenario:
when you setup image slider for lang1, and then you add lang2 to the shop,
the images of lang1 (default shop lang) will be used for lang2.
if you add lang3 then the images of lang1 will be used for lang3.
(this process is same for LTR and RTL language)

Let's imagine hummingbird is ready to preview for demo

This is what RTL language users see:

But I think something like this is better:

[atomiix]

hi @mparvazi, it seems that this PR fixes at least 1 issue (PrestaShop/PrestaShop#27777) and add a new behavior when installing the module with an RTL language. Can you be more explicit on what your PR is doing so that the QA team can validate it easier?
Thanks

[mparvazi]

hi @mparvazi, it seems that this PR fixes at least 1 issue (PrestaShop/PrestaShop#27777) and add a new behavior when installing the module with an RTL language. Can you be more explicit on what your PR is doing so that the QA team can validate it easier? Thanks

Hi @atomiix

For QA:

1. It's possible to change image of slide for one language without need to change images for all languages.
2. It adds RTL version of SAMPLE images for fresh install of module.

[sLorenzini]

hello @mparvazi,

Thank you for this PR.
Tested on 178x, develop, 80x branches with PHP 7.2 version.

With fresh install and 2 default languages on the shop:

1. with classic theme ✅

• add a new slider on one language: no more empty icon on BO & no more blank space on FO on the other language
• add RTL language on the shop on BO
• Generate RTL stylesheet on theme & logo page on BO
• Reset the ps_imageslider module
• Go to FO, refresh the page & choose the RTL language installed: the slider is compatible with RTL language

2. with hummingbird theme ✅: same steps except the "Generate RTL stylesheet"
3. However, there is a small issue on BO in the ps_imageslider configuration page.
   An asterisk is disappeared in front of "Target url" field. The field is still required. Tested with the 3 branches and same result

Without this PR:

With this PR:

Thanks

[mparvazi]

Hello @sLorenzini

Please check this PR: #74

After merging that PR, we can leave target URL empty.
Btw I didn't change anything about required fields.

Thanks.

[sLorenzini]

Hello @sLorenzini

Please check this PR: #74

After merging that PR, we can leave target URL empty. Btw I didn't change anything about required fields.

Thanks.

Hello @mparvazi,

Indeed, the target URL field could be empty with the last merged PR on the module.

In that case, your PR is QA approved ✅ .

Thank you

[Progi1984]

Thanks @mparvazi & @sLorenzini