[4.3] Local adapter thumbnails (PR 36552 clone) #38805

obuisard · 2022-09-22T00:47:20Z

Summary of Changes

This Pull Request is for issue #36533 [Solves no 2 of the 3 reported problems, the thumbs] and is a rewrite for #36552 created by @dgrammatiko.

Please refer to the original PR if you want to follow the discussions about the implementation.

The general idea is that image thumbnails are created in the Media Manager and those thumbnails are cached, increasing performance especially when a large amount of images is processed.

The thumbnail creation is 'off' by default.

Testing Instructions

Go to the FileSystem - Local plugin and set 'Create thumbnails' to 'Yes' on the 'images' folder.

Actual result BEFORE applying this Pull Request

The media manager loads full size images.

Expected result AFTER applying this Pull Request

The media manager loads a thumbnail image (max width or height of 200px).

Link to documentations

Please select:

Documentation link for docs.joomla.org: https://docs.joomla.org/Chunk4x:Extensions_Plugin_Manager_Edit_FileSystem_Group
No documentation changes for docs.joomla.org needed
Pull Request link for manual.joomla.org: Update new-deprecations.md for Image Manual#59
No documentation changes for manual.joomla.org needed

Updated thumbnails creation function

Added thumbs to the adapter

Added thumbs field (defaults to true)

Commented out lines in testUpdateFileWithoutAdapter

Added thumbs and thumbSize properties Updated construct, createFile, updateFile, delete, getPathInformation, rglob, checkContent Added getLocalThumbPaths, getThumb, createThumb functions

tab removal

Removed blank space

Replaced all use of mkdir() to Folder::create()

obuisard · 2022-09-24T16:43:11Z

tests/Codeception/api/com_media/MediaCest.php needs to be updated and fails at this point.

dgrammatiko · 2022-10-11T16:03:07Z

Hey, @obuisard I can't reply on #38722 as Nik has blocked me so I'll write my comment here:

when I asked you to resurrect this PR I meant to fork it and patch the things that other were asking me to change in the original PR and also fix the failing tests
- [4.2] Local adapter thumbnails #36552 (review)
- [4.2] Local adapter thumbnails #36552 (review)
- [4.2] Local adapter thumbnails #36552 (comment)
About the other PR: the approach on that PR is wrong in the sense that you need a data structure that holds all related info per supported media file (mime type, has preview, allows edit, etc). If the project ends up with different inputs for each of all these required data you end up with countless inputs which makes the app really hard to configure and finally use it to its full extend. Mind that since Media manager supports audio and video and other types at some point you might want to provide edit for those (I had a trim plugin for audio and video). Also one more thing: uploads should only allowed if there is a known and matching mime type, the idea that in 2022 people rely on extensions for uploads is a huge security problem. A simple composer require is all it takes: https://packagist.org/packages/league/mime-type-detection to start making things right...

obuisard · 2022-10-11T16:38:50Z

Thank you Dimitris @dgrammatiko. I did address the first review, fixed formatting and tested the code successfully.

As far as the tests are concerned, a solution would be to remove testUpdateFileWithoutAdapter all together, but I would appreciate someone's expertise on this.

dgrammatiko · 2022-10-11T17:44:35Z

a solution would be to remove testUpdateFileWithoutAdapter all together,

@laoneo could you advise here?

laoneo · 2022-10-17T19:37:05Z

Removing the test is a bad idea. Better to set the default to 0, so no thumbs will be generated and the admin has to activate it. If it is on by default, then after the update all thumbnails are generated without notice, which can lead to resource issues on large sites.

By default, thumbnails are not created

Default the creation of thumbnails to 'no'

obuisard · 2022-10-26T21:26:46Z

I have modified the code to make the thumbnail creation 0 by default. It does solve issues with tests. Thank you @laoneo for the suggestion.

@Quy

Removed the creation of the folder since it is addressed in libraries/src/Image/Image.php as suggested by @Quy in the previous PR

jwaisner · 2022-11-08T05:43:22Z

I have tested this item ✅ successfully on 53bc63a

Everything looks good after applying the PR.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38805.}

N6REJ · 2022-11-08T06:17:05Z

I have tested this item ✅ successfully on 53bc63a

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38805.}

thumb_path should not have been renamed

Added the deprecation on createThumbs

changed deprecated version number

obuisard · 2023-01-19T01:23:29Z

Troy @N6REJ and Jacob @jwaisner, would you mind re-testing this so we can merge it? Thank you so much!

jwaisner · 2023-01-19T02:08:52Z

I have tested this item ✅ successfully on d82abd9

Everything retested well. No issues.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38805.}

N6REJ · 2023-01-19T03:40:00Z

I have tested this item 🔴 unsuccessfully on d82abd9

in my test the images were already thumbnailed without the PR. This was " The currently installed Joomla! version is "‎4.3.0-dev"‎4.3.0-dev"
with the latest composer & npm.
plugin setting did not exist w/o the pr as expected.
Media manager image Without PR..

image with pr

original image

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38805.}

obuisard · 2023-01-19T17:34:41Z

in my test the images were already thumbnailed without the PR.

That's strange. The thumbnails created with the PR should be 200x200.

dgrammatiko · 2023-01-19T17:38:04Z

@obuisard what @N6REJ is showcasing is the SIZE of the container DIV, not the size of the image. To get the size of the image open the browser tools, select the image in the elements tab and in the console type $0.naturalWidth (or naturalHeight for the height). False alarm...

obuisard · 2023-01-19T17:40:49Z

@obuisard what @N6REJ is showcasing is the SIZE of the container DIV, not the size of the image. To get the size of the image open the browser tools, select the image in the elements tab and in the console type $0.naturalWidth (or naturalHeight for the height). False alarm...

Oh good catch Dimitris @dgrammatiko! @N6REJ can you double-check? Thank you!

Quy · 2023-01-19T18:17:05Z

Blurry quality:

Before PR:

After PR:

obuisard · 2023-01-19T20:28:50Z

Thank you, Troy @N6REJ for reporting those issues.
The image quality and handling of transparency for GIFs is unrelated to this PR, but improvements need to be made to the image library itself. Something for another PR...

N6REJ · 2023-01-20T00:07:30Z

I have tested this item ✅ successfully on d82abd9

retested as instructed.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38805.}

obuisard · 2023-01-20T00:51:14Z

A big thank you to Dimitris @dgrammatiko and the testers !

carlitorweb · 2023-01-20T04:04:25Z

Documentation added ✔️

dgrammatiko · 2023-01-20T10:15:59Z

@obuisard I haven't checked but is the thumbnails functionality bypasses the svg files? If not it needs to do so, the thumbs helper expects only files that the Image class can handle. I'll try to check the next days if I get some free time

obuisard · 2023-01-20T15:30:48Z

@obuisard I haven't checked but is the thumbnails functionality bypasses the svg files? If not it needs to do so, the thumbs helper expects only files that the Image class can handle. I'll try to check the next days if I get some free time

@dgrammatiko I have checked SVG and image thumbnail creation and both work well together. The image.php file specifies which image file formats can be treated by the GD library .

N6REJ · 2023-01-20T20:48:02Z

it's sad that we are using GD instead of imagemagick.

obuisard · 2023-01-20T21:24:07Z

it's sad that we are using GD instead of imagemagick.

I know, but GD is more widely installed than imagick. It would be great to offer the use of either one of those graphic libraries in Joomla.

dgrammatiko · 2023-01-20T21:28:45Z

You'll be surprised but GD (PHP 8.1+) is (median) faster than imagick. How do I know? I keep testing these for responsive-images

N6REJ · 2023-01-21T03:58:50Z

@dgrammatiko faster but I thought imagick quality was far higher?

obuisard added 6 commits September 21, 2022 19:52

Update plg_filesystem_local.ini

23aef7e

Update Image.php

d2fd2ad

Updated thumbnails creation function

Update local.php

a2dc643

Added thumbs to the adapter

Update local.xml

44b5d3e

Added thumbs field (defaults to true)

Update MediaCest.php

b65ed2d

Commented out lines in testUpdateFileWithoutAdapter

Update LocalAdapter.php

04b4cbb

Added thumbs and thumbSize properties Updated construct, createFile, updateFile, delete, getPathInformation, rglob, checkContent Added getLocalThumbPaths, getThumb, createThumb functions

joomla-cms-bot added Language Change This is for Translators PR-4.3-dev Unit/System Tests labels Sep 22, 2022

obuisard added 3 commits September 21, 2022 20:56

Update LocalAdapter.php

8c4b439

tab removal

Update Image.php

1951162

Removed blank space

Update MediaCest.php

623da04

joomla-cms-bot removed the Unit/System Tests label Sep 22, 2022

Update LocalAdapter.php

83319a6

Replaced all use of mkdir() to Folder::create()

Merge branch '4.3-dev' into pr-36552-clone

03db9e8

obuisard mentioned this pull request Oct 11, 2022

Allow Media Manager to show thumbnails for more file types. #38722

Closed

obuisard added 3 commits October 17, 2022 17:43

Update local.php

17eaa64

By default, thumbnails are not created

Update local.xml

3066a1f

Default the creation of thumbnails to 'no'

Merge branch '4.3-dev' into pr-36552-clone

84a6d92

obuisard added the Documentation Required label Oct 17, 2022

dgrammatiko mentioned this pull request Oct 21, 2022

Fix image selection in media field #31523

Closed

Update LocalAdapter.php

53bc63a

Removed the creation of the folder since it is addressed in libraries/src/Image/Image.php as suggested by @Quy in the previous PR

obuisard added 3 commits November 10, 2022 13:47

Update LocalAdapter.php

958fc7f

thumb_path should not have been renamed

Update Image.php

1e19a9c

Added the deprecation on createThumbs

Update Image.php

e9e6d68

changed deprecated version number

obuisard mentioned this pull request Nov 10, 2022

Update new-deprecations.md for Image joomla/Manual#59

Merged

Merge branch '4.3-dev' into pr-36552-clone

d82abd9

Merge branch '4.3-dev' into pr-36552-clone

bb1a022

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 20, 2023

obuisard added this to the Joomla! 4.3.0 milestone Jan 20, 2023

obuisard merged commit eadc217 into joomla:4.3-dev Jan 20, 2023

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 20, 2023

jgerman-bot mentioned this pull request Jan 21, 2023

[4.3] Local adapter thumbnails (PR 36552 clone) joomlagerman/joomla#2722

Closed

joomlapl-bot mentioned this pull request Jan 21, 2023

[4.3] Local adapter thumbnails (PR 36552 clone) JoomlaPolska/jezyk-J4#305

Closed

obuisard deleted the pr-36552-clone branch September 15, 2023 18:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[4.3] Local adapter thumbnails (PR 36552 clone) #38805

[4.3] Local adapter thumbnails (PR 36552 clone) #38805

obuisard commented Sep 22, 2022 •

edited

Loading

obuisard commented Sep 24, 2022

dgrammatiko commented Oct 11, 2022

obuisard commented Oct 11, 2022

dgrammatiko commented Oct 11, 2022

laoneo commented Oct 17, 2022

obuisard commented Oct 26, 2022

jwaisner commented Nov 8, 2022

N6REJ commented Nov 8, 2022

obuisard commented Jan 19, 2023

jwaisner commented Jan 19, 2023

N6REJ commented Jan 19, 2023 •

edited

Loading

obuisard commented Jan 19, 2023

dgrammatiko commented Jan 19, 2023

obuisard commented Jan 19, 2023

Quy commented Jan 19, 2023

obuisard commented Jan 19, 2023

N6REJ commented Jan 20, 2023 •

edited

Loading

obuisard commented Jan 20, 2023

carlitorweb commented Jan 20, 2023

dgrammatiko commented Jan 20, 2023

obuisard commented Jan 20, 2023 •

edited

Loading

N6REJ commented Jan 20, 2023

obuisard commented Jan 20, 2023

dgrammatiko commented Jan 20, 2023

N6REJ commented Jan 21, 2023 •

edited

Loading

[4.3] Local adapter thumbnails (PR 36552 clone) #38805

[4.3] Local adapter thumbnails (PR 36552 clone) #38805

Conversation

obuisard commented Sep 22, 2022 • edited Loading

Summary of Changes

Testing Instructions

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Link to documentations

obuisard commented Sep 24, 2022

dgrammatiko commented Oct 11, 2022

obuisard commented Oct 11, 2022

dgrammatiko commented Oct 11, 2022

laoneo commented Oct 17, 2022

obuisard commented Oct 26, 2022

jwaisner commented Nov 8, 2022

N6REJ commented Nov 8, 2022

obuisard commented Jan 19, 2023

jwaisner commented Jan 19, 2023

N6REJ commented Jan 19, 2023 • edited Loading

obuisard commented Jan 19, 2023

dgrammatiko commented Jan 19, 2023

obuisard commented Jan 19, 2023

Quy commented Jan 19, 2023

obuisard commented Jan 19, 2023

N6REJ commented Jan 20, 2023 • edited Loading

obuisard commented Jan 20, 2023

carlitorweb commented Jan 20, 2023

dgrammatiko commented Jan 20, 2023

obuisard commented Jan 20, 2023 • edited Loading

N6REJ commented Jan 20, 2023

obuisard commented Jan 20, 2023

dgrammatiko commented Jan 20, 2023

N6REJ commented Jan 21, 2023 • edited Loading

obuisard commented Sep 22, 2022 •

edited

Loading

N6REJ commented Jan 19, 2023 •

edited

Loading

N6REJ commented Jan 20, 2023 •

edited

Loading

obuisard commented Jan 20, 2023 •

edited

Loading

N6REJ commented Jan 21, 2023 •

edited

Loading