Filter button should not appear when no comments are available in list

[swissspidy]

Previously: #37407. See also: #38341. Both tickets contain some screenshots that demonstrate the issue.

It doesn't make sense to be able to filter the comments list table when there are are no (trashed/spam) comments available.

Maybe there are other list tables that might need to be checked as well.

[Jim_Panse]

Patch for #40188

[Jim_Panse]

Not quite sure if i did it the right way. My first patch, so have some patience with me, please. And guide me into the right direction, if i forgot something.

[swissspidy]

@Jim_Panse Thanks for your patch.

Please note that it also hides the filter button after applying a filter. For example, when I have 1 regular comment on my blog and I change the filter from "All comment types" to "Pings", the filter button now is hidden and I can't change the filter again.

[Jim_Panse]

@swissspidy Attached a new patch file and added a helper function. Please have a look, thank you!

[swissspidy]

@Jim_Panse It looks like your patch is reversed (what is red should be green).

A helper method shouldn't be needed though. Why not simply use get_comments( array( 'number' => 1 ) ) in the condition?

[Jim_Panse]

Ups, new Patch attached.

I think it makes for a much cleaner code, if i add the helper function instead of calling get_comments directly in the if statement.

[swissspidy]

It's really not needed though.

$comments = get_comments( array( 
  'number' => 1, 
) ); 
if( $comments ) {
  return true; 
} else { 
  return false; 
}

is a worse version of

return get_comments( array( 
  'number' => 1, 
) );

This means you can just as well use

if ( 'top' === $which && get_comments( array( 'number' => 1 ) ) ) { … }

which is way cleaner.

[Jim_Panse]

You are right, my bad. New Patch attached. :)

[menakas]

A thought, though I haven't deep dived into the code to find its feasibility:

Maybe there are other list tables that might need to be checked as well.

@swissspidy concern is valid. This is a feature common to all list tables (WP_List_Table) i.e, all lists should display the Filter option if and only if there is at least one list item.

In fact, in the comments list table, this functionality should reflect in all sub lists - All, Pending, Approved, Spam and Trash. Likewise, in other lists like posts, categories, tags and so on.

With this in mind, a solution at the WP_List_Table class level would be better.

[DrewAPicture]

Assigning ownership to mark the good-first-bug as "claimed".

[akbarhusen429]

fixes issue

[akbarhusen429]

fixes issue

[swissspidy]

@akbarhusen429 Have you tested this? Because there's no has_comments() method in WP_Comments_List_Table(). That means your code doesn't work.

There was such a method in 40188.2.patch​, but as per prior comments it was not really needed.

Also see another comment above:

This is a feature common to all list tables (WP_List_Table) i.e, all lists should display the Filter option if and only if there is at least one list item.
With this in mind, a solution at the WP_List_Table class level would be better.

[dinhtungdu]

I think we really encounter the problem with Comment type ( Comment and Ping) as @swissspidy said before

Please note that it also hides the filter button after applying a filter. For example, when I have 1 regular comment on my blog and I change the filter from "All comment types" to "Pings", the filter button now is hidden and I can't change the filter again.

Why a user need to see a "Ping" option when there isn't any pingback or trackback. We should check the number of comment or ping before display the filter option for it respectively.
Others like post, user, plugin doesn't have this problem.
From here, we can hide the extra_tablenav which contains Filter dropdown when there isn't any item.

[dinhtungdu]

Hide entry table nav when there is no item

[birgire]

In 40188.7.patch​

• we introduce a new comment_status_dropdown() method of WP_Comments_List_Table, to make it more readable, in a similar way as the categories_dropdown() method was introduced in WP_Posts_List_Table.

• the select option's content is escaped, just as in the walker_pagedropdown().

• we test if the filter admin_comment_types_dropdown returns an empty array, to make sure that no empty <select> dropdown is displayed.

• only display a status in the dropdown with a get_comments( array( 'number' => 1, 'type' => $type ) check, as previously suggested.

I like the idea to remove the table nav for no comments in WP_List_Table, like:

protected function display_tablenav( $which ) {
    if( ! $this->has_items() ) {
        return;
    }
    ...

but it sounded out of scope according to the current ticket title, so I did a smaller step instead, in WP_Comments_List_Table only, similar to the approach in #37407

Added a test, based on the ones in #38341

• test_filter_button_should_not_be_shown_if_there_are_no_comments()

and then two more:

• test_filter_button_should_be_shown_if_there_are_comments()
• test_filter_comment_status_dropdown_should_be_shown_if_there_are_comments()

It was a real headache to make the last two work as expected, because after lot of digging I found out that other tests where modifying the $_REQUEST global and that was being picket up here by our WP_Comments_List_Table instance. That meant the offset ended up as 120 and no comments where returned ;-)

So I will create a new ticket to fix these older tests, so currently our tests are blocked by that.

Update: See ticket #42044 addressing that.

[davidbaumwald]

The latest patch no longer applies cleanly to trunk, so marking this for a refresh. Also, with the new function, this deserves a call-out on the Misc Dev Note.

[rebasaurus]

Refresh

[whyisjake]

There is a pretty big forehead on the page if the filter buttons are missing. Should we add some CSS to make it look a little nicer?

I might be over thinking this.

[whyisjake]

In 48521:

Comments: Don't show the filter/pagination actions if there are no comments to list.

It doesn't make sense to be able to filter the comments list table when there are are no (trashed/spam) comments available.

Fixes #40188.
Props swissspidy, Jim_Panse, menakas, akbarhusen429, dinhtungdu, birgire, SergeyBiryukov, davidbaumwald, rebasaurus, whyisjake.

[whyisjake]

Missed pressing submit on the comment, but 40188.3.diff is adding a check to prevent the top actions nav from being shown. This gets rid of the big forehead on the page.

[SergeyBiryukov]

if ( ! empty( $output ) && $this->has_items() )

It's not quite clear why this check doesn't use the $has_items static variable declared above, while the next check does use the variable:

if ( ( 'spam' === $comment_status || 'trash' === $comment_status ) && current_user_can( 'moderate_comments' ) && $has_items ) {

If that's intentional, it should be documented.

It's also worth noting that class-wp-posts-list-table.php and class-wp-ms-sites-list-table.php don't check $this->has_items(), just ! empty( $output ). Would be nice to bring some consistency here.

[desrosj]

Reopening this to discuss/address the points raised in comment:23.

[desrosj]

I'm also going to exclude this from the miscellaneous developer note. Even though there is a new method being introduced, there is not much here for developers to be aware of.

[whyisjake]

@SergeyBiryukov 40188.4.diff​ attached should make the conditionals the same.

It's also worth noting that class-wp-posts-list-table.php and class-wp-ms-sites-list-table.php don't check $this->has_items(), just ! empty( $output ). Would be nice to bring some consistency here.

Moving this over to #50815 so that it isn't blocked against the 5.5 milestone.

[whyisjake]

If we can get a review here, we can address this little bit of cleanup in 5.5 RC2 today.

[SergeyBiryukov]

In 48722:

Comments: Use the existing static variable instead of calling ::has_items() again in WP_Comments_List_Table::extra_tablenav().

Additionally, removed unnecessary esc_html() on the Filter button label. Core translations are considered safe, and this label is not escaped in any other instance.

Props whyisjake, SergeyBiryukov.
Fixes #40188.

[SergeyBiryukov]

Reopening for the 5.5 branch.

[SergeyBiryukov]

[48722] caused unit test failures:

There were 2 failures:

1) Tests_Admin_includesListTable::test_filter_button_should_be_shown_if_there_are_comments
Failed asserting that '<div class="alignleft actions"></div>' contains "id="post-query-submit"".

/var/www/tests/phpunit/tests/admin/includesListTable.php:316

2) Tests_Admin_includesListTable::test_filter_comment_status_dropdown_should_be_shown_if_there_are_comments
Failed asserting that '<div class="alignleft actions"></div>' contains "id="filter-by-comment-type"".

/var/www/tests/phpunit/tests/admin/includesListTable.php:338

Reverting for now, let's clean this up in #50815.

[SergeyBiryukov]

In 48724:

Comments: Restore the ::has_items() call in WP_Comments_List_Table::extra_tablenav() for now, until unit tests are updated accordingly.

Follow-up to [48722].

Fixes #40188. See #50815.

[SergeyBiryukov]

Replying to SergeyBiryukov:

Reopening for the 5.5 branch.

For reference, [48722] and [48724] are in trunk only and should not be backported to the 5.5 branch.

[SergeyBiryukov]

In 48741:

Comments: Remove a few more unnecessary instances of esc_html() in WP_Comments_List_Table::comment_status_dropdown().

Core translations are considered safe, and these labels are not escaped in any other instances.

Follow-up to [48521], [48722], [48724].

See #40188, #50815.