[#46598] - [5.4] mod_articles image
- Pending
- Medium
- Build: 5.4-dev
- # 46598
- Diff
- brianteeman:article
User tests: Successful: Unsuccessful:
Summary of Changes
Make sure the image is displayed AND ensure there are no empty list elements
Testing Instructions
Create a mod_articles module and set it to display just the article image - ensure one of the articles that would be displayed does NOT have an image
Actual result BEFORE applying this Pull Request
module is empty except for empty list elements
Expected result AFTER applying this Pull Request
image is displayed for all articles that have an image AND there are no empty list elements
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
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Modules Front End |
The more I think about it the more I think that the if statement should just be removed completely. It doesnt serve any purpose (that i can see) and will just result in an empty list item which is invalid thml
Tested, but: none of the articles has an image so I end up with a list of five empty list items.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46598.
yes thats why I commented
The more I think about it the more I think that the if statement should just be removed completely. It doesnt serve any purpose (that i can see) and will just result in an empty list item which is invalid thml
or the if statement should be moved before the containing li element
@drmenzelit I'd appreciate your thoughts on moving the if statement
If we remove the if statement completely only the plugin events ($item->event->afterDisplayTitle, etc.) will be displayed, if all other elements are set to hide.
Movin the if statement before the li element can end in an empty ul. Probably the better solution.
| Labels |
Added:
PR-5.4-dev
|
||
I have updated the PR and description/test instructions so that the if statement now wraps the li element instead of being inside the li. This additionally ensure that we have no empty list elements which would be both invalid html and an accessibility failure
I have tested this and found a problem: If there is no image there is an empty list item. This part: || $params->get('img_intro_full') checks whether to use the intro or full image but it does not check that either image actually exists.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46598.
I have tested this item ✅ successfully on 985771f
I have successfully tested this PR. Nice catch @brianteeman and thank you for the fix.
Happy Holidays!
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46598.