[#40618] - [5.0] Replace hard coded img element by JLayout joomla.html.image and DRY
- Closed
- 13 Dec 2023
- Medium
- Build: 5.0-dev
- # 40618
- Diff
- hans2103:feature/5.0-dev/replace-img-by-jlayout-for-mod-banners
User tests: Successful: Unsuccessful:
Pull Request for no Issue
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
- Joomla 5 with testing sample data. (adjust mod_banners config and unset the chosen client, otherwise you don't see images... #40616)
- Go to frontend index.php/banners
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
joomla-cms-bot
- | Category | ⇒ | Modules Front End |
| Status | New | ⇒ | Pending |
| Labels |
Added:
PR-5.0-dev
|
||
| Title |
|
||||||
| Labels |
Added:
Feature
|
||
Looks good to me. The $alt seems to be not correct. but this is not in scope of this PR.
code style
@richard67 No man. Having formalized code style doesnt mean that its good code style. Nor does it mean that it should be blindly followed.
code style
@richard67 No man. Having formalized code style doesnt mean that its good code style. Nor does it mean that it should be blindly followed.
@wojtekLs Better than following your comments which are completely useless since you talk about stuff of which you have no knowledge, e.g. our rules.
https://developer.joomla.org/coding-standards/php-code.html
Mixed language usage (e.g. at the layout files)
For layout files and all files where we use a mix of PHP and HTML (all PHP files in the view/tmpl and layout folder) we additionally wrap every line into a block and use the alternative syntax for control structures. This should make the code easier to read and make it easier to move blocks around without creating fatal errors due to missing tags.
@wojtekLs I haven't made these rules. For changing these rules it needs a decision by the production department. We all know that, only you insist on violating these rules because you comment here without having made yourself familiar with our rules.
@richard67 stupid rules are not enforceable. Period.
Why do we have to put to with this troll
Please, can we get the focus on the PR again? The discussion about code styling is out of scope for this issue.
Thank you
The width/height settings are ignored.
Before PR:
<img
src="http://localhost/Joomla_5.0.0-alpha2-Alpha-Full_Package/images/banners/shop-ad-books.jpg#joomlaImage://local-images/banners/shop-ad-books.jpg?width=468&height=60"
alt="Banner"
width="50" height="50" >
After PR:
<img src="http://localhost/Joomla_5.0.0-alpha2-Alpha-Full_Package/images/banners/shop-ad-books.jpg" alt="Banner" width="468" height="60" loading="lazy">
This pull request has been automatically rebased to 5.1-dev.
The width/height settings are ignored.
Before PR:
<img src="http://localhost/Joomla_5.0.0-alpha2-Alpha-Full_Package/images/banners/shop-ad-books.jpg#joomlaImage://local-images/banners/shop-ad-books.jpg?width=468&height=60" alt="Banner" width="50" height="50" >After PR:
<img src="http://localhost/Joomla_5.0.0-alpha2-Alpha-Full_Package/images/banners/shop-ad-books.jpg" alt="Banner" width="468" height="60" loading="lazy">
are they? The attributes width and height are still there.
| Labels |
Added:
PR-5.1-dev
Removed: PR-5.0-dev |
||
| Category | Modules Front End | ⇒ | Unit Tests Repository Administration com_admin com_content com_contenthistory com_fields com_finder com_installer com_joomlaupdate com_media NPM Change JavaScript com_menus com_messages com_templates com_users com_workflow |
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-12-13 14:47:46 |
| Closed_By | ⇒ | hans2103 | |
| Labels |
Added:
Unit/System Tests
NPM Resource Changed
|
||
Maybe you should first study our code style before making recommendations as if you were a long term contributor.