Hmm. We are repeating the same code again and again. Is there a way to add a new helper like lazyImage to handle this?

Is there a way to add a new helper like lazyImage to handle this?

My approach would be to add a new JLayout and not another HTMLHelper thing. The first is very approachable to any front ender the second requires writing a plugin to override it. Basically, the HTMLHelper (s) should be used only for logic/processing and never for spitting out HTML.

My 2c

That should work, too. If we do not have to repeat that same code again and again, and have a way for third party developers to re-use the code, we should be fine.

@dgrammatiko Reading the above discussion it's not 100% clear for me: Is this PR ready for testing now, or do we have to wait for some changes?

Reading the above discussion it's not 100% clear for me: Is this PR ready for testing now, or do we have to wait for some changes?

The changes are valid but also it's valid that at the current state we have a lot of duplicate code. I will add a new layout for rendering images and use it core wide

I will add a new layout for rendering images and use it core wide

@dgrammatiko In this PR here or in another, future one?

@richard67 in this PR
@joomdonation is this any better?

@dgrammatiko I still have a question about the code. To render an image, we are using this code

echo LayoutHelper::render(
		'joomla.html.image',
		[
			'image' => [
				'src' => $item->imageSrc,
				'alt' => $item->imageAlt,
				'attributes' => []
			],
		]
	);

I wonder why we need to use 'image' key here? Shouldn't something like below is faster (to write):

echo LayoutHelper::render(
		'joomla.html.image',
		[		
			'src' => $item->imageSrc,
			'alt' => $item->imageAlt,
			'attributes' => []			
		]
	);

And for additional attributes like alt, attributes, can we not pass it if the data is not available?

Not really an issue with code. Just some questions from me.

I wonder why we need to use 'image' key here? Shouldn't something like below is faster (to write)

Yes having just the attributes for the image element would be easier, the reason I used the key ‘image’ has more to do with future expanding the layout to render a figure with caption and/or picture with source nested elements (responsive images). Having the values under a named key will make that easier, instead of naming the keys with funny names . That said jlayouts support sublayouts so in the future this could be a sub layout of figure?
That’s a valid question and I have no clue if this is better than using a plain array and deal with the figure/picture using sublayouts

Without seeing the actual code, I'm unsure if it is good or bad. I think my questions about optional attributes still valid, with or without using image key

And for additional attributes like alt, attributes, can we not pass it if the data is not available?

Could anyone else looks at the current implementation before we doing real test? @Fedik ?

I agree with @joomdonation #35780 (comment)
Make it simple ?
In the future no one will remember why it made like that.

For picture we can check for source variable or have another layout, kind of joomla.html.picture, when it will be need.
(In my code I use similar blabla.html.image and blabla.html.picture)

@joomdonation @Fedik is this version any better?

yes and no ;)

Please move htmlspecialchars stuff inside image layout,
there you can use $this->escape($blabla);

This way we can be sure that the values is always escaped,

@Fedik are the changes ok now?

Looks good to me :)

I have tested this item ✅ successfully on 4c54bb5

Tested with category blog.

Image url before patch:

Image url after patch:

In other views (module newsflash or contacts) I don't see changes in the image url.

Please see the alt attribute.

<img src="/Joomla_4.0.6-dev+pr.35780-Development-Full_Package/images/sampledata/cassiopeia/nasa2-400.jpg" itemprop="image" alt="alt=&quot;&quot;">

@Quy it should fine now, thanks

@dgrammatiko please can you help me with testing instructions? I'm happy to test but I don't understand how to. The previous issue you linked to mentions we can see images before hand. What should I be looking for? I don't want to accept a test by accident with incorrect testing.

@uglyeoin I've updated the testing instructions, hopefully it's easy to follow

For Article Category Blog, no width, height, and loading attributes.

Before PR:

<figure class="left item-image">
	<img src="/joomla-cms-4.0-dev/images/sampledata/fruitshop/tamarind.jpg"
			 			 itemprop="image"
			width="375" height="488" loading="lazy"	/>
			<figcaption class="caption">Full Caption</figcaption>

After PR:

<figure class="left item-image">
	<img src="/joomla-cms-4.0-dev/images/sampledata/fruitshop/tamarind.jpg" itemprop="image" alt="">			<figcaption class="caption">Full Caption</figcaption>
	</figure>

For Article Category Blog, no width, height, and loading attributes.

That was a bug, I thought I knew the API I introduced many months ago but I was wrong. My bad. It should be fine now

For Contact Categories, on the front end:

<img src="/joomla-cms-4.0-dev/images/3.webp#joomlaImage://local-images/3.webp?width=1280&height=720" >

Please resolve conflicts.

I have tested this item ✅ successfully on 4c54bb5

Thank you!!!

@Quy thanks for all the fixes and the tests

I have tested this item ✅ successfully on 4c54bb5

RTC

@wilsonge

I have tested this item ✅ successfully on 4c54bb5

I have tested this item ✅ successfully on 4c54bb5

By the way, tested also with DPMedia and the hash is removed.

By the way, tested also with DPMedia and the hash is removed.

That was the whole point of this PR: provide one simple JLayout to fix the hash problem universally. Now we need to backport it to 3.x and ask every DEV to render all their images using this layout...

Thanks!