Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.1] Replace hard coded img element by JLayout joomla.html.image and DRY #42508

Merged

Conversation

hans2103
Copy link
Contributor

Pull Request for no Issue
This PR replaces #40618
After solving the merge conflicts there are just too many changed files.

Summary of Changes

This PR will change the hard coded <img src=""... inside the default view of mod_banners by a call to JLayout joomla.html.image and keeps it DRY.

Testing Instructions

Actual result BEFORE applying this Pull Request

You will see a banner image on the page.

Expected result AFTER applying this Pull Request

You will see a banner image on the page. The same as BEFORE. The HTML output is also the same as BEFORE.
The only difference is that it is rendered by JLayout which allows users to create an override.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

Co-authored-by: Quy <quy@nomonkeybiz.com>
@Quy
Copy link
Contributor

Quy commented Dec 15, 2023

I have tested this item ✅ successfully on a1ba0fa


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

@brianteeman
Copy link
Contributor

The only difference is that it is rendered by JLayout which allows users to create an override.

Am I being daft here but you could always create an override for the module output. But now if you want to create an override then it will impact not just the image layout in the module but everywhere that the image layout is being used.

@hans2103
Copy link
Contributor Author

hans2103 commented Jan 3, 2024

The only difference is that it is rendered by JLayout which allows users to create an override.

Am I being daft here but you could always create an override for the module output. But now if you want to create an override then it will impact not just the image layout in the module but everywhere that the image layout is being used.

isn't that great?
The JLayout perfect spot to implement a cdn for images.
It is up to the JLayout how images should be rendered, not the module.
The module should forward the information to the JLayout to render the image.

@Hackwar Hackwar added Feature PBF Pizza, Bugs and Fun labels Feb 20, 2024
@ufuk-avcu
Copy link

I have tested this item ✅ successfully on a1ba0fa


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

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 24, 2024
@reDimDev
Copy link

I have tested this item ✅ successfully on 0b6155a


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

@bembelimen bembelimen merged commit dcebae6 into joomla:5.1-dev Feb 28, 2024
0 of 2 checks passed
@bembelimen
Copy link
Contributor

Thanks

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 28, 2024
@bembelimen bembelimen added this to the Joomla! 5.1.0 milestone Feb 28, 2024
@hans2103 hans2103 deleted the feature/5.1-dev/replace-img-by-jlayout branch March 3, 2024 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature PBF Pizza, Bugs and Fun PR-5.1-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants