That's a good idea. However may I suggest to do it already in the main module file and not in the layout?

Just check if $list contains anything and only load the layout then, otherwise return.
We do that already in some modules I think.

Strange definition of DRY!

@Bakual I don't think it should be within the module file itself, but the layout instead. From a logical point, there is something to say for both variations. But putting it in the module file, and not the layout, is actually limiting: If somebody wants to display the message "No items found", the current approach offers a solution - the approach of using the main module file does not.

@sovainfo How could a discussion on DRY improve this pull request?

Agree with your solution, not the argument you use. When the template would be called three times it would require to test at those three times whether the list is empty before calling it. With the test in the template there would only be one test. I know this is unlikely, but it is the principle that matters.

Disagree because a function should not play dummy and depend on the caller to only call when needed or provide the correct input. Expect the function to produce relevant output. Whether that includes something like "No items found" depends on the situation. This can be handled at both sides. When you choose to do that at the called function, it doesn't mean there can't be situations where you would require it at the caller. It is not that simple.

I understand your points. But I think we should not get into a large discussion. My pull request fixed the problem of an empty module instance, where the module title is showing but the content is empty.

The question whether this should be in the module file or in the layout file leads to an important discussion. There seems to be some disagreement already in the core regarding this, and I think a discussion on this therefor should not be taken lightly. It is probably needed to discuss this on the Google Discussion Groups instead, so that there is a solid vision of where this kind of logic should be placed.

As for now, my pull request does not mingle in with this discussion, but just continues the path we are already on. It can be tested and merged - at least in my opinion.

Modules that stop output in the layout file:

• modules/mod_articles_archive/tmpl/default.php
• modules/mod_tags_popular/tmpl/default.php
• modules/mod_users_latest/tmpl/default.php

Modules that stop output in the main file:

• modules/mod_articles_category/mod_articles_category.php
• modules/mod_related_items/mod_related_items.php
• modules/mod_tags_similar/mod_tags_similar.php
• modules/mod_weblinks/mod_weblinks.php

Modules that don't have a solution at all:

• modules/mod_articles_categories/tmpl/default.php
• modules/mod_articles_latest/tmpl/default.php

So I completely agree with you that this is a mess. I'm just saying that my pull request is not ment to straighten out that mess. It's only there to offer a solution if there is none yet.

@Yireo I'm fine with either way.

Let's get it tested and see if it works

I opened up a discussion here: https://groups.google.com/forum/#!topic/joomla-dev-cms/pGFO_aqIBTQ Please feel free to participate in the discussion.

Testing on this single pull request can still take place though.

#test Issue confirmed, PR resolves the issue.
Confirm that Title and <ul> only appear when there is content.

I'm really not sure about this - I can see the support emails now. "I've created and published the module but it doesn't appear!". Surely its really a very obscure edge case to have a popular articles module without any articles on a live site.

This comment was created with the J!Tracker Application at http://issues.joomla.org/.

@test

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

Looks like there's a merge conflict.

@uyenvu could you clarify if you were able to successfully apply the proposed code to your test site?

Maybe there isn't a merge conflict with 3.3.7, but rather just with staging.

hi Nicksavov

I used Joomla! Patch Tester component and it was successfully applied the proposed code to the test site.

I tested with the Joomla patch tester too and can confirm that a module that does not contain articles, is not shown.

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

Great that we have multiple good tests thanks.

I am still concerned that this might not be something we want to do

I'm really not sure about this - I can see the support emails now. "I've created and published the module but it doesn't appear!". Surely its really a very obscure edge case to have a popular articles module without any articles on a live site.

Setting to needs review

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

BTW: it's not the module 'Most Popular Articles' but 'Most Read Content'.

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

@lianneholland confusing I know but in the code it is called mod_articles_popular and in the english text that you see it is called Most Read Content but they are the same thing

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

I agree with @brianteeman that not showing the module can be confusing for users. What I would do is to show the "No articles found" message. This way users can see that there are no items and disable the module if they really don't want to show it.

@yireo can you please rebase the PR to fix the conficts and add the else statement to display the "No articles found message"?

Thanks!

Great idea @phproberto now why didnt I think of that

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

@yireo Could you update your PR with the suggestion of @phproberto ? Thanks.

@yireo This PR is quite old now and the request from @roland-d has gone answered. It will be automatically closed in a few weeks if the PR is not updated as requested