WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#37407 closed defect (bug) (fixed)

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

Reported by: juhise Owned by: swissspidy
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.5.3
Component: Administration Keywords: has-ux-feedback has-screenshots has-patch has-unit-tests commit
Focuses: ui Cc:
PR Number:

Description

When no posts are available in list then filter button should not appear like bulk action button. This filter button does not do anything, it just confuses the user.

Attachments (11)

Page__list_before_patch.png (32.7 KB) - added by juhise 3 years ago.
Post_list_before_patch.png (24.9 KB) - added by juhise 3 years ago.
After_Patch.png (13.9 KB) - added by juhise 3 years ago.
37407.patch (1.1 KB) - added by juhise 3 years ago.
37407.diff (1.0 KB) - added by swissspidy 3 years ago.
37407.2.diff (1.9 KB) - added by swissspidy 3 years ago.
37407.3.diff (1.5 KB) - added by swissspidy 3 years ago.
37407.4.diff (2.1 KB) - added by dipesh.kakadiya 3 years ago.
changes With test case
37407.5.diff (2.1 KB) - added by dipesh.kakadiya 3 years ago.
Updated test case
37407.6.diff (3.1 KB) - added by swissspidy 3 years ago.
37407.7.diff (2.9 KB) - added by dipesh.kakadiya 3 years ago.
minor typo 'are_now_posts' => 'are_not_posts' for testing functions

Download all attachments as: .zip

Change History (26)

@juhise
3 years ago

@juhise
3 years ago

#1 @Kenshino
3 years ago

  • Keywords ui-feedback ux-feedback has-patch has-screenshots added

#2 @karmatosed
3 years ago

  • Keywords commit added; ui-feedback removed

Thanks for this @juhise, you are absolutely right that showing a filter when there is nothing to filter isn't a good idea. I'm going to tag and recommend this is committed as your patch looks good to me.

Last edited 3 years ago by karmatosed (previous) (diff)

#3 @juhise
3 years ago

@karmatosed

Thanks for considering it.

#4 @karmatosed
3 years ago

  • Keywords has-ux-feedback added; ux-feedback removed

@melchoyce would you consider this as a good commit to get in? I think it could be.

#5 @karmatosed
3 years ago

  • Milestone changed from Awaiting Review to 4.7

@michaelarestad can you please commit this as the code and patch look good to me and be great to get this one in.

#6 @melchoyce
3 years ago

@karmatosed Maybe a dev committer could look at this? I'm not comfortable committing PHP.

@swissspidy
3 years ago

#7 @swissspidy
3 years ago

37407.diff simplifies the code and fixes the indentation. Otherwise it looks good to me.

Note that this changes behaviour for anyone hooking into the manage_posts_extra_tablenav action and similar hooks. Probably neglectable though.

#8 @karmatosed
3 years ago

@swissspidy are you cool about committing this then? Would be great to see this get in from the good work @juhise has done.

@swissspidy
3 years ago

#9 @swissspidy
3 years ago

  • Keywords has-unit-tests added
  • Owner set to swissspidy
  • Status changed from new to assigned

#10 @swissspidy
3 years ago

  • Keywords needs-patch needs-unit-tests added; has-patch commit has-unit-tests removed

@karmatosed Unfortunately there's one big problem with the current patch: when I filter posts by a category with zero posts attached to it, the tablenav gets hidden.

To fix the original issue, the filter button without a dropdown, the whole if ( $this->has_items() ) check needs to be added to WP_Posts_List_Table::extra_tablenav() instead.

#11 @swissspidy
3 years ago

  • Keywords has-patch added; needs-patch removed

I found output buffering to be the only reliable way here. Here's how 37407.3.diff looks like in action:

https://cldup.com/qGADZGb3Od.png

@swissspidy
3 years ago

@dipesh.kakadiya
3 years ago

changes With test case

@dipesh.kakadiya
3 years ago

Updated test case

This ticket was mentioned in Slack in #core by dipeshkakadiya. View the logs.


3 years ago

@swissspidy
3 years ago

#13 @swissspidy
3 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

@dipesh.kakadiya Thanks for the tests, unfortunately they also pass without the fix applied. Also note that we don't get any notification simply for uploaded attachments. You'd need to write a comment as well.

Uploaded a new patch with tests.

@dipesh.kakadiya
3 years ago

minor typo 'are_now_posts' => 'are_not_posts' for testing functions

#14 @swissspidy
3 years ago

  • Keywords commit added

@dipesh.kakadiya Actually, it should be if_there_are_no_posts, not if_there_are_now_posts.

#15 @swissspidy
3 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 38854:

List Tables: Do not show filter button when there are no filter options available.

Props juhise, dipesh.kakadiya, swissspidy.
Fixes #37407.

Note: See TracTickets for help on using tickets.