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

Added option to not count empty modules in countModules and getModules #7606

Closed
wants to merge 1 commit into from
Closed

Added option to not count empty modules in countModules and getModules #7606

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Aug 1, 2015

This is an adapted PR of #7594

Templates use the $this->countModules('myposition') to determine whether or not to show a position (and surrounding html).
However, when a module is empty (no content), the countModules is not affected.
This can result in positions being shown with no modules in it.

This PR adds the option to not include empty modules in that count.
This can be done by adding false as second argument (setting $include_empty to false):
$this->countModules('myposition', false)

Testing Instructions

Easiest way to test with an empty module is to use Advanced Module Manager, as that will make the content truly empty even if the module has an empty <p></p> tag.
So just make a Custom HTML Module and leave the content area empty. Assign it to a module position (that has no other modules assigned to it). Like manually entering testempty in the module position field. And switch on the 'Hide if Empty' option.

Or create your own module that returns an empty content.

Then in your template place this somewhere:

<?php
echo "\n\n<pre>==========================\n";
echo 'Real module count for position "testempty": ' . $this->countModules('testempty');
echo "\n==========================\n";
echo 'Not-empty module count for position "testempty": ' . $this->countModules('testempty', false);
echo "\n==========================</pre>\n\n";
?>

Opinion

In my opinion the default for $include_empty should be false.
As people are saying in #7594 that would change behaviour. That is true, but I believe it only changes it in edge cases and only when those edge cases don't want the empty modules to be counted.
However, that isn't worth my time to fight for, so hope this 'addition' instead of a 'fix' does get accepted.
Then it is up to the template developers to implement it.

@Fedik
Copy link
Member

Fedik commented Aug 1, 2015

Well, here still something wrong with the logic,
first need to define what is "empty module",
because from point of view if module exist it cannot be empty, because it have at least id 😄

And on time when someone call countModules in the template index.php there not exists any module that has content, except custom html module ... "menu module", "latest articles module" and other, do not have content on this point of time.

I tested in joomla test installation and in the protostar template:

var_dump($this->countModules('position-7')); // return 5 (there 5 menu modules)
var_dump($this->countModules('position-7', false)); // return zero

so, from my point of view such changes will confuse people a lot more than before

@ghost
Copy link
Author

ghost commented Aug 1, 2015

Yep, I see what you mean.
Oh well, seems like what I want is not possible with how Joomla core goes about module collection and rendering.

I'll not waste any more of my and others time on this.

Thanks for testing.

Closed.

@ghost ghost closed this Aug 1, 2015
@ghost ghost deleted the module.count branch October 27, 2015 21:11
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants