[#35250] - [5.3] [GSoC 21] Media Manager - Responsive Images final version and "Insert/Edit Image" form improvements
- Pending
- Medium
- Build: 4.1-dev
- # 35250
- Diff
- eyvazahmadzada:responsive-images-final
User tests: Successful: Unsuccessful:
Pull Request for the "Responsive Images and Insert/Edit Image form improvements" task of the final evaluation period of Media Manager.
The final version of #34803.
Summary of Changes
... First PR's changes
- Improve "Insert/Edit Image" form. While adding an image to the content editors from the media manager, it was possible to add some image details (image description and class, figure caption and class, lazy loading configuration), but there was no option in the “Insert/Edit Image” to change these details thereafter. To improve it, I added extra form controls to the "Advanced" section of the form which allows users to update image details and to customize responsive sizes for any image.
- Improve responsive image size customization. Responsive image size customization forms previously had only 2 fields: width and height. So the sizes didn't have an identifier and images are cropped by a default creation method. To make the functionality fully customizable, I added two additional fields for customizing the image creation method and the size title.
- Implement functions to detect unused responsive image versions. I added the functions that compare initial and final versions of web content and detect the images that are not being used anymore. Currently, an alert that contains the names of the images gets popped up, but I’m planning to implement a modal where users can select and remove these images with just a few clicks after GSoC.
Testing Instructions
... Follow the same steps as the first PR.
- Additional instructions. Responsive sizes and image details can be edited by right-clicking on the image -> Image in content editors.
Documentation Changes Required
ResponsiveImagesHelper, Image and HTMLHelper class methods must be added or updated.
Special thanks to my mentors (@sebenns, @fancyFranci, @GeraintEdwards, @shivamdiehard, Chris Keen) and @bembelimen for assisting me with the project.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_admin com_banners com_categories com_contact com_content com_newsfeeds com_tags Language & Strings Repository JavaScript NPM Change Front End Layout |
To improve it, I added extra form controls to the "Advanced" section of the form
I'm wondering why this is in the advanced section? This entire section can be disabled in the tinymce plugin
@eyvazahmadzada nice work but I have fundamental objections with the implementation that your mentors asked you to do here.
- Responsive images SHOULD keep the same aspect ratio
- Responsive images are artefacts so they SHOULDN'T be store next to the original image
- The addition of more input fields per form is totally anachronistic, Joomla should move to a separation of concerns (data/layout)
- The changes in the tinyMCE image should be a PR upstream
Also not really related with this PR but:
- images as all the other static assets needs to be versioned
| Labels |
Added:
Language Change
NPM Resource Changed
?
|
||
| Category | Administration com_admin com_banners com_categories com_contact com_content com_newsfeeds com_tags Language & Strings Repository JavaScript NPM Change Front End Layout | ⇒ | Administration com_admin com_banners com_categories com_contact com_content com_newsfeeds com_tags Language & Strings Repository JavaScript NPM Change Front End Layout Libraries Modules |
To improve it, I added extra form controls to the "Advanced" section of the form
I'm wondering why this is in the advanced section? This entire section can be disabled in the tinymce plugin
I thought setting custom responsive versions for an image would be an advanced action, no?
@eyvazahmadzada nice work but I have fundamental objections with the implementation that your mentors asked you to do here.
- Responsive images SHOULD keep the same aspect ratio
- Responsive images are artefacts so they SHOULDN'T be store next to the original image
- The addition of more input fields per form is totally anachronistic, Joomla should move to a separation of concerns (data/layout)
- The changes in the tinyMCE image should be a PR upstream
Also not really related with this PR but:
- images as all the other static assets needs to be versioned
By default, images are created by resizing, so they actually keep their aspect ratio. But this can be customized for each image.
After applying the PR and without doing anything else.
Notice
: Undefined property: stdClass::$image_intro_sizes in
C:\htdocs\joomla-cms\layouts\joomla\content\intro_image.php
on line
30
Notice
: Undefined property: stdClass::$image_intro_size_options in
C:\htdocs\joomla-cms\layouts\joomla\content\intro_image.php
on line
30
Notice
: Undefined property: stdClass::$image_intro_method in
C:\htdocs\joomla-cms\layouts\joomla\content\intro_image.php
on line
30
Notice
: Trying to get property 'params' of non-object in
C:\htdocs\joomla-cms\libraries\src\Helper\ResponsiveImagesHelper.php
on line
345
Notice
: Trying to get property 'params' of non-object in
C:\htdocs\joomla-cms\libraries\src\Helper\ResponsiveImagesHelper.php
on line
386
After applying the PR and without doing anything else.
Notice : Undefined property: stdClass::$image_intro_sizes in C:\htdocs\joomla-cms\layouts\joomla\content\intro_image.php on line 30 Notice : Undefined property: stdClass::$image_intro_size_options in C:\htdocs\joomla-cms\layouts\joomla\content\intro_image.php on line 30 Notice : Undefined property: stdClass::$image_intro_method in C:\htdocs\joomla-cms\layouts\joomla\content\intro_image.php on line 30 Notice : Trying to get property 'params' of non-object in C:\htdocs\joomla-cms\libraries\src\Helper\ResponsiveImagesHelper.php on line 345 Notice : Trying to get property 'params' of non-object in C:\htdocs\joomla-cms\libraries\src\Helper\ResponsiveImagesHelper.php on line 386
First I thought that happened because the plugin wasn't installed or activated but then I completely reset my site to default to make sure of that. The weird thing is that this error didn't show up in any case
| Labels |
Added:
?
Removed: ? |
||
After applying the PR and without doing anything else.
Notice : Undefined property: stdClass::$image_intro_sizes in C:\htdocs\joomla-cms\layouts\joomla\content\intro_image.php on line 30 Notice : Undefined property: stdClass::$image_intro_size_options in C:\htdocs\joomla-cms\layouts\joomla\content\intro_image.php on line 30 Notice : Undefined property: stdClass::$image_intro_method in C:\htdocs\joomla-cms\layouts\joomla\content\intro_image.php on line 30 Notice : Trying to get property 'params' of non-object in C:\htdocs\joomla-cms\libraries\src\Helper\ResponsiveImagesHelper.php on line 345 Notice : Trying to get property 'params' of non-object in C:\htdocs\joomla-cms\libraries\src\Helper\ResponsiveImagesHelper.php on line 386
Hi @brianteeman, could you please provide me with the steps to reproduce this issue? I installed Joomla from scratch on my new laptop and still didn't get this error.
| Category | Administration com_admin com_banners com_categories com_contact com_content com_newsfeeds com_tags Language & Strings Repository JavaScript NPM Change Front End Layout Libraries Modules | ⇒ | Unit Tests Repository Administration com_admin SQL Postgresql |
This PR has been destroyed and the merge has been done incorrectly leading to over 1000 files being changed ... This PR needs some reverting before progress can be made.
| Labels |
Added:
?
Removed: Language Change NPM Resource Changed |
||
| Category | Administration com_admin Repository Unit Tests SQL Postgresql | ⇒ | Administration com_admin com_banners com_categories com_contact com_content com_newsfeeds com_tags Language & Strings Repository JavaScript NPM Change Front End Layout Libraries Modules |
This pull request has automatically rebased to 4.2-dev.
This pull request has been automatically rebased to 5.0-dev.
This pull request has been automatically rebased to 5.1-dev.
This pull request has been automatically rebased to 5.2-dev.
| Title |
|
||||||
This pull request has been automatically rebased to 5.3-dev.
| Title |
|
||||||
Why have you created unique language strings for every component. If the strings are always the same then you should just define and use a global string