? ? Pending

User tests: Successful: Unsuccessful:

avatar dgrammatiko
dgrammatiko
13 Feb 2022

Pull Request is a continuation of #37008, #36613 and #35780

Summary of Changes

  • Render images using the jLayout

Testing Instructions

Check the images render as expected

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Documentation Changes Required

I didn't touch the mod_banners/tmpl/default.php but that one should also be patched

avatar dgrammatiko dgrammatiko - open - 13 Feb 2022
avatar dgrammatiko dgrammatiko - change - 13 Feb 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 13 Feb 2022
Category Front End com_content com_finder com_tags com_users Modules
avatar dgrammatiko dgrammatiko - change - 13 Feb 2022
Labels Added: ?
avatar laoneo
laoneo - comment - 14 Feb 2022

Somehow I find it strange that we can't write a simple image tag anymore in core.

avatar dgrammatiko
dgrammatiko - comment - 14 Feb 2022

Somehow I find it strange that we can't write a simple image tag anymore in core.

Easily fixable:

  • remove remote adapters
  • Remove support for the web spec loading=lazy

and it would be like 2010 😀

avatar laoneo
laoneo - comment - 14 Feb 2022

2010 was not bad, less complicated as now 🤣

avatar dgrammatiko
dgrammatiko - comment - 14 Feb 2022

2010 was not bad, less complicated as now 🤣

I mean you can remove the complicated JLayout and add the same functionality in the sef.php plugin. BUT there's a performance cost, with the JLayout approach you run the code only for images using the media field but with the sef plugin you will run that code for each and every image in the HTML. And since for me User Experience is more beneficial than Developer's experience/convenience thus this PR. The project could very easily destroy any meaningful SEO/ROI by executing code repeatedly for cases that is not required but then again will keep devs that think it's awkward to cater themselves for the output of their code very happy. Not up to me to decide, I just proposed what seems reasonable by my own standards. Feel free to close this and add the code in the sef.php...

avatar bembelimen
bembelimen - comment - 14 Feb 2022

I still would prefere to implement the layout properly in HTMLHelper::image and use this everywhere, it's much more convenient

avatar dgrammatiko
dgrammatiko - comment - 14 Feb 2022

I still would prefere to implement the layout properly in HTMLHelper::image and use this everywhere, it's much more convenient

Probably I would also agreed with this if the HTMLHelper::image had more friendly/memorable arguments:

	/**
	 * Write a `<img>` element
	 *
	 * @param   string        $file        The relative or absolute URL to use for the src attribute.
	 * @param   string        $alt         The alt text.
	 * @param   array|string  $attribs     Attributes to be added to the `<img>` element
	 * @param   boolean       $relative    Flag if the path to the file is relative to the /media folder (and searches in template).
	 * @param   integer       $returnPath  Defines the return value for the method:
	 *                                     -1: Returns a `<img>` tag without looking for relative files
	 *                                     0: Returns a `<img>` tag while searching for relative files
	 *                                     1: Returns the file path to the image while searching for relative files
	 *
	 * @return  string|null  HTML markup for the image, relative path to the image, or null if path is to be returned but image is not found
	 *
	 * @since   1.5
	 */

But that last integer it's a very good lesson for new devs how NOT to assign arguments on a function 😀

Then again this is just code and someone could maybe fix this in a B/C way (we did that for the script/stylesheet functions somewhere around 3.5). That said I still believe the JLayout is the right API for echoing HTML fragments, it's what the project uses everywhere else (all bootstrap, fields, XTDButtons, etc)

avatar simbus82 simbus82 - test_item - 17 Feb 2022 - Tested successfully
avatar simbus82
simbus82 - comment - 17 Feb 2022

I have tested this item ✅ successfully on 4448757

Images are well rendered, i have tested feeds too!


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37022.

avatar jwaisner jwaisner - test_item - 23 Feb 2022 - Tested successfully
avatar jwaisner
jwaisner - comment - 23 Feb 2022

I have tested this item ✅ successfully on 4448757


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37022.

avatar jwaisner
jwaisner - comment - 23 Feb 2022

I have tested this item ✅ successfully on 4448757


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37022.

avatar jwaisner
jwaisner - comment - 23 Feb 2022

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37022.

avatar jwaisner jwaisner - change - 23 Feb 2022
Status Pending Ready to Commit
avatar laoneo laoneo - change - 25 Feb 2022
Labels Added: ?
avatar laoneo laoneo - change - 25 Feb 2022
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2022-02-25 15:53:16
Closed_By laoneo
avatar laoneo laoneo - close - 25 Feb 2022
avatar laoneo laoneo - merge - 25 Feb 2022
avatar laoneo
laoneo - comment - 25 Feb 2022

Thanks!

avatar dgrammatiko
dgrammatiko - comment - 25 Feb 2022

Nice, thanks!

avatar laoneo
laoneo - comment - 25 Feb 2022

Can you also have a look on #37148?

avatar RickR2H
RickR2H - comment - 24 Apr 2022

I have tested this item ✅ successfully on 81a98cf

Tested all views


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37022.

avatar RickR2H RickR2H - test_item - 24 Apr 2022 - Tested successfully
avatar dgrammatiko
dgrammatiko - comment - 24 Apr 2022

@RickR2H this was merged on 25th of February (it's not a pending PR)

Add a Comment

Login with GitHub to post a comment