Skip to content
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

🐛 [BUG] - is_imageslider some problems #361

Open
maofree opened this issue Dec 26, 2023 · 8 comments
Open

🐛 [BUG] - is_imageslider some problems #361

maofree opened this issue Dec 26, 2023 · 8 comments
Labels
bug Something isn't working is_imageslider minor

Comments

@maofree
Copy link

maofree commented Dec 26, 2023

Description

Hi

I report here some problems that I have with is_imageslider:

  1. the url of placeholder.jpeg doesn't consider any folders that contain the site, for example if the site was included in the "shop" folder, the URL of this image does not include it
  2. placeholder.jpeg appears only when creating new slides, but if a new language has been activated, it does not appear
  3. once an object has been created it is not possible to modify an image even if I insert a new file in the selector, however for texts it is possible to change them
  4. then there is a critical error for sites with many languages, example 11, after having loaded the images for all the languages, at each save it tells me that 1 is always missing for 1 or 2 languages, and at each save they are different languages. If I deactivate two languages then it makes me generate the slide correctly. If I then try to reactivate the two languages and insert the photos for the reactivated languages and save, I get this error because it doesn't see the selected images. I'll send you a video to better explain this error.

1
2
3
4

bye

Node.js version

v16

php version

8.1

OS and it's version

linux

Browsers

Chrome

Required module/theme

is_imageslider

Reproduction steps

1. Use a website with 11 languages
2. create a new slide and add images for each language and then save
3. if you get the errore "This value should not be empty. - Language: Dansk (Danish)"
disable two languages and then try again. Then reactive the 2 languages and add to them the missing images to get the last error

Logs

Oksydan\IsImageslider\Form\DataHandler\ImageSliderFormDataHandler::eraseFile(): Argument #1 ($fileName) must be of type string, null given, called in /var/www/html/kasastore/8/modules/is_imageslider/src/Form/DataHandler/ImageSliderFormDataHandler.php on line 152
@maofree maofree added the bug Something isn't working label Dec 26, 2023
@maofree
Copy link
Author

maofree commented Dec 26, 2023

now I tried again to insert all the images due to the error reported in point 4 and it made me generate the slide without errors, just when I was making the video :).
I tried again to create a new slide this time it gives me errors in two languages, then I tried to put them back and it makes me save the slide, so it seems that the errors are there on the first try and if I put them back then it makes me save.

Tried on the real website and it is not possible to creare a new slide with 11 languages, with 9 yes

https://watch.screencastify.com/v/11UeESMiPp6CaCUIiu5p

@Oksydan
Copy link
Owner

Oksydan commented Dec 29, 2023

Hi @maofree,

problem with uploading images for 11 languages might be due to limitation of upload_max_filesize or post_max_size on your server. OFC we have to handle this kind of error better with an exception and proper error message.
I am adding this to is_imageslider EPIC

@maofree
Copy link
Author

maofree commented Dec 30, 2023

Hi @Oksydan
I think it is necessary to change the way in which the data is saved after pressing the Save button.
From what I've seen, if there are errors, images are not saved and new slides are created.
if I take this site as an example which has 11 languages, to create a slide it would have to save 44 images, 4 per language, 2 jpeg and 2 webp, then in the future if it were to also manage the avif format it would become 6, so it would become very likely to encounter limitations of the server, especially in shared ones.
a simple way to get around this problem is to use a foreach and have it manage texts and images only for one language at a time, so you can save 2-4-6 images at a time without server limit problems.
then there is another problem, that when there are errors, since nothing is saved, every time you have to put back photos and texts for each language and try again, instead if it saves the data for some languages, you could only try again where there are errors.

@maofree
Copy link
Author

maofree commented Dec 30, 2023

1
these are the limits of php, but with litespeed there are other parameters that I don't have access to. However, by managing the data one language at a time, there should no longer be these problems

@Oksydan
Copy link
Owner

Oksydan commented Jan 6, 2024

Hi @maofree,

webp files are not created within upload process. webp files are created on demand.
I am thinking how to change it 🤔. I can make FileType not required so you could be able to upload them separately and it could be the easiest workaround tbh.
On the other hand, I am thinking of adding something like a all langs switch for images. If the all langs switch is checked, we could set this image for each language, but it will only save one file.

@Oksydan Oksydan added the minor label Jan 6, 2024
@maofree
Copy link
Author

maofree commented Jan 6, 2024

Hi @Oksydan
can you tell me where are saved the image associations? in the database this module does not have a table and I don't even think it is in the configuration table. I think what you suggest is correct, because in this way it is possible to replace images, or add new ones if they were not present for some languages, I usually avoid too rigid solutions, because each site and server have different configurations, so it is right to consider more wait

@Oksydan
Copy link
Owner

Oksydan commented Jan 6, 2024

Hi,

are you asking about where are stored images names?
All of them exists in image_slider_lang table and these fields are inside ImageSliderLang ORM Entity

@maofree
Copy link
Author

maofree commented Jan 6, 2024

Hi
I had seen those tables, but I thought they were from another module, I thought you used names that started with is_
I didn't check the module code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working is_imageslider minor
Projects
None yet
Development

No branches or pull requests

2 participants