JoomGalleryfriends / JG4-dev

Development repository for JoomGallery v4.x
GNU General Public License v3.0
10 stars 7 forks source link

Image & Category Manager #31

Closed Elfangor93 closed 2 years ago

Elfangor93 commented 2 years ago

What is this pull request for

Very basic functionality of Image and Category Manager realized. It is now possible to use all the buttons and functionalities of the Image and Category Manager.

How to test this pull request

szepty-ziemi commented 2 years ago

ENV: obraz

Warnings are displayed:

GIVEN User is editing image in Image Manager WHEN User set language for image different than ALL AND Save & Close THEN Warning is displayed:

Warning: Undefined property: stdClass::$language_image in /home/siristru/public_html/j4/layouts/joomla/content/language.php on line 21

Warning: Undefined property: stdClass::$language_title in /home/siristru/public_html/j4/layouts/joomla/content/language.php on line 25 Undefined

obraz

szepty-ziemi commented 2 years ago

obraz

But this warning doesn't show each time.

szepty-ziemi commented 2 years ago

Questions:

  1. Shouldn't be Publish -> YES set by default? (also apply to Category Manager) obraz

  2. Shouldn't be Recording date field by empty by default? obraz

szepty-ziemi commented 2 years ago

While copying images (Save as Copy) zero (0) is added to image alias. When copying the same file again 1 is used:

obraz

Shouldn't it follow the same logic as in Joomla articles?

  1. Alias is given 2 (instead 0)
  2. (2) is added to article name

obraz

szepty-ziemi commented 2 years ago

There is an issue while changing category for already existing image.

GIVEN User is editing an image WHEN User changes original category to another AND Click Save THEN An error is displayed that image can't be moved

obraz

szepty-ziemi commented 2 years ago

I notice that sometimes thumbnail are not loaded:

obraz

This may be related to hosting. After reloading page they are correctly loaded.

szepty-ziemi commented 2 years ago

Maybe by default we should display some generic image here:

obraz

Some kind of placeholder.

szepty-ziemi commented 2 years ago

When trying load animated GIF image:

obraz

szepty-ziemi commented 2 years ago

I created new image type "fix" that uses watermark and resize. I used gif with transparency. Image was processed but transparency was lost:

obraz

szepty-ziemi commented 2 years ago

Question: Shouldn't filtering by category follow the same look and logic as category filtering in articles? That is ability to select multiple categories and showing selected category as "label" with x.

obraz

Selected category in com_content

obraz

szepty-ziemi commented 2 years ago

Ordering categories in Category Manager doesn't work:

szepty-ziemi commented 2 years ago

In Global configuration -> General settings -> Image processing -> Image processing (static images)

When adding new imagetypes they seems to "push" down previous/original imagetypes, kinda rep;acing them and inherit their settings on the list:

obraz

There ware three original, then I added "fix". it appeared on the top of the list. Then added "show" and it appeared on the top of the list, pushing down "fix"... and "fix' inherited locked activate switch to YES... and also it can't be removed as it doesn't have minus button at right.

szepty-ziemi commented 2 years ago

Lack of feature: After adding new imagetype it will appear in the image settings... and it will be empty. That is obvious as image was uploaded BEFORE new image type was created. But maybe we need a "Recreate" button in Image Manager to be able to create it?

obraz

szepty-ziemi commented 2 years ago

Category global settings override doesn't work.

In Category X I've set to add watermark, what should override global settings:

obraz

(also information under that setting should be updated, to point into right place... but that later on)

Uploaded an image to this category and watermark is not added (it's added only on image type which has watermark enabled):

obraz

The same apply to override watermark settings in Image Manager. Here "recreate" button will be helpful as well as it's possible to set watermark after image was uploaded.

But when I set this to add watermark BEFORE I create image... watermark is not added:

obraz

szepty-ziemi commented 2 years ago

My category X does have thumbnail set:

obraz

But this is not reflected on category list:

obraz

szepty-ziemi commented 2 years ago

Hidden categories and images - this setting is not reflected on the lists. For hidden or unpublished categories on select list, we use names in brackets eg. [First category]. This is very helpful to identify that this category is not visible at the frontend.

Maybe we should reflect this also on the category and image lists by adding brackets to titles as well?

obraz

obraz

Elfangor93 commented 2 years ago

@szepty-ziemi I added rections to your issues:

MrMusic commented 2 years ago

If the alias of an existing category is changed, a new folder is created in the directory structure instead of changing the existing one.

Tested with PHP 8.1 OS: windows Joomla 4.1.5 Error Reporting: Maximum

MrMusic commented 2 years ago

When creating a new image, the message appears:

Deprecated: Optional parameter $type declared before required parameter $pks is implicitly treated as a required parameter in ...administrator\components\com_joomgallery\src\Model\ImageModel.php on line 860

This message also seems to prevent the image from being displayed in the image manager.

Tested with PHP 8.1 OS: windows Joomla 4.1.5 Error Reporting: Maximum

MrMusic commented 2 years ago

When changing an already existing image, the following error occurs during saving:

Save failed with the following error: Incorrect integer value: '' for columnjg4dev.jg4_joomgallery.checked_outat row 1

Tested with PHP 8.1 OS: windows Joomla 4.1.5 Error Reporting: Maximum

MrMusic commented 2 years ago

When trying to delete a category that contains images, the following error occurs:

An error has occurred.

    0 trim(): Argument #1 ($string) must be of type string, array given 

and nothing will be deleted

Tested with PHP 8.1 OS: windows Joomla 4.1.5 Error Reporting: Maximum

Elfangor93 commented 2 years ago

Deprecated: Optional parameter $type declared before required parameter $pks is implicitly treated as a required parameter in ...administrator\components\com_joomgallery\src\Model\ImageModel.php on line 860

Can not confirm this issue. Image creation without any messages or errors. Tested with different PHP versions v7.4.x up to v8.1.x

Elfangor93 commented 2 years ago

There is currently no possibility to delete categories wich contains images and/or subcategories. The way how we want to make this happen has to be discussed and will be integrated in a next pull reqest.

Same for the settings displayed in the parameters tab of images and categories. This setting scurrently have no functionality. This functionality will be integrated in a next PR.

Elfangor93 commented 2 years ago

@szepty-ziemi and @MrMusic please confirm that your issues found are fixed now.

szepty-ziemi commented 2 years ago

ENV: https://prnt.sc/fY47BdGuCYZD

Table seems to have too low colspan:

obraz

szepty-ziemi commented 2 years ago

✓ Issue 1 https://github.com/JoomGalleryfriends/JG4-dev/pull/31#issuecomment-1199304358

X Issue 2 https://github.com/JoomGalleryfriends/JG4-dev/pull/31#issuecomment-1199340299

Still persist. I noticed that it appears after a while user is not active for a while in Image Manager list. Then trying to create an image and warning is displayed.

obraz

✓ Issue 3 https://github.com/JoomGalleryfriends/JG4-dev/pull/31#issuecomment-1199359715

✓ Issue 4 https://github.com/JoomGalleryfriends/JG4-dev/pull/31#issuecomment-1199372962

✓ Issue 5 https://github.com/JoomGalleryfriends/JG4-dev/pull/31#issuecomment-1199446665

Issue 6 - ignored

✓ Issue 7 - https://github.com/JoomGalleryfriends/JG4-dev/pull/31#issuecomment-1200129652

X Issue 8 - https://github.com/JoomGalleryfriends/JG4-dev/pull/31#issuecomment-1200130122

Still persist in my instance. It may be related with GD version?

obraz

When tried with IM:

obraz

Here is that image: http://j4.siristru.smarthost.pl/gN4HK.gif Could you guys try on your end?

✓ Issue 9 https://github.com/JoomGalleryfriends/JG4-dev/pull/31#issuecomment-1200131094

✓ Issue 10 https://github.com/JoomGalleryfriends/JG4-dev/pull/31#issuecomment-1200132552

szepty-ziemi commented 2 years ago

Question: What about WEBP files? I've tried to upload that image type and I get error:

obraz

Does this depend from image processing library or we can do something about it?

szepty-ziemi commented 2 years ago

There is an issue when renaming category:

GIVEN User has already created a category WHEN User rename category THEN Category folder in file structure remain with old name BUT Category folder in file structure should change name to new one

There is no matter if folder contains images. It behave the same way for both scenarios. Name is changed correctly in Category and Image Managers. Issue apply only to file structure.

obraz

obraz

After renaming category:

obraz

In file system folder remain with old name:

obraz

Elfangor93 commented 2 years ago

Question: What about WEBP files? I've tried to upload that image type and I get error:

Well, the current IMGtool service is not able to process WEBP images. But there are several other things that I dont like here.

I like this case. We found here multiple issues at ones, but not the WEBP, thats not an issue right now ;-)

Elfangor93 commented 2 years ago

There is an issue when renaming category:

The folder name is related to the alias not to the title. In order for the folder to be renamed you need to rename the alias. In your case I can not see any issue...

szepty-ziemi commented 2 years ago

There is an issue when renaming category:

The folder name is related to the alias not to the title. In order for the folder to be renamed you need to rename the alias. In your case I can not see any issue...

Checked once again and you're right. Changing alias change also folder name. All good.

Elfangor93 commented 2 years ago

Issue 8 could be solved for the IM processor. For GD I still was not able to reproduce the error and therefore to detect the failure source...

Issue 2 could be solved.

Elfangor93 commented 2 years ago

@szepty-ziemi I tried to fix the bug on issue 8. Error when creating GIF image with GD. Can you please check, if that helped?

szepty-ziemi commented 2 years ago

@szepty-ziemi I tried to fix the bug on issue 8. Error when creating GIF image with GD. Can you please check, if that helped?

Issue resolved. Animated gif is uploaded and image is created:

obraz

Elfangor93 commented 2 years ago

But there are several other things that I dont like here.

  • In debug it tells us that the image is added successfully even there is no image added in the filesystem.
  • A filename is stored to the database even there are no files available.
  • A green message appears giving the user the feeling that everything went well.

I created issue #35 to care about that. I will not fix this in this PR... I think we can merge this PR now, what do you think @szepty-ziemi?

szepty-ziemi commented 2 years ago

@Elfangor93 yes, we can merge this PR :D